Every time we accept something in MS mode that isn't standards compliant, we should accept it with a warning (for stuff that's harmless, an Extension warning, for stuff that can lead to bugs a Warning warning), unless it's really hard to implement.
(See also https://docs.google.com/presentation/d/1oxNHaVjA9Gn_rTzX6HIpJHP7nXRua_0URXxxJ3oYRq0/edit#slide=id.g71ecd450e_2_812 slide 27.) On Fri, Jan 22, 2016 at 2:44 PM, Ehsan Akhgari <ehsan.akhg...@gmail.com> wrote: > 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
