[PATCH] Emulate MSVC handling of commas during macro arg expansion
Will Wilson
will at indefiant.com
Tue Jun 25 23:29:49 PDT 2013
Thanks for the reviews!
Updated patch attached with unique flag value, spelling change and trailing
whitespace culled. Can someone commit if it looks acceptable?
- Will.
On 26 June 2013 01:57, Richard Smith <richard at metafoo.co.uk> wrote:
> 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
>
--
*Indefiant Ltd.*
*
*
Firsby Lodge, New Main Road, Scamblesby, Louth, Lincs LN11 9XH UK
*Tel: +44 20 8123 7663 England Registered No. 07936820 VAT No. **128556202*
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130626/eaa93ffe/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: msvc_macros_commas.patch
Type: application/octet-stream
Size: 3440 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130626/eaa93ffe/attachment.obj>
More information about the cfe-commits
mailing list