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

[Aaron Ballman]

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

>      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
>