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@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@+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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
