<div dir="ltr">I guess the idea is to disable this warning on debug builds then, right?<div><br></div><div>assert(&a != NULL) or assert(this) are things that make sense to write in debug builds: The comparison won't be optimized away, and it helps catch bugs where code accidentally calls a non-virtual method on a NULL this, or where code as a reference to a NULL pointer.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Aug 8, 2014 at 3:41 PM, Richard Trieu <span dir="ltr"><<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rtrieu<br>
Date: Fri Aug 8 17:41:43 2014<br>
New Revision: 215251<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=215251&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=215251&view=rev</a><br>
Log:<br>
Extend tautological pointer compare and pointer to bool conversion warnings to<br>
macro arguments.<br>
<br>
Previously, these warnings skipped any code in a macro expansion. Preform an<br>
additional check and warn when the expression and context locations are both<br>
in the macro argument.<br>
<br>
The most obvious case not caught is passing a pointer directly to a macro,<br>
i.e 'assert(&array)' but 'assert(&array && "valid array")' is caught. This is<br>
because macro arguments are not typed and the conversion happens inside the<br>
macro.<br>
<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaChecking.cpp<br>
cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp<br>
cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp<br>
cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp<br>
cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=215251&r1=215250&r2=215251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=215251&r1=215250&r2=215251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Aug 8 17:41:43 2014<br>
@@ -6340,6 +6340,22 @@ static bool CheckForReference(Sema &Sema<br>
return true;<br>
}<br>
<br>
+// Returns true if the SourceLocation is expanded from any macro body.<br>
+// Returns false if the SourceLocation is invalid, is from not in a macro<br>
+// expansion, or is from expanded from a top-level macro argument.<br>
+static bool IsInAnyMacroBody(const SourceManager &SM, SourceLocation Loc) {<br>
+ if (Loc.isInvalid())<br>
+ return false;<br>
+<br>
+ while (Loc.isMacroID()) {<br>
+ if (SM.isMacroBodyExpansion(Loc))<br>
+ return true;<br>
+ Loc = SM.getImmediateMacroCallerLoc(Loc);<br>
+ }<br>
+<br>
+ return false;<br>
+}<br>
+<br>
/// \brief Diagnose pointers that are always non-null.<br>
/// \param E the expression containing the pointer<br>
/// \param NullKind NPCK_NotNull if E is a cast to bool, otherwise, E is<br>
@@ -6353,8 +6369,12 @@ void Sema::DiagnoseAlwaysNonNullPointer(<br>
return;<br>
<br>
// Don't warn inside macros.<br>
- if (E->getExprLoc().isMacroID())<br>
+ if (E->getExprLoc().isMacroID()) {<br>
+ const SourceManager &SM = getSourceManager();<br>
+ if (IsInAnyMacroBody(SM, E->getExprLoc()) ||<br>
+ IsInAnyMacroBody(SM, Range.getBegin()))<br>
return;<br>
+ }<br>
E = E->IgnoreImpCasts();<br>
<br>
const bool IsCompare = NullKind != Expr::NPCK_NotNull;<br>
<br>
Modified: cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp?rev=215251&r1=215250&r2=215251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp?rev=215251&r1=215250&r2=215251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-bool-conversion.cpp Fri Aug 8 17:41:43 2014<br>
@@ -118,3 +118,30 @@ namespace Pointer {<br>
// expected-warning@-1{{address of 'S::a' will always evaluate to 'true'}}<br>
}<br>
}<br>
+<br>
+namespace macros {<br>
+ #define assert(x) if (x) {}<br>
+ #define zero_on_null(x) ((x) ? *(x) : 0)<br>
+<br>
+ int array[5];<br>
+ void fun();<br>
+ int x;<br>
+<br>
+ void test() {<br>
+ assert(array);<br>
+ assert(array && "expecting null pointer");<br>
+ // expected-warning@-1{{address of array 'array' will always evaluate to 'true'}}<br>
+<br>
+ assert(fun);<br>
+ assert(fun && "expecting null pointer");<br>
+ // expected-warning@-1{{address of function 'fun' will always evaluate to 'true'}}<br>
+ // expected-note@-2 {{prefix with the address-of operator to silence this warning}}<br>
+<br>
+ // TODO: warn on assert(&x) while not warning on zero_on_null(&x)<br>
+ zero_on_null(&x);<br>
+ assert(zero_on_null(&x));<br>
+ assert(&x);<br>
+ assert(&x && "expecting null pointer");<br>
+ // expected-warning@-1{{address of 'x' will always evaluate to 'true'}}<br>
+ }<br>
+}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp?rev=215251&r1=215250&r2=215251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp?rev=215251&r1=215250&r2=215251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-tautological-compare.cpp Fri Aug 8 17:41:43 2014<br>
@@ -136,3 +136,43 @@ namespace PointerCompare {<br>
// expected-warning@-1{{comparison of address of 'S::a' equal to a null pointer is always false}}<br>
}<br>
}<br>
+<br>
+namespace macros {<br>
+ #define assert(x) if (x) {}<br>
+ int array[5];<br>
+ void fun();<br>
+ int x;<br>
+<br>
+ void test() {<br>
+ assert(array == 0);<br>
+ // expected-warning@-1{{comparison of array 'array' equal to a null pointer is always false}}<br>
+ assert(array != 0);<br>
+ // expected-warning@-1{{comparison of array 'array' not equal to a null pointer is always true}}<br>
+ assert(array == 0 && "expecting null pointer");<br>
+ // expected-warning@-1{{comparison of array 'array' equal to a null pointer is always false}}<br>
+ assert(array != 0 && "expecting non-null pointer");<br>
+ // expected-warning@-1{{comparison of array 'array' not equal to a null pointer is always true}}<br>
+<br>
+ assert(fun == 0);<br>
+ // expected-warning@-1{{comparison of function 'fun' equal to a null pointer is always false}}<br>
+ // expected-note@-2{{prefix with the address-of operator to silence this warning}}<br>
+ assert(fun != 0);<br>
+ // expected-warning@-1{{comparison of function 'fun' not equal to a null pointer is always true}}<br>
+ // expected-note@-2{{prefix with the address-of operator to silence this warning}}<br>
+ assert(fun == 0 && "expecting null pointer");<br>
+ // expected-warning@-1{{comparison of function 'fun' equal to a null pointer is always false}}<br>
+ // expected-note@-2{{prefix with the address-of operator to silence this warning}}<br>
+ assert(fun != 0 && "expecting non-null pointer");<br>
+ // expected-warning@-1{{comparison of function 'fun' not equal to a null pointer is always true}}<br>
+ // expected-note@-2{{prefix with the address-of operator to silence this warning}}<br>
+<br>
+ assert(&x == 0);<br>
+ // expected-warning@-1{{comparison of address of 'x' equal to a null pointer is always false}}<br>
+ assert(&x != 0);<br>
+ // expected-warning@-1{{comparison of address of 'x' not equal to a null pointer is always true}}<br>
+ assert(&x == 0 && "expecting null pointer");<br>
+ // expected-warning@-1{{comparison of address of 'x' equal to a null pointer is always false}}<br>
+ assert(&x != 0 && "expecting non-null pointer");<br>
+ // expected-warning@-1{{comparison of address of 'x' not equal to a null pointer is always true}}<br>
+ }<br>
+}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp?rev=215251&r1=215250&r2=215251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp?rev=215251&r1=215250&r2=215251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp Fri Aug 8 17:41:43 2014<br>
@@ -110,3 +110,31 @@ namespace function_return_reference {<br>
// 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}}<br>
}<br>
}<br>
+<br>
+namespace macros {<br>
+ #define assert(x) if (x) {}<br>
+<br>
+ void test(int &x) {<br>
+ assert(&x != 0);<br>
+ // 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}}<br>
+ assert(&x == 0);<br>
+ // 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}}<br>
+ assert(&x != 0 && "Expecting valid reference");<br>
+ // 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}}<br>
+ assert(&x == 0 && "Expecting invalid reference");<br>
+ // 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}}<br>
+ }<br>
+<br>
+ class S {<br>
+ void test() {<br>
+ assert(this != 0);<br>
+ // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to true}}<br>
+ assert(this == 0);<br>
+ // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false}}<br>
+ assert(this != 0 && "Expecting valid reference");<br>
+ // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to true}}<br>
+ assert(this == 0 && "Expecting invalid reference");<br>
+ // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false}}<br>
+ }<br>
+ };<br>
+}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp?rev=215251&r1=215250&r2=215251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp?rev=215251&r1=215250&r2=215251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp Fri Aug 8 17:41:43 2014<br>
@@ -95,3 +95,27 @@ namespace function_return_reference {<br>
// 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}}<br>
}<br>
}<br>
+<br>
+namespace macros {<br>
+ #define assert(x) if (x) {}<br>
+ #define zero_on_null(x) ((x) ? *(x) : 0)<br>
+<br>
+ void test(int &x) {<br>
+ // TODO: warn on assert(&x) but not on zero_on_null(&x)<br>
+ zero_on_null(&x);<br>
+ assert(zero_on_null(&x));<br>
+ assert(&x);<br>
+<br>
+ assert(&x && "Expecting valid reference");<br>
+ // 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}}<br>
+ }<br>
+<br>
+ class S {<br>
+ void test() {<br>
+ assert(this);<br>
+<br>
+ assert(this && "Expecting invalid reference");<br>
+ // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true}}<br>
+ }<br>
+ };<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>