This generates hundreds of warnings when doing check-all.

Here's the offending code:


utils/unittest/googletest/include/gtest/internal/gtest-port.h

// Cygwin 1.7 and below doesn't support ::std::wstring.
// Solaris' libc++ doesn't support it either.  Android has
// no support for it at least as recent as Froyo (2.2).
// Minix currently doesn't support it either.
# define GTEST_HAS_STD_WSTRING \
(!(GTEST_OS_LINUX_ANDROID || GTEST_OS_CYGWIN || GTEST_OS_SOLARIS || GTEST_OS_HAIKU || defined(_MINIX)))


-Krzysztof



On 1/19/2016 9:15 AM, Nico Weber via cfe-commits 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



--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to