Extend tautological pointer compare and pointer to bool conversion warnings to

macro arguments.

Previously, these warnings skipped any code in a macro expansion.  Preform an
additional check and warn when the expression and context locations are both
in the macro argument.

The most obvious case not caught is passing a pointer directly to a macro,
i.e 'assert(&array)' but 'assert(&array && "valid array")' is caught.  This is
because macro arguments are not typed and the conversion happens inside the
macro.

llvm-svn: 215251

clang/lib/Sema/SemaChecking.cpp

@@ -6340,6 +6340,22 @@ static bool CheckForReference(Sema &SemaRef, const Expr *E,
   return true;
 }
 
+// Returns true if the SourceLocation is expanded from any macro body.
+// Returns false if the SourceLocation is invalid, is from not in a macro
+// expansion, or is from expanded from a top-level macro argument.
+static bool IsInAnyMacroBody(const SourceManager &SM, SourceLocation Loc) {
+  if (Loc.isInvalid())
+    return false;
+
+  while (Loc.isMacroID()) {
+    if (SM.isMacroBodyExpansion(Loc))
+      return true;
+    Loc = SM.getImmediateMacroCallerLoc(Loc);
+  }
+
+  return false;
+}
+
 /// \brief Diagnose pointers that are always non-null.
 /// \param E the expression containing the pointer
 /// \param NullKind NPCK_NotNull if E is a cast to bool, otherwise, E is
@@ -6353,8 +6369,12 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E,
     return;
 
   // Don't warn inside macros.
-  if (E->getExprLoc().isMacroID())
+  if (E->getExprLoc().isMacroID()) {
+    const SourceManager &SM = getSourceManager();
+    if (IsInAnyMacroBody(SM, E->getExprLoc()) ||
+        IsInAnyMacroBody(SM, Range.getBegin()))
       return;
+  }
   E = E->IgnoreImpCasts();
 
   const bool IsCompare = NullKind != Expr::NPCK_NotNull;

clang/test/SemaCXX/warn-bool-conversion.cpp

@@ -118,3 +118,30 @@ namespace Pointer {
     // expected-warning@-1{{address of 'S::a' will always evaluate to 'true'}}
   }
 }
+
+namespace macros {
+  #define assert(x) if (x) {}
+  #define zero_on_null(x) ((x) ? *(x) : 0)
+
+  int array[5];
+  void fun();
+  int x;
+
+  void test() {
+    assert(array);
+    assert(array && "expecting null pointer");
+    // expected-warning@-1{{address of array 'array' will always evaluate to 'true'}}
+
+    assert(fun);
+    assert(fun && "expecting null pointer");
+    // expected-warning@-1{{address of function 'fun' will always evaluate to 'true'}}
+    // expected-note@-2 {{prefix with the address-of operator to silence this warning}}
+
+    // TODO: warn on assert(&x) while not warning on zero_on_null(&x)
+    zero_on_null(&x);
+    assert(zero_on_null(&x));
+    assert(&x);
+    assert(&x && "expecting null pointer");
+    // expected-warning@-1{{address of 'x' will always evaluate to 'true'}}
+  }
+}

clang/test/SemaCXX/warn-tautological-compare.cpp

@@ -136,3 +136,43 @@ namespace PointerCompare {
     // expected-warning@-1{{comparison of address of 'S::a' equal to a null pointer is always false}}
   }
 }
+
+namespace macros {
+  #define assert(x) if (x) {}
+  int array[5];
+  void fun();
+  int x;
+
+  void test() {
+    assert(array == 0);
+    // expected-warning@-1{{comparison of array 'array' equal to a null pointer is always false}}
+    assert(array != 0);
+    // expected-warning@-1{{comparison of array 'array' not equal to a null pointer is always true}}
+    assert(array == 0 && "expecting null pointer");
+    // expected-warning@-1{{comparison of array 'array' equal to a null pointer is always false}}
+    assert(array != 0 && "expecting non-null pointer");
+    // expected-warning@-1{{comparison of array 'array' not equal to a null pointer is always true}}
+
+    assert(fun == 0);
+    // expected-warning@-1{{comparison of function 'fun' equal to a null pointer is always false}}
+    // expected-note@-2{{prefix with the address-of operator to silence this warning}}
+    assert(fun != 0);
+    // expected-warning@-1{{comparison of function 'fun' not equal to a null pointer is always true}}
+    // expected-note@-2{{prefix with the address-of operator to silence this warning}}
+    assert(fun == 0 && "expecting null pointer");
+    // expected-warning@-1{{comparison of function 'fun' equal to a null pointer is always false}}
+    // expected-note@-2{{prefix with the address-of operator to silence this warning}}
+    assert(fun != 0 && "expecting non-null pointer");
+    // expected-warning@-1{{comparison of function 'fun' not equal to a null pointer is always true}}
+    // expected-note@-2{{prefix with the address-of operator to silence this warning}}
+
+    assert(&x == 0);
+    // expected-warning@-1{{comparison of address of 'x' equal to a null pointer is always false}}
+    assert(&x != 0);
+    // expected-warning@-1{{comparison of address of 'x' not equal to a null pointer is always true}}
+    assert(&x == 0 && "expecting null pointer");
+    // expected-warning@-1{{comparison of address of 'x' equal to a null pointer is always false}}
+    assert(&x != 0 && "expecting non-null pointer");
+    // expected-warning@-1{{comparison of address of 'x' not equal to a null pointer is always true}}
+  }
+}

clang/test/SemaCXX/warn-tautological-undefined-compare.cpp

@@ -110,3 +110,31 @@ namespace function_return_reference {
     // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
   }
 }
+
+namespace macros {
+  #define assert(x) if (x) {}
+
+  void test(int &x) {
+    assert(&x != 0);
+    // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
+    assert(&x == 0);
+    // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to false}}
+    assert(&x != 0 && "Expecting valid reference");
+    // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to true}}
+    assert(&x == 0 && "Expecting invalid reference");
+    // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; comparison may be assumed to always evaluate to false}}
+  }
+
+  class S {
+    void test() {
+      assert(this != 0);
+      // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to true}}
+      assert(this == 0);
+      // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false}}
+      assert(this != 0 && "Expecting valid reference");
+      // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to true}}
+      assert(this == 0 && "Expecting invalid reference");
+      // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false}}
+    }
+  };
+}

clang/test/SemaCXX/warn-undefined-bool-conversion.cpp

@@ -95,3 +95,27 @@ namespace function_return_reference {
     // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; pointer may be assumed to always convert to true}}
   }
 }
+
+namespace macros {
+  #define assert(x) if (x) {}
+  #define zero_on_null(x) ((x) ? *(x) : 0)
+
+  void test(int &x) {
+    // TODO: warn on assert(&x) but not on zero_on_null(&x)
+    zero_on_null(&x);
+    assert(zero_on_null(&x));
+    assert(&x);
+
+    assert(&x && "Expecting valid reference");
+    // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; pointer may be assumed to always convert to true}}
+  }
+
+  class S {
+    void test() {
+      assert(this);
+
+      assert(this && "Expecting invalid reference");
+      // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true}}
+    }
+  };
+}