[PATCH] Emulate MSVC handling of commas during macro arg expansion

[Richard Smith]

On Tue, Jun 25, 2013 at 3:39 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>> Index: test/Preprocessor/microsoft-ext.c
>> ===================================================================
>> --- test/Preprocessor/microsoft-ext.c (revision 184863)
>> +++ test/Preprocessor/microsoft-ext.c (working copy)
>> @@ -22,3 +22,14 @@
>>  ACTION_TEMPLATE(InvokeArgument,
>>                  HAS_1_TEMPLATE_PARAMS(int, k),
>>                  AND_2_VALUE_PARAMS(p0, p1));
>> +
>> +
>> +// This tests compatibility with behaviour needed for type_traits in VS2012
>> +// Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012
>> +#define _COMMA ,
>> +
>> +#define MAKER(_arg1, _comma, _arg2) void func(_arg1 _comma _arg2) {}
>> +#define MAKE_FUNC(_makerP1, _makerP2, _arg1, _comma, _arg2) \
>> +                  _makerP1 ## _makerP2(_arg1, _comma, _arg2)
>> +
>> +MAKE_FUNC(MAK, ER, int a, _COMMA, int b); // CHECK: void func(int a , int b) {}
>> Index: include/clang/Lex/Token.h
>> ===================================================================
>> --- include/clang/Lex/Token.h (revision 184863)
>> +++ include/clang/Lex/Token.h (working copy)
>> @@ -74,6 +74,7 @@
>>      StartOfLine   = 0x01,  // At start of line or only after whitespace.
>>      LeadingSpace  = 0x02,  // Whitespace exists before this token.
>>      DisableExpand = 0x04,  // This identifier may never be macro expanded.
>> +    IgnoredComma = 0x04,   // [reused] Comma is not an argument separator (MS)
>
> I'm not too keen on reusing this value, even though it may be safe to do.

Agreed. We have a spare bit here, let's not worry about packing them
until we actually run out.

>>      NeedsCleaning = 0x08,  // Contained an escaped newline or trigraph.
>>      LeadingEmptyMacro = 0x10, // Empty macro exists before this token.
>>      HasUDSuffix = 0x20,    // This string or character literal has a ud-suffix.
>> Index: lib/Lex/TokenLexer.cpp
>> ===================================================================
>> --- lib/Lex/TokenLexer.cpp (revision 184863)
>> +++ lib/Lex/TokenLexer.cpp (working copy)
>> @@ -277,6 +277,14 @@
>>          unsigned FirstResult = ResultToks.size();
>>          unsigned NumToks = MacroArgs::getArgLength(ResultArgToks);
>>          ResultToks.append(ResultArgToks, ResultArgToks+NumToks);
>> +
>> +        // In Microsoft-compatibility mode, we follow MSVC's preprocessing
>> +        // behaviour by not considering single commas from nested macro
>> +        // expansions as argument separators. Set a flag on the token so we can
>> +        // test for this later when the macro expansion is processed.
>
> Trailing whitespace.  Also, not certain if we have a preference for US
> spellings or not (behavior instead of behaviour).
>
>> +        if (PP.getLangOpts().MicrosoftMode && NumToks == 1 &&
>> +            ResultToks.back().is(tok::comma))
>> +          ResultToks.back().setFlag(Token::IgnoredComma);
>>
>>          // If the '##' came from expanding an argument, turn it into 'unknown'
>>          // to avoid pasting.
>> Index: lib/Lex/PPMacroExpansion.cpp
>> ===================================================================
>> --- lib/Lex/PPMacroExpansion.cpp (revision 184863)
>> +++ lib/Lex/PPMacroExpansion.cpp (working copy)
>> @@ -458,7 +458,12 @@
>>          }
>>        } else if (Tok.is(tok::l_paren)) {
>>          ++NumParens;
>> -      } else if (Tok.is(tok::comma) && NumParens == 0) {
>> +      } else if (Tok.is(tok::comma) && NumParens == 0 &&
>> +                 !(Tok.getFlags() & Token::IgnoredComma)) {
>> +        // In Microsoft-compatibility mode, single commas from nested macro
>> +        // expansions should not be considered as argument separators. We test
>> +        // for this with the IgnoredComma token flag above.
>
> Trailing whitespace
>
>> +
>>          // Comma ends this argument if there are more fixed arguments expected.
>>          // However, if this is a variadic macro, and this is part of the
>>          // variadic part, then the comma is just an argument token.
>
> Other than the nitpicks, the patch LGTM and I'm very excited for it!
> Thank you for working on it.  :-)
>
> ~Aaron
>
> On Tue, Jun 25, 2013 at 5:44 PM, Will Wilson <will at indefiant.com> wrote:
>> Hi All,
>>
>> This patch is a rework of João's original patch applied as r163022 and later
>> reverted in r164672. I've spent a few (long...) days attempting to get to
>> the bottom of what caused the failures that led to the original patch being
>> reverted. This should fix (once and for all) the error seen when compiling
>> VS2012 type_traits and by extension most of the VS2012 STL headers:
>>
>> In file included from C:\Program Files (x86)\Microsoft Visual Studio
>> 11.0\VC\include\type_traits:1820:
>> C:\Program Files (x86)\Microsoft Visual Studio
>> 11.0\VC\include\xrefwrap(156,20): error : too many arguments provided to
>> function-like macro invocation
>>
>>
>> The main changes in this patch are:
>>
>> Tokens used as macro arguments that expand to a *single* comma have special
>> treatment - the comma will not be treated as a macro argument separator in
>> MicrosoftMode. Tokens that expand into multiple tokens that may include
>> commas are treated normally.
>> I've reused 0x04 for the IgnoredComma flag as the other case (DisableExpand)
>> is exclusively used for identifiers and won't conflict with tok::comma. It
>> didn't seem worth using up an entirely new value for such a rare edge case
>> as this.
>>
>> In terms of testing, clang should now be able to compile VS2012 type_traits.
>> I tested successfully against Nico's original GTest test case (PR13924) and
>> against boost.python 1.42.0 which also failed with the original patch. I've
>> also added a new test case based on the VS2012 xstddef macros that led to
>> the patch being needed in the first place.
>>
>> All tests passed against latest trunk. Please review and commit if it
>> LGTY...
>>
>> Cheers,
>> Will.
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits