Warn about comparisons between arrays and improve self-comparison

warnings, from Troy Straszheim! Fixes PR6163.

llvm-svn: 105631

Author: DougGregor

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -2917,8 +2917,10 @@ def err_ret_local_block : Error<
 
 // For non-floating point, expressions of the form x == x or x != x
 // should result in a warning, since these always evaluate to a constant.
-def warn_selfcomparison : Warning<
-  "self-comparison always results in a constant value">;
+// Array comparisons have similar warnings
+def warn_comparison_always : Warning<
+  "%select{self-|array }0comparison always evaluates to %select{false|true|a constant}1">;
+
 def warn_stringcompare : Warning<
   "result of comparison against %select{a string literal|@encode}0 is "
   "unspecified (use strncmp instead)">;

clang/lib/Sema/SemaExpr.cpp

@@ -5292,13 +5292,6 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
   if (lex->getType()->isVectorType() || rex->getType()->isVectorType())
     return CheckVectorCompareOperands(lex, rex, Loc, isRelational);
 
-  // C99 6.5.8p3 / C99 6.5.9p4
-  if (lex->getType()->isArithmeticType() && rex->getType()->isArithmeticType())
-    UsualArithmeticConversions(lex, rex);
-  else {
-    UsualUnaryConversions(lex);
-    UsualUnaryConversions(rex);
-  }
   QualType lType = lex->getType();
   QualType rType = rex->getType();
 
@@ -5312,10 +5305,36 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     Expr *LHSStripped = lex->IgnoreParens();
     Expr *RHSStripped = rex->IgnoreParens();
     if (DeclRefExpr* DRL = dyn_cast<DeclRefExpr>(LHSStripped))
-      if (DeclRefExpr* DRR = dyn_cast<DeclRefExpr>(RHSStripped))
+      if (DeclRefExpr* DRR = dyn_cast<DeclRefExpr>(RHSStripped)) {
         if (DRL->getDecl() == DRR->getDecl() &&
-            !isa<EnumConstantDecl>(DRL->getDecl()))
-          DiagRuntimeBehavior(Loc, PDiag(diag::warn_selfcomparison));
+            !isa<EnumConstantDecl>(DRL->getDecl())) {
+          DiagRuntimeBehavior(Loc, PDiag(diag::warn_comparison_always)
+                              << 0 // self-
+                              << (Opc == BinaryOperator::EQ
+                                  || Opc == BinaryOperator::LE
+                                  || Opc == BinaryOperator::GE));
+        } else if (lType->isArrayType() && rType->isArrayType() &&
+                   !DRL->getDecl()->getType()->isReferenceType() &&
+                   !DRR->getDecl()->getType()->isReferenceType()) {
+            // what is it always going to eval to?
+            char always_evals_to;
+            switch(Opc) {
+            case BinaryOperator::EQ: // e.g. array1 == array2
+              always_evals_to = 0; // false
+              break;
+            case BinaryOperator::NE: // e.g. array1 != array2
+              always_evals_to = 1; // true
+              break;
+            default:
+              // best we can say is 'a constant'
+              always_evals_to = 2; // e.g. array1 <= array2
+              break;
+            }
+            DiagRuntimeBehavior(Loc, PDiag(diag::warn_comparison_always)
+                                << 1 // array
+                                << always_evals_to);
+        }
+      }
 
     if (isa<CastExpr>(LHSStripped))
       LHSStripped = LHSStripped->IgnoreParenCasts();
@@ -5358,6 +5377,17 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     }
   }
 
+  // C99 6.5.8p3 / C99 6.5.9p4
+  if (lex->getType()->isArithmeticType() && rex->getType()->isArithmeticType())
+    UsualArithmeticConversions(lex, rex);
+  else {
+    UsualUnaryConversions(lex);
+    UsualUnaryConversions(rex);
+  }
+
+  lType = lex->getType();
+  rType = rex->getType();
+
   // The result of comparisons is 'bool' in C++, 'int' in C.
   QualType ResultTy = getLangOptions().CPlusPlus ? Context.BoolTy:Context.IntTy;
 
@@ -5632,7 +5662,11 @@ QualType Sema::CheckVectorCompareOperands(Expr *&lex, Expr *&rex,
     if (DeclRefExpr* DRL = dyn_cast<DeclRefExpr>(lex->IgnoreParens()))
       if (DeclRefExpr* DRR = dyn_cast<DeclRefExpr>(rex->IgnoreParens()))
         if (DRL->getDecl() == DRR->getDecl())
-          DiagRuntimeBehavior(Loc, PDiag(diag::warn_selfcomparison));
+          DiagRuntimeBehavior(Loc,
+                              PDiag(diag::warn_comparison_always)
+                                << 0 // self-
+                                << 2 // "a constant"
+                              );
   }
 
   // Check for comparisons of floating point operands using != and ==.

clang/test/Sema/compare.c

