Re: [Tinycc-devel] 4 bugfixes for Rob Landley's revision 470

[Rob Landley]

On Wednesday 05 September 2007 3:28:57 pm Joshua Phillips wrote:
> I have made some fixes to tcc that may be useful...

Cool!  Thanks.

I'd like to apply these individually, and add test cases to the test suite, 
but I think I can break 'em up myself...

> Fixed variable-length array in struct:
>     struct foo {
>        int a;
>        void *b[];
>     };
>     sizeof(struct foo) is now 4 instead of 0.

Cool.

> Added opcode definition for 8-bit sign-extended immediates in 16/32-bit
> arithmetic
> not really a fix (hence not counted as one of the four), but I was a bit
> frustrated when asm ("addl $4,%esp") produced the long, 6-byte opcode. I
> fixed this to use the shorter opcode where possible.

Ok.

> Fix backslash parsing between a false #if/ifdef..#endif
>     as described in http://www.landley.net/code/tinycc/bugs/preprocessor.c

Cool.

Hmmm, could you give me some more information about the handle_stray_noerror() 
thing?  It seems like rather than special-casing \r\n, we might want to just 
handle any trailing whitespace after a \ line extender?  (This isn't specific 
to your patch, this is the underlying function being more brittle than I'm 
comfortable with.)

Yeah, I know c99-draft 5.1.1.2 #1 pargraph 2 says that only \n immediately 
after a backslash gets removed, but we're already breaking that looking for 
\r...

Heh.  The obvious way to code this is:

/* handle '\[\r]\n' */
static int handle_stray_noerror(void)
{
    while (ch == '\\') {
        inp();
        skip_spaces();
        if (ch == '\n') {
            file->line_num++;
            inp();
        } else return 1;
    }
    return 0;
}

And then have handle_stray() do: if(handle_stray_noerror() error("blah");

Except that doesn't work, naievely because skip_spaces hasn't been declared 
yet, but more fundamentally the because skip_spaces() doesn't call inp(), it 
calls cinp().  What's cinp()?  It's a #define to minp().  (WHY???)  And 
minp(), of course, is defined as:

static void minp(void)
{
    inp();
    if (ch == '\\')
        handle_stray();
}

Congratulations, we have achieved recursion.

Grrr.  I note also that is_space() is shadowing the global "ch" with its local 
declaration of ch.  And that ch is hardish to grep for because it appears 
in "char".

Right...  Fix committed, lemme handle the rest in another email.

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.