[PATCH] D15866: Warn when using `defined` in a macro definition.
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 4 12:38:39 PST 2016
thakis created this revision.
thakis added a reviewer: rsmith.
thakis added a subscriber: cfe-commits.
As far as I can tell, doing
#define HAVE_FOO_BAR defined(FOO) && defined(BAR)
#if HAVE_FOO
...
#endif
has undefined behavior per [cpp.cond]p4. In practice, it can have different behavior in gcc and Visual Studio – see the comment in PPExpressions.cpp. So we should warn on this.
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 maybe this should be a Warning only for object-like macros and an Extension for function-like macros? But it's undefined behavior in both cases as far as I can tell.
http://reviews.llvm.org/D15866
Files:
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/PPExpressions.cpp
test/Lexer/cxx-features.cpp
test/Preprocessor/expr_define_expansion.c
Index: test/Lexer/cxx-features.cpp
===================================================================
--- test/Lexer/cxx-features.cpp
+++ test/Lexer/cxx-features.cpp
@@ -1,11 +1,12 @@
-// RUN: %clang_cc1 -std=c++98 -verify %s
-// RUN: %clang_cc1 -std=c++11 -verify %s
-// RUN: %clang_cc1 -std=c++1y -fsized-deallocation -verify %s
-// RUN: %clang_cc1 -std=c++1y -fsized-deallocation -fconcepts-ts -DCONCEPTS_TS=1 -verify %s
-// RUN: %clang_cc1 -fcoroutines -DCOROUTINES -verify %s
+// RUN: %clang_cc1 -Wno-expansion-to-defined -std=c++98 -verify %s
+// RUN: %clang_cc1 -Wno-expansion-to-defined -std=c++11 -verify %s
+// RUN: %clang_cc1 -Wno-expansion-to-defined -std=c++1y -fsized-deallocation -verify %s
+// RUN: %clang_cc1 -Wno-expansion-to-defined -std=c++1y -fsized-deallocation -fconcepts-ts -DCONCEPTS_TS=1 -verify %s
+// RUN: %clang_cc1 -Wno-expansion-to-defined -fcoroutines -DCOROUTINES -verify %s
// 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: lib/Lex/PPExpressions.cpp
===================================================================
--- lib/Lex/PPExpressions.cpp
+++ lib/Lex/PPExpressions.cpp
@@ -82,6 +82,28 @@
static bool EvaluateDefined(PPValue &Result, Token &PeekTok, DefinedTracker &DT,
bool ValueLive, Preprocessor &PP) {
SourceLocation beginLoc(PeekTok.getLocation());
+
+ // [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())
+ PP.Diag(beginLoc, diag::warn_defined_in_macro);
+
Result.setBegin(beginLoc);
// Get the next token, don't expand it.
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -656,6 +656,10 @@
def note_header_guard : Note<
"%0 is defined here; did you mean %1?">;
+def warn_defined_in_macro : Warning<
+ "macro expansion producing 'defined' has undefined behavior">,
+ InGroup<DiagGroup<"expansion-to-defined">>;
+
let CategoryName = "Nullability Issue" in {
def err_pp_assume_nonnull_syntax : Error<"expected 'begin' or 'end'">;
Index: test/Preprocessor/expr_define_expansion.c
===================================================================
--- test/Preprocessor/expr_define_expansion.c
+++ test/Preprocessor/expr_define_expansion.c
@@ -1,6 +1,10 @@
// RUN: %clang_cc1 %s -E -CC -pedantic -verify
-// expected-no-diagnostics
#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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15866.43912.patch
Type: text/x-patch
Size: 3617 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160104/6d3c769f/attachment.bin>
More information about the cfe-commits
mailing list