<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>