Do you mean only a warning for the case this patch is handling, or also for cases such as the second test case in https://llvm.org/bugs/show_bug.cgi?id=25875#c1 too?
(I think it would probably be a good idea to warn in both cases.) On Fri, Jan 22, 2016 at 2:39 PM, Nico Weber <tha...@chromium.org> wrote: > Is it possible to emit some -Wmicrosoft warning in cases where this is > necessary? > > On Fri, Jan 22, 2016 at 2:26 PM, Ehsan Akhgari via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: ehsan >> Date: Fri Jan 22 13:26:44 2016 >> New Revision: 258530 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=258530&view=rev >> Log: >> [MSVC Compat] Accept elided commas in macro function arguments >> >> Summary: >> This fixes PR25875. When the trailing comma in a macro argument list is >> elided, we need to treat it similarly to the case where a variadic macro >> misses one actual argument. >> >> Reviewers: rnk, rsmith >> >> Subscribers: cfe-commits >> >> Differential Revision: http://reviews.llvm.org/D15670 >> >> Modified: >> cfe/trunk/include/clang/Lex/Token.h >> cfe/trunk/lib/Lex/PPMacroExpansion.cpp >> cfe/trunk/lib/Lex/TokenLexer.cpp >> cfe/trunk/test/Preprocessor/microsoft-ext.c >> >> Modified: cfe/trunk/include/clang/Lex/Token.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Token.h?rev=258530&r1=258529&r2=258530&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Lex/Token.h (original) >> +++ cfe/trunk/include/clang/Lex/Token.h Fri Jan 22 13:26:44 2016 >> @@ -85,6 +85,7 @@ public: >> IgnoredComma = 0x80, // This comma is not a macro argument >> separator (MS). >> StringifiedInMacro = 0x100, // This string or character literal is >> formed by >> // macro stringizing or charizing >> operator. >> + CommaAfterElided = 0x200, // The comma following this token was >> elided (MS). >> }; >> >> tok::TokenKind getKind() const { return Kind; } >> @@ -297,6 +298,11 @@ public: >> bool stringifiedInMacro() const { >> return (Flags & StringifiedInMacro) ? true : false; >> } >> + >> + /// Returns true if the comma after this token was elided. >> + bool commaAfterElided() const { >> + return (Flags & CommaAfterElided) ? true : false; >> + } >> }; >> >> /// \brief Information about the conditional stack (\#if directives) >> >> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=258530&r1=258529&r2=258530&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original) >> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Fri Jan 22 13:26:44 2016 >> @@ -723,6 +723,7 @@ MacroArgs *Preprocessor::ReadFunctionLik >> // heap allocations in the common case. >> SmallVector<Token, 64> ArgTokens; >> bool ContainsCodeCompletionTok = false; >> + bool FoundElidedComma = false; >> >> SourceLocation TooManyArgsLoc; >> >> @@ -765,6 +766,10 @@ MacroArgs *Preprocessor::ReadFunctionLik >> // If we found the ) token, the macro arg list is done. >> if (NumParens-- == 0) { >> MacroEnd = Tok.getLocation(); >> + if (!ArgTokens.empty() && >> + ArgTokens.back().commaAfterElided()) { >> + FoundElidedComma = true; >> + } >> break; >> } >> } else if (Tok.is(tok::l_paren)) { >> @@ -909,7 +914,7 @@ MacroArgs *Preprocessor::ReadFunctionLik >> // then we have an empty "()" argument empty list. This is fine, >> even if >> // the macro expects one argument (the argument is just empty). >> isVarargsElided = MI->isVariadic(); >> - } else if (MI->isVariadic() && >> + } else if ((FoundElidedComma || MI->isVariadic()) && >> (NumActuals+1 == MinArgsExpected || // A(x, ...) -> A(X) >> (NumActuals == 0 && MinArgsExpected == 2))) {// A(x,...) >> -> A() >> // Varargs where the named vararg parameter is missing: OK as >> extension. >> >> Modified: cfe/trunk/lib/Lex/TokenLexer.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/TokenLexer.cpp?rev=258530&r1=258529&r2=258530&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Lex/TokenLexer.cpp (original) >> +++ cfe/trunk/lib/Lex/TokenLexer.cpp Fri Jan 22 13:26:44 2016 >> @@ -154,12 +154,17 @@ bool TokenLexer::MaybeRemoveCommaBeforeV >> // Remove the comma. >> ResultToks.pop_back(); >> >> - // If the comma was right after another paste (e.g. >> "X##,##__VA_ARGS__"), >> - // then removal of the comma should produce a placemarker token (in C99 >> - // terms) which we model by popping off the previous ##, giving us a >> plain >> - // "X" when __VA_ARGS__ is empty. >> - if (!ResultToks.empty() && ResultToks.back().is(tok::hashhash)) >> - ResultToks.pop_back(); >> + if (!ResultToks.empty()) { >> + // If the comma was right after another paste (e.g. >> "X##,##__VA_ARGS__"), >> + // then removal of the comma should produce a placemarker token (in >> C99 >> + // terms) which we model by popping off the previous ##, giving us a >> plain >> + // "X" when __VA_ARGS__ is empty. >> + if (ResultToks.back().is(tok::hashhash)) >> + ResultToks.pop_back(); >> + >> + // Remember that this comma was elided. >> + ResultToks.back().setFlag(Token::CommaAfterElided); >> + } >> >> // Never add a space, even if the comma, ##, or arg had a space. >> NextTokGetsSpace = false; >> >> Modified: cfe/trunk/test/Preprocessor/microsoft-ext.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/microsoft-ext.c?rev=258530&r1=258529&r2=258530&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Preprocessor/microsoft-ext.c (original) >> +++ cfe/trunk/test/Preprocessor/microsoft-ext.c Fri Jan 22 13:26:44 2016 >> @@ -34,3 +34,12 @@ ACTION_TEMPLATE(InvokeArgument, >> >> MAKE_FUNC(MAK, ER, int a, _COMMA, int b); >> // CHECK: void func(int a , int b) {} >> + >> +#define macro(a, b) (a - b) >> +void function(int a); >> +#define COMMA_ELIDER(...) \ >> + macro(x, __VA_ARGS__); \ >> + function(x, __VA_ARGS__); >> +COMMA_ELIDER(); >> +// CHECK: (x - ); >> +// CHECK: function(x); >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > -- Ehsan
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
