Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -204,6 +204,7 @@ 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">; Index: include/clang/Basic/DiagnosticLexKinds.td =================================================================== --- include/clang/Basic/DiagnosticLexKinds.td +++ include/clang/Basic/DiagnosticLexKinds.td @@ -658,6 +658,13 @@ 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; +def warn_defined_in_function_type_macro : Extension< + "macro expansion producing 'defined' has undefined behavior">, + InGroup; + let CategoryName = "Nullability Issue" in { def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">; Index: lib/Lex/PPExpressions.cpp =================================================================== --- lib/Lex/PPExpressions.cpp +++ lib/Lex/PPExpressions.cpp @@ -82,6 +82,7 @@ static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT, bool ValueLive, Preprocessor &PP) { SourceLocation beginLoc(PeekTok.getLocation()); + Result.setBegin(beginLoc); // Get the next token, don't expand it. @@ -140,6 +141,51 @@ 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 +223,8 @@ 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 Index: test/Lexer/cxx-features.cpp =================================================================== --- test/Lexer/cxx-features.cpp +++ test/Lexer/cxx-features.cpp @@ -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 Index: test/Preprocessor/expr_define_expansion.c =================================================================== --- test/Preprocessor/expr_define_expansion.c +++ test/Preprocessor/expr_define_expansion.c @@ -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