@@ -3,7 +3,7 @@
 int test(char *C) { // nothing here should warn.
   return C != ((void*)0);
   return C != (void*)0;
-  return C != 0;  
+  return C != 0;
   return C != 1;  // expected-warning {{comparison between pointer and integer ('char *' and 'int')}}
 }
 
@@ -218,7 +218,7 @@ int pointers(int *a) {
 
 int function_pointers(int (*a)(int), int (*b)(int), void (*c)(int)) {
   return a > b; // expected-warning {{ordered comparison of function pointers}}
-  return function_pointers > function_pointers; // expected-warning {{ordered comparison of function pointers}}
+  return function_pointers > function_pointers; // expected-warning {{self-comparison always evaluates to false}} expected-warning{{ordered comparison of function pointers}}
   return a > c; // expected-warning {{comparison of distinct pointer types}}
   return a == (void *) 0;
   return a == (void *) 1; // expected-warning {{equality comparison between function pointer and void pointer}}
@@ -229,6 +229,7 @@ int void_pointers(void* foo) {
   return foo == (void*) 1;
 }
 
+
 int test1(int i) {
   enum en { zero };
   return i > zero;

clang/test/Sema/ext_vector_comparisons.c

@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-unreachable-code %s
+
+typedef __attribute__(( ext_vector_type(4) )) int int4;
+
+static int4 test1() {
+  int4 vec, rv;
+
+  // comparisons to self...
+  return vec == vec; // expected-warning{{self-comparison always evaluates to a constant}}
+  return vec != vec; // expected-warning{{self-comparison always evaluates to a constant}}
+  return vec < vec; // expected-warning{{self-comparison always evaluates to a constant}}
+  return vec <= vec; // expected-warning{{self-comparison always evaluates to a constant}}
+  return vec > vec; // expected-warning{{self-comparison always evaluates to a constant}}
+  return vec >= vec; // expected-warning{{self-comparison always evaluates to a constant}}
+}
+
+
+typedef __attribute__(( ext_vector_type(4) )) float float4;
+
+static int4 test2() {
+  float4 vec, rv;
+
+  // comparisons to self.  no warning, they're floats
+  return vec == vec; // no-warning
+  return vec != vec; // no-warning
+  return vec < vec;  // no-warning
+  return vec <= vec; // no-warning
+  return vec > vec;  // no-warning
+  return vec >= vec; // no-warning
+}

clang/test/Sema/self-comparison.c

@@ -1,11 +1,21 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
 int foo(int x) {
-  return x == x; // expected-warning {{self-comparison always results}}
+  return x == x; // expected-warning {{self-comparison always evaluates to true}}
 }
 
 int foo2(int x) {
-  return (x) != (((x))); // expected-warning {{self-comparison always results}}
+  return (x) != (((x))); // expected-warning {{self-comparison always evaluates to false}}
+}
+
+void foo3(short s, short t) { 
+  if (s == s) {} // expected-warning {{self-comparison always evaluates to true}}
+  if (s == t) {} // no-warning
+}
+
+void foo4(void* v, void* w) {
+  if (v == v) {} // expected-warning {{self-comparison always evaluates to true}}
+  if (v == w) {} // no-warning
 }
 
 int qux(int x) {
@@ -36,3 +46,34 @@ int compare_enum() {
 int compare_sizeof(int x) {
   return sizeof(x == x); // no-warning
 }
+
+int array_comparisons() {
+  int array1[2];
+  int array2[2];
+
+  //
+  // compare same array
+  //
+  return array1 == array1; // expected-warning{{self-comparison always evaluates to true}}
+  return array1 != array1; // expected-warning{{self-comparison always evaluates to false}}
+  return array1 < array1; // expected-warning{{self-comparison always evaluates to false}}
+  return array1 <= array1; // expected-warning{{self-comparison always evaluates to true}}
+  return array1 > array1; // expected-warning{{self-comparison always evaluates to false}}
+  return array1 >= array1; // expected-warning{{self-comparison always evaluates to true}}
+
+  //
+  // compare differrent arrays
+  //
+  return array1 == array2; // expected-warning{{array comparison always evaluates to false}}
+  return array1 != array2; // expected-warning{{array comparison always evaluates to true}}
+
+  //
+  // we don't know what these are going to be
+  //
+  return array1 < array2; // expected-warning{{array comparison always evaluates to a constant}}
+  return array1 <= array2; // expected-warning{{array comparison always evaluates to a constant}}
+  return array1 > array2; // expected-warning{{array comparison always evaluates to a constant}}
+  return array1 >= array2; // expected-warning{{array comparison always evaluates to a constant}}
+
+}
+

clang/test/SemaCXX/overloaded-operator.cpp

@@ -49,6 +49,13 @@ struct B {
   }
 };
 
+// we shouldn't see warnings about self-comparison,
+// this is a member function, we dunno what it'll do
+bool i(B b)
+{
+  return b == b;
+}
+
 enum Enum1 { };
 enum Enum2 { };
 

clang/test/SemaCXX/warn-self-comparisons.cpp

@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void f(int (&array1)[2], int (&array2)[2]) {
+  if (array1 == array2) { } // no warning
+}