[cfe-commits] [patch] Elided __VA_ARGS__ now erase preceding comma in MS mode

[Aaron Ballman]

On Mon, Aug 27, 2012 at 9:10 AM, Martin Vejnár <Martin.Vejnar at avg.com> wrote:
> From: aaron.ballman at gmail.com [mailto:aaron.ballman at gmail.com]
>> On Mon, Aug 27, 2012 at 5:42 AM, Martin Vejnár <Martin.Vejnar at avg.com>
>> wrote:
>> > I’ve beed asked to post this patch to cfe-commits, so there you go.
>> > The preprocessor show now consume a comma before elided
>> __VA_ARGS__
>> > even if there is no pasting operator in between. E.g.
>>
>> In the future, can you submit as an svn patch since not everyone uses git?
>
> Sure, sorry for that, I didn't realize it would be difficult to apply.

No worries!

>> > +    if (PP.getLangOpts().MicrosoftMode &&
>> > +        (unsigned)ArgNo == Macro->getNumArgs()-1 && // is __VA_ARGS__
>> > +        ActualArgs->isVarargsElidedUse() &&       // Argument elided.
>> > +        !ResultToks.empty() && ResultToks.back().is(tok::comma)) {
>> > +      // Never add a space, even if the comma or arg had a space.
>> > +      NextTokGetsSpace = false;
>> > +
>> > +      // Remove the comma
>> > +      ResultToks.pop_back();
>> > +
>> > +      // If the comma was right after another paste (e.g.
>> "X##,__VA_ARGS__"),
>> > +      // the paste is ignored by MS compilers.
>>
>> Minor nit: the comment doesn't seem to match the code.  You've checked
>> for a comma then a hash, but the comment implies you've found a hash and
>> then a comma.
>
> No, actually, the comment is correct. ResultToks is a vector of tokens that have already been consumed, so they are popped from right to left (the comma is popped, then the preceding hashhash).

Ahhhh, okay, that makes far more sense then!  Thanks for the explanation.

>> > +      if (!ResultToks.empty() && ResultToks.back().is(tok::hashhash))
>> > +        ResultToks.pop_back();
>> > +      continue;
>> > +    }
>> > +
>>
>> >
>> > --- /dev/null
>> > +++ b/test/Preprocessor/macro_fn_ms_comma_swallow.c
>> > @@ -0,0 +1,7 @@
>> > +// Test the GNU comma swallowing extension.
>> > +// RUN: %clang_cc1 %s -E -fms-compatibility | FileCheck
>> > +-strict-whitespace %s
>> > +
>> > +// should eat the comma before emtpy varargs // CHECK: 1: {foo}
>> > +#define X1(b, ...) {b,__VA_ARGS__}
>> > +1: X1(foo)
>>
>> Instead of creating an entire test case for this, it might make more sense to
>> put this test macro_fn_comma_swallow.c and add a second pass to the test
>> using a check prefix for MS.
>
> Oh, I didn't know it was possible. I've read up on -check-prefix, thanks.
>
>> Otherwise, everything looks good!  Thanks for the patch!
>
> Thank you! :)

My pleasure!

~Aaron