r258530 - [MSVC Compat] Accept elided commas in macro function arguments
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 22 12:22:50 PST 2016
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.akhgari at 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 <thakis at 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 at 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 at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
>
> --
> Ehsan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160122/94862f4d/attachment-0001.html>
More information about the cfe-commits
mailing list