<div dir="ltr">Does this overlap at all with GCC's -Waddress? (it might be that it should be a subgroup of -Waddress - or generalized to catch some more cases (what about &x == 0 or if (&x)))</div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Tue, Jan 28, 2014 at 9:05 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">
Extend -Wtautological-compare to warn when an array is compared to a null value. Arrays can be implicitly converted to a valid pointer, so it will never be null. Thus, the result of the comparison can be known at run time.<br>
<br>
This will not warn on externally linked weak arrays, which can be null, and inside macros, which can sometimes expect pointers. Several tests were updated to disable this warning.<br>
<br>
int foo() {<br>
int array[5];<br>
if (array != 0) // New warning here<br>
return array[0];<br>
else<br>
return 0;<br>
}<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D2645" target="_blank">http://llvm-reviews.chandlerc.com/D2645</a><br>
<br>
Files:<br>
test/Sema/const-eval.c<br>
test/SemaCXX/nullptr_in_arithmetic_ops.cpp<br>
test/SemaCXX/warn-tautological-compare.cpp<br>
test/SemaCXX/null_in_arithmetic_ops.cpp<br>
test/SemaCXX/constant-expression-cxx11.cpp<br>
include/clang/Basic/DiagnosticSemaKinds.td<br>
include/clang/Basic/DiagnosticGroups.td<br>
lib/Sema/SemaExpr.cpp<br>
<br>
Index: test/Sema/const-eval.c<br>
===================================================================<br>
--- test/Sema/const-eval.c<br>
+++ test/Sema/const-eval.c<br>
@@ -1,4 +1,4 @@<br>
-// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-linux %s<br>
+// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-linux %s -Wno-tautological-array-compare<br>
<br>
#define EVAL_EXPR(testno, expr) int test##testno = sizeof(struct{char qq[expr];});<br>
int x;<br>
Index: test/SemaCXX/nullptr_in_arithmetic_ops.cpp<br>
===================================================================<br>
--- test/SemaCXX/nullptr_in_arithmetic_ops.cpp<br>
+++ test/SemaCXX/nullptr_in_arithmetic_ops.cpp<br>
@@ -1,4 +1,4 @@<br>
-// RUN: %clang_cc1 -fsyntax-only -fblocks -std=c++11 -verify %s<br>
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-array-compare -fblocks -std=c++11 -verify %s<br>
<br>
void foo() {<br>
int a;<br>
Index: test/SemaCXX/warn-tautological-compare.cpp<br>
===================================================================<br>
--- test/SemaCXX/warn-tautological-compare.cpp<br>
+++ test/SemaCXX/warn-tautological-compare.cpp<br>
@@ -25,3 +25,40 @@<br>
if (x < kintmax) {}<br>
}<br>
}<br>
+<br>
+namespace ArrayCompare {<br>
+#define GetValue(ptr) ((ptr != 0) ? ptr[0] : 0)<br>
+ extern int a[] __attribute__((weak));<br>
+ int b[] = {8,13,21};<br>
+ struct {<br>
+ int x[10];<br>
+ } c;<br>
+ const char str[] = "text";<br>
+ void ignore() {<br>
+ if (a == 0) {}<br>
+ if (a != 0) {}<br>
+ (void)GetValue(b);<br>
+ }<br>
+ void test() {<br>
+ if (b == 0) {}<br>
+ // expected-warning@-1{{comparison of array of type 'int [3]' equal to a null pointer is always false}}<br>
+ if (b != 0) {}<br>
+ // expected-warning@-1{{comparison of array of type 'int [3]' not equal to a null pointer is always true}}<br>
+ if (0 == b) {}<br>
+ // expected-warning@-1{{comparison of array of type 'int [3]' equal to a null pointer is always false}}<br>
+ if (0 != b) {}<br>
+ // expected-warning@-1{{comparison of array of type 'int [3]' not equal to a null pointer is always true}}<br>
+ if ("true" == 0) {}<br>
+ // expected-warning@-1{{comparison of array of type 'const char [5]' equal to a null pointer is always false}}<br>
+ if ("true" != 0) {}<br>
+ // expected-warning@-1{{comparison of array of type 'const char [5]' not equal to a null pointer is always true}}<br>
+ if (c.x != 0) {}<br>
+ // expected-warning@-1{{comparison of array of type 'int [10]' not equal to a null pointer is always true}}<br>
+ if (c.x != 0) {}<br>
+ // expected-warning@-1{{comparison of array of type 'int [10]' not equal to a null pointer is always true}}<br>
+ if (str == 0) {}<br>
+ // expected-warning@-1{{comparison of array of type 'const char [5]' equal to a null pointer is always false}}<br>
+ if (str != 0) {}<br>
+ // expected-warning@-1{{comparison of array of type 'const char [5]' not equal to a null pointer is always true}}<br>
+ }<br>
+}<br>
Index: test/SemaCXX/null_in_arithmetic_ops.cpp<br>
===================================================================<br>
--- test/SemaCXX/null_in_arithmetic_ops.cpp<br>
+++ test/SemaCXX/null_in_arithmetic_ops.cpp<br>
@@ -1,4 +1,4 @@<br>
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -fblocks -Wnull-arithmetic -verify -Wno-string-plus-int %s<br>
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -fblocks -Wnull-arithmetic -verify -Wno-string-plus-int -Wno-tautological-array-compare %s<br>
#include <stddef.h><br>
<br>
void f() {<br>
Index: test/SemaCXX/constant-expression-cxx11.cpp<br>
===================================================================<br>
--- test/SemaCXX/constant-expression-cxx11.cpp<br>
+++ test/SemaCXX/constant-expression-cxx11.cpp<br>
@@ -1,4 +1,4 @@<br>
-// RUN: %clang_cc1 -triple i686-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -fsyntax-only -fcxx-exceptions -verify -std=c++11 -pedantic %s -Wno-comment<br>
+// RUN: %clang_cc1 -triple i686-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -fsyntax-only -fcxx-exceptions -verify -std=c++11 -pedantic %s -Wno-comment -Wno-tautological-array-compare<br>
<br>
namespace StaticAssertFoldTest {<br>
<br>
Index: include/clang/Basic/DiagnosticSemaKinds.td<br>
===================================================================<br>
--- include/clang/Basic/DiagnosticSemaKinds.td<br>
+++ include/clang/Basic/DiagnosticSemaKinds.td<br>
@@ -4643,6 +4643,10 @@<br>
def warn_runsigned_always_true_comparison : Warning<<br>
"comparison of %0 unsigned%select{| enum}2 expression is always %1">,<br>
InGroup<TautologicalCompare>;<br>
+def warn_tautological_array_compare : Warning<<br>
+ "comparison of array of type %0 %select{not equal to|equal to}1 a null "<br>
+ "pointer is always %select{true|false}1">,<br>
+ InGroup<TautologicalArrayCompare>;<br>
def warn_comparison_of_mixed_enum_types : Warning<<br>
"comparison of two values with different enumeration types"<br>
"%diff{ ($ and $)|}0,1">,<br>
Index: include/clang/Basic/DiagnosticGroups.td<br>
===================================================================<br>
--- include/clang/Basic/DiagnosticGroups.td<br>
+++ include/clang/Basic/DiagnosticGroups.td<br>
@@ -283,8 +283,10 @@<br>
def StringPlusChar : DiagGroup<"string-plus-char">;<br>
def StrncatSize : DiagGroup<"strncat-size">;<br>
def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;<br>
+def TautologicalArrayCompare : DiagGroup<"tautological-array-compare">;<br>
def TautologicalCompare : DiagGroup<"tautological-compare",<br>
- [TautologicalOutOfRangeCompare]>;<br>
+ [TautologicalOutOfRangeCompare,<br>
+ TautologicalArrayCompare]>;<br>
def HeaderHygiene : DiagGroup<"header-hygiene">;<br>
def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;<br>
def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">;<br>
Index: lib/Sema/SemaExpr.cpp<br>
===================================================================<br>
--- lib/Sema/SemaExpr.cpp<br>
+++ lib/Sema/SemaExpr.cpp<br>
@@ -7646,6 +7646,30 @@<br>
}<br>
}<br>
<br>
+static void checkArrayNullCompare(Sema &S, SourceLocation Loc,<br>
+ ExprResult &ArrayExpr, ExprResult &NullExpr,<br>
+ bool IsEquality) {<br>
+ if (Loc.isMacroID())<br>
+ return;<br>
+<br>
+ Expr *Array = ArrayExpr.get()->IgnoreParens();<br>
+ ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Array);<br>
+ if (!ICE || ICE->getCastKind() != CK_ArrayToPointerDecay)<br>
+ return;<br>
+<br>
+ Array = ICE->getSubExpr();<br>
+<br>
+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Array)) {<br>
+ ValueDecl *VD = DRE->getDecl();<br>
+ if (VD->hasExternalFormalLinkage() && VD->isWeak())<br>
+ return;<br>
+ }<br>
+<br>
+ S.Diag(Loc, diag::warn_tautological_array_compare)<br>
+ << Array->getSourceRange() << NullExpr.get()->getSourceRange()<br>
+ << Array->getType() << IsEquality;<br>
+}<br>
+<br>
static void diagnoseLogicalNotOnLHSofComparison(Sema &S, ExprResult &LHS,<br>
ExprResult &RHS,<br>
SourceLocation Loc,<br>
@@ -7830,6 +7854,14 @@<br>
bool RHSIsNull = RHS.get()->isNullPointerConstant(Context,<br>
Expr::NPC_ValueDependentIsNull);<br>
<br>
+ if (!IsRelational && LHSIsNull != RHSIsNull) {<br>
+ bool IsEquality = Opc == BO_EQ;<br>
+ if (RHSIsNull)<br>
+ checkArrayNullCompare(*this, Loc, LHS, RHS, IsEquality);<br>
+ else<br>
+ checkArrayNullCompare(*this, Loc, RHS, LHS, IsEquality);<br>
+ }<br>
+<br>
// All of the following pointer-related warnings are GCC extensions, except<br>
// when handling null pointer constants.<br>
if (LHSType->isPointerType() && RHSType->isPointerType()) { // C99 6.5.8p2<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>
<br></blockquote></div><br></div>