[PATCH] Emulate MSVC handling of commas during macro arg expansion
Reid Kleckner
rnk at google.com
Wed Jun 26 10:18:25 PDT 2013
I also tried this patch on a bunch of gtest code and on an iostream hello
world program using VS 2012's iostream and it works for both. Awesome!
I can also repro the assertions Aaron hits in the Preprocessor tests.
They're caused by:
- HasUCN = 0x40 // This identifier contains a UCN.
+ HasUCN = 0x40, // This identifier contains a UCN.
+ IgnoredComma = 0x08, // This comma is not a macro argument separator
(MS).
It should be 0x80, not 0x08.
With that change, everything passes, and I committed it as r184968.
On Wed, Jun 26, 2013 at 9:18 AM, Aaron Ballman <aaron at aaronballman.com>wrote:
> I applied the patch, but am getting crashes with a few of the test
> cases. Eg) test\Preprocessor\macro_fn_comma_swallow2.c has an
> -fms-compatibility test which fails:
>
> Assertion failed: Length < Tok.getLength() && "NeedsCleaning flag set
> on token that didn't need cleaning!", file
> F:\official-llvm\llvm\tools\clang\lib\Lex\Lexer.cpp, line 290
> Stack dump:
> 0. Program arguments: build\bin\debug\clang -cc1 -E -fms-compatibility
> llvm\tools\clang\test\Preprocessor\macro_fn_comma_swallow2.c
> 0x65E4C94A (0x0000000A 0x00000000 0x0544E640 0x65F27114), exit() +
> 0x13A bytes(s)
> 0x65F37A9C (0x0544E684 0x0544E654 0x0544DB18 0x0544DB48), abort() +
> 0x1C bytes(s)
> 0x65F27114 (0x04527580 0x04527518 0x00000122 0x0544E888), _wassert() +
> 0xC04 bytes(s)
> 0x0221129B (0x0544E8F8 0x0036228E 0x0033FEE0 0x0544E77C),
> getSpellingSlow() + 0x19B bytes(s),
> f:\official-llvm\llvm\tools\clang\lib\lex\lexer.cpp, line 290 + 0x2D
> byte(s)
> 0x02209890 (0x0544E8F8 0x0544E730 0x00344DE0 0x0033FEE0),
> clang::Lexer::getSpelling() + 0x150 bytes(s),
> f:\official-llvm\llvm\tools\clang\lib\lex\lexer.cpp, line 412 + 0x17
> byte(s)
> 0x021BBE5D (0x0544E8F8 0x0544E730 0x00000000 0x0544E914),
> clang::Preprocessor::getSpelling() + 0x2D bytes(s),
> f:\official-llvm\llvm\tools\clang\include\clang\lex\preprocessor.h,
> line 992 + 0x1F byte(s)
> 0x021BAAC9 (0x003494E0 0x0544E8F8 0x003600C8 0x00391260),
> PrintPreprocessedTokens() + 0x249 bytes(s),
> f:\official-llvm\llvm\tools\clang\lib\frontend\printpreprocessedoutput.cpp,
> line 634 + 0x15 byte(s)
> 0x021B955A (0x003494E0 0x00391260 0x00342CD0 0x0544E9F4),
> clang::DoPrintPreprocessedInput() + 0x28A bytes(s),
> f:\official-llvm\llvm\tools\clang\lib\frontend\printpreprocessedoutput.cpp,
> line 744 + 0x15 byte(s)
> 0x020F948D (0x0544E9A0 0xCCCCCCCC 0xCCCCCCCC 0xCCCCCCCC),
> clang::PrintPreprocessedAction::ExecuteAction() + 0x14D bytes(s),
> f:\official-llvm\llvm\tools\clang\lib\frontend\frontendactions.cpp,
> line 556 + 0x1B byte(s)
> 0x020F4F6C (0x0544EA90 0x0544FAAC 0xCCCCCCCC 0xCCCCCCCC),
> clang::FrontendAction::Execute() + 0xAC bytes(s),
> f:\official-llvm\llvm\tools\clang\lib\frontend\frontendaction.cpp,
> line 378 + 0xF byte(s)
> 0x020E0081 (0x00345E18 0x0544EFF0 0xCCCCCCCC 0xCCCCCCCC),
> clang::CompilerInstance::ExecuteAction() + 0x281 bytes(s),
> f:\official-llvm\llvm\tools\clang\lib\frontend\compilerinstance.cpp,
> line 686
> 0x01E1B3B8 (0x0033FE50 0x0544FEE0 0xCCCCCCCC 0xCCCCCCCC),
> clang::ExecuteCompilerInvocation() + 0x308 bytes(s),
>
> f:\official-llvm\llvm\tools\clang\lib\frontendtool\executecompilerinvocation.cpp,
> line 236 + 0x11 byte(s)
> 0x00BE6BC0 (0x0544FAB4 0x0544FAC0 0x00340298 0x00BD1320), cc1_main() +
> 0x2F0 bytes(s),
> f:\official-llvm\llvm\tools\clang\tools\driver\cc1_main.cpp, line 100
> + 0xE byte(s)
> 0x00BD5399 (0x00000005 0x00339B10 0x0033B330 0x7A2FBA24), main() +
> 0x149 bytes(s),
> f:\official-llvm\llvm\tools\clang\tools\driver\driver.cpp, line 360 +
> 0x45 byte(s)
> 0x02F32139 (0x0544FF44 0x749633AA 0x7EFDE000 0x0544FF84),
> __tmainCRTStartup() + 0x199 bytes(s),
> f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c, line 536 + 0x19
> byte(s)
> 0x02F3227D (0x7EFDE000 0x0544FF84 0x76EC9EF2 0x7EFDE000),
> mainCRTStartup() + 0xD bytes(s),
> f:\dd\vctools\crt_bld\self_x86\crt\src\crtexe.c, line 377
> 0x749633AA (0x7EFDE000 0xEAA34A4D 0x00000000 0x00000000),
> BaseThreadInitThunk() + 0x12 bytes(s)
> 0x76EC9EF2 (0x02F32270 0x7EFDE000 0x00000000 0x00000000),
> RtlInitializeExceptionChain() + 0x63 bytes(s)
> 0x76EC9EC5 (0x02F32270 0x7EFDE000 0x00000000 0x00000000),
> RtlInitializeExceptionChain() + 0x36 bytes(s)
>
> I get the same failure with Preprocessor\microsoft-ext.c
>
> Do all of the test cases pass cleanly for you?
>
> Thanks!
>
> ~Aaron
>
> On Wed, Jun 26, 2013 at 2:29 AM, Will Wilson <will at indefiant.com> wrote:
> > 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
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130626/3c168c16/attachment.html>
More information about the cfe-commits
mailing list