r258128 - Add -Wexpansion-to-undefined: warn when using `defined` in a macro definition.
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 19 12:30:10 PST 2016
I'll take a look. If it's urgent it's also possible to disable this warning.
On Jan 19, 2016 2:29 PM, "Diego Novillo via cfe-commits" <
cfe-commits at lists.llvm.org> wrote:
> Nico, this is producing tons of warnings on an LLVM build and is actually
> breaking our internal builds (we build with -Werror).
>
> I fixed one file that was producing this, but there are several that have
> the same problem (e.g., gtest-port.h). Could you fix them or rollback?
>
>
> Thanks. Diego.
>
> On Tue, Jan 19, 2016 at 10:15 AM, Nico Weber via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: nico
>> Date: Tue Jan 19 09:15:31 2016
>> New Revision: 258128
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=258128&view=rev
>> Log:
>> Add -Wexpansion-to-undefined: warn when using `defined` in a macro
>> definition.
>>
>> [cpp.cond]p4:
>> Prior to evaluation, macro invocations in the list of preprocessing
>> tokens that will become the controlling constant expression are replaced
>> (except for those macro names modified by the 'defined' unary operator),
>> just as in normal text. If the token 'defined' is generated as a result
>> of this replacement process or use of the 'defined' unary operator does
>> not match one of the two specified forms prior to macro replacement, the
>> behavior is undefined.
>>
>> This isn't an idle threat, consider this program:
>> #define FOO
>> #define BAR defined(FOO)
>> #if BAR
>> ...
>> #else
>> ...
>> #endif
>> clang and gcc will pick the #if branch while Visual Studio will take the
>> #else branch. Emit a warning about this undefined behavior.
>>
>> One problem is that this also applies to function-like macros. While the
>> example above can be written like
>>
>> #if defined(FOO) && defined(BAR)
>> #defined HAVE_FOO 1
>> #else
>> #define HAVE_FOO 0
>> #endif
>>
>> there is no easy way to rewrite a function-like macro like `#define FOO(x)
>> (defined __foo_##x && __foo_##x)`. Function-like macros like this are
>> used in
>> practice, and compilers seem to not have differing behavior in that case.
>> So
>> this a default-on warning only for object-like macros. For function-like
>> macros, it is an extension warning that only shows up with `-pedantic`.
>> (But it's undefined behavior in both cases.)
>>
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>> cfe/trunk/lib/Lex/PPExpressions.cpp
>> cfe/trunk/test/Lexer/cxx-features.cpp
>> cfe/trunk/test/Preprocessor/expr_define_expansion.c
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Jan 19 09:15:31
>> 2016
>> @@ -204,6 +204,7 @@ def OverloadedShiftOpParentheses: DiagGr
>> def DanglingElse: DiagGroup<"dangling-else">;
>> def DanglingField : DiagGroup<"dangling-field">;
>> def DistributedObjectModifiers :
>> DiagGroup<"distributed-object-modifiers">;
>> +def ExpansionToUndefined : DiagGroup<"expansion-to-undefined">;
>> def FlagEnum : DiagGroup<"flag-enum">;
>> def IncrementBool : DiagGroup<"increment-bool",
>> [DeprecatedIncrementBool]>;
>> def InfiniteRecursion : DiagGroup<"infinite-recursion">;
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Jan 19
>> 09:15:31 2016
>> @@ -658,6 +658,13 @@ def warn_header_guard : Warning<
>> def note_header_guard : Note<
>> "%0 is defined here; did you mean %1?">;
>>
>> +def warn_defined_in_object_type_macro : Warning<
>> + "macro expansion producing 'defined' has undefined behavior">,
>> + InGroup<ExpansionToUndefined>;
>> +def warn_defined_in_function_type_macro : Extension<
>> + "macro expansion producing 'defined' has undefined behavior">,
>> + InGroup<ExpansionToUndefined>;
>> +
>> let CategoryName = "Nullability Issue" in {
>>
>> def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
>>
>> Modified: cfe/trunk/lib/Lex/PPExpressions.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPExpressions.cpp?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Lex/PPExpressions.cpp (original)
>> +++ cfe/trunk/lib/Lex/PPExpressions.cpp Tue Jan 19 09:15:31 2016
>> @@ -140,6 +140,51 @@ static bool EvaluateDefined(PPValue &Res
>> PP.LexNonComment(PeekTok);
>> }
>>
>> + // [cpp.cond]p4:
>> + // Prior to evaluation, macro invocations in the list of
>> preprocessing
>> + // tokens that will become the controlling constant expression are
>> replaced
>> + // (except for those macro names modified by the 'defined' unary
>> operator),
>> + // just as in normal text. If the token 'defined' is generated as a
>> result
>> + // of this replacement process or use of the 'defined' unary
>> operator does
>> + // not match one of the two specified forms prior to macro
>> replacement, the
>> + // behavior is undefined.
>> + // This isn't an idle threat, consider this program:
>> + // #define FOO
>> + // #define BAR defined(FOO)
>> + // #if BAR
>> + // ...
>> + // #else
>> + // ...
>> + // #endif
>> + // clang and gcc will pick the #if branch while Visual Studio will
>> take the
>> + // #else branch. Emit a warning about this undefined behavior.
>> + if (beginLoc.isMacroID()) {
>> + bool IsFunctionTypeMacro =
>> + PP.getSourceManager()
>> + .getSLocEntry(PP.getSourceManager().getFileID(beginLoc))
>> + .getExpansion()
>> + .isFunctionMacroExpansion();
>> + // For object-type macros, it's easy to replace
>> + // #define FOO defined(BAR)
>> + // with
>> + // #if defined(BAR)
>> + // #define FOO 1
>> + // #else
>> + // #define FOO 0
>> + // #endif
>> + // and doing so makes sense since compilers handle this differently
>> in
>> + // practice (see example further up). But for function-type macros,
>> + // there is no good way to write
>> + // # define FOO(x) (defined(M_ ## x) && M_ ## x)
>> + // in a different way, and compilers seem to agree on how to behave
>> here.
>> + // So warn by default on object-type macros, but only warn in
>> -pedantic
>> + // mode on function-type macros.
>> + if (IsFunctionTypeMacro)
>> + PP.Diag(beginLoc, diag::warn_defined_in_function_type_macro);
>> + else
>> + PP.Diag(beginLoc, diag::warn_defined_in_object_type_macro);
>> + }
>> +
>> // Invoke the 'defined' callback.
>> if (PPCallbacks *Callbacks = PP.getPPCallbacks()) {
>> Callbacks->Defined(macroToken, Macro,
>> @@ -177,8 +222,8 @@ static bool EvaluateValue(PPValue &Resul
>> if (IdentifierInfo *II = PeekTok.getIdentifierInfo()) {
>> // Handle "defined X" and "defined(X)".
>> if (II->isStr("defined"))
>> - return(EvaluateDefined(Result, PeekTok, DT, ValueLive, PP));
>> -
>> + return EvaluateDefined(Result, PeekTok, DT, ValueLive, PP);
>> +
>> // If this identifier isn't 'defined' or one of the special
>> // preprocessor keywords and it wasn't macro expanded, it turns
>> // into a simple 0, unless it is the C++ keyword "true", in which
>> case it
>>
>> Modified: cfe/trunk/test/Lexer/cxx-features.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/cxx-features.cpp?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Lexer/cxx-features.cpp (original)
>> +++ cfe/trunk/test/Lexer/cxx-features.cpp Tue Jan 19 09:15:31 2016
>> @@ -6,6 +6,7 @@
>>
>> // expected-no-diagnostics
>>
>> +// FIXME using `defined` in a macro has undefined behavior.
>> #if __cplusplus < 201103L
>> #define check(macro, cxx98, cxx11, cxx1y) cxx98 == 0 ?
>> defined(__cpp_##macro) : __cpp_##macro != cxx98
>> #elif __cplusplus < 201304L
>>
>> Modified: cfe/trunk/test/Preprocessor/expr_define_expansion.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/expr_define_expansion.c?rev=258128&r1=258127&r2=258128&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Preprocessor/expr_define_expansion.c (original)
>> +++ cfe/trunk/test/Preprocessor/expr_define_expansion.c Tue Jan 19
>> 09:15:31 2016
>> @@ -1,6 +1,28 @@
>> -// RUN: %clang_cc1 %s -E -CC -pedantic -verify
>> -// expected-no-diagnostics
>> +// RUN: %clang_cc1 %s -E -CC -verify
>> +// RUN: %clang_cc1 %s -E -CC -DPEDANTIC -pedantic -verify
>>
>> #define FOO && 1
>> #if defined FOO FOO
>> #endif
>> +
>> +#define A
>> +#define B defined(A)
>> +#if B // expected-warning{{macro expansion producing 'defined' has
>> undefined behavior}}
>> +#endif
>> +
>> +#define m_foo
>> +#define TEST(a) (defined(m_##a) && a)
>> +
>> +#if defined(PEDANTIC)
>> +// expected-warning at +4{{macro expansion producing 'defined' has
>> undefined behavior}}
>> +#endif
>> +
>> +// This shouldn't warn by default, only with pedantic:
>> +#if TEST(foo)
>> +#endif
>> +
>> +
>> +// Only one diagnostic for this case:
>> +#define INVALID defined(
>> +#if INVALID // expected-error{{macro name missing}}
>> +#endif
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160119/b5971311/attachment-0001.html>
More information about the cfe-commits
mailing list