r215251 - Extend tautological pointer compare and pointer to bool conversion warnings to

Richard Trieu rtrieu at google.com
Fri Aug 8 15:41:43 PDT 2014


Author: rtrieu
Date: Fri Aug  8 17:41:43 2014
New Revision: 215251

URL: http://llvm.org/viewvc/llvm-project?rev=215251&view=rev
Log:
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.


Modified:
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp
    cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp
    cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp
    cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=215251&r1=215250&r2=215251&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Aug  8 17:41:43 2014
@@ -6340,6 +6340,22 @@ static bool CheckForReference(Sema &Sema
   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(
     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;

Modified: cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp?rev=215251&r1=215250&r2=215251&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp Fri Aug  8 17:41:43 2014
@@ -118,3 +118,30 @@ namespace Pointer {
     // expected-warning at -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 at -1{{address of array 'array' will always evaluate to 'true'}}
+
+    assert(fun);
+    assert(fun && "expecting null pointer");
+    // expected-warning at -1{{address of function 'fun' will always evaluate to 'true'}}
+    // expected-note at -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 at -1{{address of 'x' will always evaluate to 'true'}}
+  }
+}

Modified: cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp?rev=215251&r1=215250&r2=215251&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp Fri Aug  8 17:41:43 2014
@@ -136,3 +136,43 @@ namespace PointerCompare {
     // expected-warning at -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 at -1{{comparison of array 'array' equal to a null pointer is always false}}
+    assert(array != 0);
+    // expected-warning at -1{{comparison of array 'array' not equal to a null pointer is always true}}
+    assert(array == 0 && "expecting null pointer");
+    // expected-warning at -1{{comparison of array 'array' equal to a null pointer is always false}}
+    assert(array != 0 && "expecting non-null pointer");
+    // expected-warning at -1{{comparison of array 'array' not equal to a null pointer is always true}}
+
+    assert(fun == 0);
+    // expected-warning at -1{{comparison of function 'fun' equal to a null pointer is always false}}
+    // expected-note at -2{{prefix with the address-of operator to silence this warning}}
+    assert(fun != 0);
+    // expected-warning at -1{{comparison of function 'fun' not equal to a null pointer is always true}}
+    // expected-note at -2{{prefix with the address-of operator to silence this warning}}
+    assert(fun == 0 && "expecting null pointer");
+    // expected-warning at -1{{comparison of function 'fun' equal to a null pointer is always false}}
+    // expected-note at -2{{prefix with the address-of operator to silence this warning}}
+    assert(fun != 0 && "expecting non-null pointer");
+    // expected-warning at -1{{comparison of function 'fun' not equal to a null pointer is always true}}
+    // expected-note at -2{{prefix with the address-of operator to silence this warning}}
+
+    assert(&x == 0);
+    // expected-warning at -1{{comparison of address of 'x' equal to a null pointer is always false}}
+    assert(&x != 0);
+    // expected-warning at -1{{comparison of address of 'x' not equal to a null pointer is always true}}
+    assert(&x == 0 && "expecting null pointer");
+    // expected-warning at -1{{comparison of address of 'x' equal to a null pointer is always false}}
+    assert(&x != 0 && "expecting non-null pointer");
+    // expected-warning at -1{{comparison of address of 'x' not equal to a null pointer is always true}}
+  }
+}

Modified: cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp?rev=215251&r1=215250&r2=215251&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp Fri Aug  8 17:41:43 2014
@@ -110,3 +110,31 @@ namespace function_return_reference {
     // expected-warning at -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 at -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 at -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 at -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 at -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 at -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 at -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 at -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 at -1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false}}
+    }
+  };
+}

Modified: cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp?rev=215251&r1=215250&r2=215251&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp Fri Aug  8 17:41:43 2014
@@ -95,3 +95,27 @@ namespace function_return_reference {
     // expected-warning at -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 at -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 at -1{{'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true}}
+    }
+  };
+}





More information about the cfe-commits mailing list