[PATCH] Emulate MSVC handling of commas during macro arg expansion
Aaron Ballman
aaron at aaronballman.com
Wed Jun 26 06:18:06 PDT 2013
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
More information about the cfe-commits
mailing list