<div dir="ltr"><div style>Thanks for the reviews!</div><div style><br></div>Updated patch attached with unique flag value, spelling change and trailing whitespace culled. Can someone commit if it looks acceptable?<div><br>
</div><div style>- Will.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On 26 June 2013 01:57, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Jun 25, 2013 at 3:39 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
>> Index: test/Preprocessor/microsoft-ext.c<br>
>> ===================================================================<br>
>> --- test/Preprocessor/microsoft-ext.c (revision 184863)<br>
>> +++ test/Preprocessor/microsoft-ext.c (working copy)<br>
>> @@ -22,3 +22,14 @@<br>
>> ACTION_TEMPLATE(InvokeArgument,<br>
>> HAS_1_TEMPLATE_PARAMS(int, k),<br>
>> AND_2_VALUE_PARAMS(p0, p1));<br>
>> +<br>
>> +<br>
>> +// This tests compatibility with behaviour needed for type_traits in VS2012<br>
>> +// Test based on _VARIADIC_EXPAND_0X macros in xstddef of VS2012<br>
>> +#define _COMMA ,<br>
>> +<br>
>> +#define MAKER(_arg1, _comma, _arg2) void func(_arg1 _comma _arg2) {}<br>
>> +#define MAKE_FUNC(_makerP1, _makerP2, _arg1, _comma, _arg2) \<br>
>> + _makerP1 ## _makerP2(_arg1, _comma, _arg2)<br>
>> +<br>
>> +MAKE_FUNC(MAK, ER, int a, _COMMA, int b); // CHECK: void func(int a , int b) {}<br>
>> Index: include/clang/Lex/Token.h<br>
>> ===================================================================<br>
>> --- include/clang/Lex/Token.h (revision 184863)<br>
>> +++ include/clang/Lex/Token.h (working copy)<br>
>> @@ -74,6 +74,7 @@<br>
>> StartOfLine = 0x01, // At start of line or only after whitespace.<br>
>> LeadingSpace = 0x02, // Whitespace exists before this token.<br>
>> DisableExpand = 0x04, // This identifier may never be macro expanded.<br>
>> + IgnoredComma = 0x04, // [reused] Comma is not an argument separator (MS)<br>
><br>
> I'm not too keen on reusing this value, even though it may be safe to do.<br>
<br>
</div></div>Agreed. We have a spare bit here, let's not worry about packing them<br>
until we actually run out.<br>
<div class="HOEnZb"><div class="h5"><br>
>> NeedsCleaning = 0x08, // Contained an escaped newline or trigraph.<br>
>> LeadingEmptyMacro = 0x10, // Empty macro exists before this token.<br>
>> HasUDSuffix = 0x20, // This string or character literal has a ud-suffix.<br>
>> Index: lib/Lex/TokenLexer.cpp<br>
>> ===================================================================<br>
>> --- lib/Lex/TokenLexer.cpp (revision 184863)<br>
>> +++ lib/Lex/TokenLexer.cpp (working copy)<br>
>> @@ -277,6 +277,14 @@<br>
>> unsigned FirstResult = ResultToks.size();<br>
>> unsigned NumToks = MacroArgs::getArgLength(ResultArgToks);<br>
>> ResultToks.append(ResultArgToks, ResultArgToks+NumToks);<br>
>> +<br>
>> + // In Microsoft-compatibility mode, we follow MSVC's preprocessing<br>
>> + // behaviour by not considering single commas from nested macro<br>
>> + // expansions as argument separators. Set a flag on the token so we can<br>
>> + // test for this later when the macro expansion is processed.<br>
><br>
> Trailing whitespace. Also, not certain if we have a preference for US<br>
> spellings or not (behavior instead of behaviour).<br>
><br>
>> + if (PP.getLangOpts().MicrosoftMode && NumToks == 1 &&<br>
>> + ResultToks.back().is(tok::comma))<br>
>> + ResultToks.back().setFlag(Token::IgnoredComma);<br>
>><br>
>> // If the '##' came from expanding an argument, turn it into 'unknown'<br>
>> // to avoid pasting.<br>
>> Index: lib/Lex/PPMacroExpansion.cpp<br>
>> ===================================================================<br>
>> --- lib/Lex/PPMacroExpansion.cpp (revision 184863)<br>
>> +++ lib/Lex/PPMacroExpansion.cpp (working copy)<br>
>> @@ -458,7 +458,12 @@<br>
>> }<br>
>> } else if (Tok.is(tok::l_paren)) {<br>
>> ++NumParens;<br>
>> - } else if (Tok.is(tok::comma) && NumParens == 0) {<br>
>> + } else if (Tok.is(tok::comma) && NumParens == 0 &&<br>
>> + !(Tok.getFlags() & Token::IgnoredComma)) {<br>
>> + // In Microsoft-compatibility mode, single commas from nested macro<br>
>> + // expansions should not be considered as argument separators. We test<br>
>> + // for this with the IgnoredComma token flag above.<br>
><br>
> Trailing whitespace<br>
><br>
>> +<br>
>> // Comma ends this argument if there are more fixed arguments expected.<br>
>> // However, if this is a variadic macro, and this is part of the<br>
>> // variadic part, then the comma is just an argument token.<br>
><br>
> Other than the nitpicks, the patch LGTM and I'm very excited for it!<br>
> Thank you for working on it. :-)<br>
><br>
> ~Aaron<br>
><br>
> On Tue, Jun 25, 2013 at 5:44 PM, Will Wilson <<a href="mailto:will@indefiant.com">will@indefiant.com</a>> wrote:<br>
>> Hi All,<br>
>><br>
>> This patch is a rework of João's original patch applied as r163022 and later<br>
>> reverted in r164672. I've spent a few (long...) days attempting to get to<br>
>> the bottom of what caused the failures that led to the original patch being<br>
>> reverted. This should fix (once and for all) the error seen when compiling<br>
>> VS2012 type_traits and by extension most of the VS2012 STL headers:<br>
>><br>
>> In file included from C:\Program Files (x86)\Microsoft Visual Studio<br>
>> 11.0\VC\include\type_traits:1820:<br>
>> C:\Program Files (x86)\Microsoft Visual Studio<br>
>> 11.0\VC\include\xrefwrap(156,20): error : too many arguments provided to<br>
>> function-like macro invocation<br>
>><br>
>><br>
>> The main changes in this patch are:<br>
>><br>
>> Tokens used as macro arguments that expand to a *single* comma have special<br>
>> treatment - the comma will not be treated as a macro argument separator in<br>
>> MicrosoftMode. Tokens that expand into multiple tokens that may include<br>
>> commas are treated normally.<br>
>> I've reused 0x04 for the IgnoredComma flag as the other case (DisableExpand)<br>
>> is exclusively used for identifiers and won't conflict with tok::comma. It<br>
>> didn't seem worth using up an entirely new value for such a rare edge case<br>
>> as this.<br>
>><br>
>> In terms of testing, clang should now be able to compile VS2012 type_traits.<br>
>> I tested successfully against Nico's original GTest test case (PR13924) and<br>
>> against boost.python 1.42.0 which also failed with the original patch. I've<br>
>> also added a new test case based on the VS2012 xstddef macros that led to<br>
>> the patch being needed in the first place.<br>
>><br>
>> All tests passed against latest trunk. Please review and commit if it<br>
>> LGTY...<br>
>><br>
>> Cheers,<br>
>> Will.<br>
>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><span style="background-color:rgb(255,255,255);color:rgb(68,68,68);font-family:Arial,Helvetica,sans-serif"><b>Indefiant Ltd.</b></span></div><div>
<font color="#444444" face="Arial, Helvetica, sans-serif"><span style="font-size:12px"><b><br></b></span></font></div><font color="#444444" face="Arial, Helvetica, sans-serif"><span style="font-size:12px">Firsby Lodge, New Main Road, Scamblesby, Louth, Lincs LN11 9XH UK<br>
</span></font><div style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0px;padding-top:0px;padding-right:0px;padding-bottom:0px;padding-left:0px;background-color:rgb(255,255,255)"><span style="color:rgb(68,68,68);font-family:Arial,Helvetica,sans-serif;font-size:x-small"><i>Tel: +44 20 8123 7663 England Registered No. 07936820 VAT No. </i></span><span style="background-color:transparent"><font color="#444444" face="Arial, Helvetica, sans-serif" size="1"><i>128556202</i></font></span></div>
</div>