Re: RFA: fix macro expansion bug

[Pedro Alves <pedro@codesourcery.com>]

On Thursday 11 December 2008 21:36:24, Tom Tromey wrote:
> This patch fixes a macro expansion bug pointed out by Pedro.
> 
> The bug is that get_pp_number incorrectly handles pp-number tokens
> starting with '.'.  According to the C standard, it ought to only
> accept '.' followed by a digit at the beginning of a pp-number.

Indeed it does.  Clearly a bug.

> However, it unconditionally accepts a leading '.'.
> 
> Built and regtested on x86-64 (compile farm).
> Test case included.
> 
> Please review.
> 

Looks good, this is OK.

Though, I'm having a bit of trouble convincing myself that the logic to
handle 'pp-number e|E|p|P|. sign' below is 100% sane.

static int
get_pp_number (struct macro_buffer *tok, char *p, char *end)
{
...
      while (p < end)
        {
          if (macro_is_digit (*p)
              || macro_is_identifier_nondigit (*p)
              || *p == '.')
            p++;
          else if (p + 2 <= end
                   && strchr ("eEpP.", *p)
                   && (p[1] == '+' || p[1] == '-'))
            p += 2;
          else
            break;
        }

It seems macro_is_identifier_nondigit will always eat any of "eEpP",
thus, say, when parsing "1e-" only "1e" will be identified as a pp
number, leaving "+" in the stream.  Is this right?

> +
> +# Regression test for pp-number bug.
> +gdb_test "macro define si_addr fields.fault.si_addr" \
> +  "" \
> +  "define si_addr macro"
> +gdb_test "macro expand siginfo.si_addr" \
> +  "expands to: siginfo. fields.fault.si_addr" \
                          ^
Just curious, as it's just a visual annoyance: do you know where
this space comes from?  Do we store the definition with the space for
some reason?  We don't get that extra space if the define came
from the code, instead of from a 'macro define'.

-- 
Pedro Alves