r190118 - Add self-comparison warnings for fields.

[Eli Friedman]

Author: efriedma
Date: Thu Sep  5 22:13:09 2013
New Revision: 190118

URL: http://llvm.org/viewvc/llvm-project?rev=190118&view=rev
Log:
Add self-comparison warnings for fields.

This expands very slightly what -Wtautological-compare considers to be
tautological to include implicit accesses to C++ fields and ObjC ivars.
I don't want to turn this into a full expression-identity check, but
these additions seem pretty well-contained, and maintain the theme
of checking for "x == x".

<rdar://problem/14431127>

Added:
    cfe/trunk/test/SemaCXX/self-comparison.cpp
    cfe/trunk/test/SemaObjC/self-comparison.m
Modified:
    cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=190118&r1=190117&r2=190118&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Sep  5 22:13:09 2013
@@ -7472,6 +7472,22 @@ static void diagnoseLogicalNotOnLHSofCom
       << FixItHint::CreateInsertion(SecondClose, ")");
 }
 
+// Get the decl for a simple expression: a reference to a variable,
+// an implicit C++ field reference, or an implicit ObjC ivar reference.
+static ValueDecl *getCompareDecl(Expr *E) {
+  if (DeclRefExpr* DR = dyn_cast<DeclRefExpr>(E))
+    return DR->getDecl();
+  if (ObjCIvarRefExpr* Ivar = dyn_cast<ObjCIvarRefExpr>(E)) {
+    if (Ivar->isFreeIvar())
+      return Ivar->getDecl();
+  }
+  if (MemberExpr* Mem = dyn_cast<MemberExpr>(E)) {
+    if (Mem->isImplicitAccess())
+      return Mem->getMemberDecl();
+  }
+  return 0;
+}
+
 // C99 6.5.8, C++ [expr.rel]
 QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
                                     SourceLocation Loc, unsigned OpaqueOpc,
@@ -7508,37 +7524,34 @@ QualType Sema::CheckCompareOperands(Expr
     // obvious cases in the definition of the template anyways. The idea is to
     // warn when the typed comparison operator will always evaluate to the same
     // result.
-    if (DeclRefExpr* DRL = dyn_cast<DeclRefExpr>(LHSStripped)) {
-      if (DeclRefExpr* DRR = dyn_cast<DeclRefExpr>(RHSStripped)) {
-        if (DRL->getDecl() == DRR->getDecl() &&
-            !IsWithinTemplateSpecialization(DRL->getDecl())) {
-          DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always)
-                              << 0 // self-
-                              << (Opc == BO_EQ
-                                  || Opc == BO_LE
-                                  || Opc == BO_GE));
-        } else if (LHSType->isArrayType() && RHSType->isArrayType() &&
-                   !DRL->getDecl()->getType()->isReferenceType() &&
-                   !DRR->getDecl()->getType()->isReferenceType()) {
-            // what is it always going to eval to?
-            char always_evals_to;
-            switch(Opc) {
-            case BO_EQ: // e.g. array1 == array2
-              always_evals_to = 0; // false
-              break;
-            case BO_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, 0, PDiag(diag::warn_comparison_always)
-                                << 1 // array
-                                << always_evals_to);
+    ValueDecl *DL = getCompareDecl(LHSStripped);
+    ValueDecl *DR = getCompareDecl(RHSStripped);
+    if (DL && DR && DL == DR && !IsWithinTemplateSpecialization(DL)) {
+      DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always)
+                          << 0 // self-
+                          << (Opc == BO_EQ
+                              || Opc == BO_LE
+                              || Opc == BO_GE));
+    } else if (DL && DR && LHSType->isArrayType() && RHSType->isArrayType() &&
+               !DL->getType()->isReferenceType() &&
+               !DR->getType()->isReferenceType()) {
+        // what is it always going to eval to?
+        char always_evals_to;
+        switch(Opc) {
+        case BO_EQ: // e.g. array1 == array2
+          always_evals_to = 0; // false
+          break;
+        case BO_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, 0, PDiag(diag::warn_comparison_always)
+                            << 1 // array
+                            << always_evals_to);
     }
 
     if (isa<CastExpr>(LHSStripped))

Added: cfe/trunk/test/SemaCXX/self-comparison.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/self-comparison.cpp?rev=190118&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/self-comparison.cpp (added)
+++ cfe/trunk/test/SemaCXX/self-comparison.cpp Thu Sep  5 22:13:09 2013
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int foo(int x) {
+  return x == x; // expected-warning {{self-comparison always evaluates to true}}
+}
+
+struct X {
+  bool operator==(const X &x);
+};
+
+struct A {
+  int x;
+  X x2;
+  int a[3];
+  int b[3];
+  bool f() { return x == x; } // expected-warning {{self-comparison always evaluates to true}}
+  bool g() { return x2 == x2; } // no-warning
+  bool h() { return a == b; } // expected-warning {{array comparison always evaluates to false}}
+  bool i() {
+    int c[3];
+    return a == c; // expected-warning {{array comparison always evaluates to false}}
+  }
+};

Added: cfe/trunk/test/SemaObjC/self-comparison.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/self-comparison.m?rev=190118&view=auto
==============================================================================
--- cfe/trunk/test/SemaObjC/self-comparison.m (added)
+++ cfe/trunk/test/SemaObjC/self-comparison.m Thu Sep  5 22:13:09 2013
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+
+ at interface A {
+  id xxx;
+}
+-(int)bar;
+ at end
+ at implementation A
+-(int)bar {
+  return xxx == xxx; // expected-warning {{self-comparison always evaluates to true}}
+}
+ at end