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

[Martin Vejnár]

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.

> > +    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).

> > +      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! :)
-- 
Martin