Improve detection of same value in comparisons

[rtrieu]

Summary

Self comparison under -Wtautological-compare and -Wtautological-overlap-compare work by detecting that two operands refer to the same value. The easy cases of operands referring to the same Decl are already handled. This change extends the detection to look through variable members, as well as detecting static member access via different routes. Also unify the detection method to one location as previously, each warning had its own.

struct S {

int x;
static int y;

};

bool foo(S s) {

return s.x == 1 && s.x == 2;  // detects this is always false
return S::y == s.y;  // detects this is always true

}

[NoQ]

I like where this is going! I'll add myself so that not to forget to review the CFG bits next week.

[jfb]

Does this work for unions as well?

test/Analysis/array-struct-region.cpp

38

Is this the only way? Or do casts also disable the diagnostic?

test/SemaCXX/self-comparison.cpp

78

s1.array[0] == s1.array[0]?

87

The test only has ==. Do other operators trigger?

[rtrieu]

In D66045#1624389, @jfb wrote:

Does this work for unions as well?

This will work for union accesses via the same member name, but not different member names.

union U { int x; int y; } u;
bool b1 = u.x == u.x;  // warn always true
bool b2 = u.x == u.y;  // no warning

It's easy to do when the types are the same, but more difficult when the types are different, so keeping with the same name member access is the safe way for now.

test/Analysis/array-struct-region.cpp

38

There's other ways to do this. I picked "+ 0" here. Explicit casts, like to void* or const S* would also disable it. I think pragmas would also work. I don't particularly care which way since this is an analyzer test.

test/SemaCXX/self-comparison.cpp

78

No, array accesses aren't checked yet. But it's a good idea to look into it.

87

All the standard comparison operators will work here. I'll add tests.

[NoQ]

lib/AST/Expr.cpp

3931–3932

Does this actually happen *after* instantiation? If not - do we even need to emit warnings on templates that aren't instantiated? I guess we still need this branch because isSameComparisonOperand() lives in the Expr class and should always work regardless, but in this case i guess this deserves a unittest(?)

3976

Mmm, what are other cases when a MemberExpr doesn't have a FieldDecl as its getMemberDecl()? Can this be turned into an assert?

test/Analysis/array-struct-region.cpp

38

I'd rather disable the newly introduced warning on this test file, because this is a path-sensitive static analysis test and this change may accidentally ruin the original test.

[rtrieu]

lib/AST/Expr.cpp

3931–3932

This happens before instantiation. The test cases are in (test/SemaTemplate/self-comparison.cpp). The basic idea is to allow integral template parameters to be treated as variables.

template<int A>
void foo(int x) {
  bool b1 = A == A;
  bool b2 = x == x;
}

If we treat template parameter A the same as variable x, we can diagnose a few more tautological types, even without instantiation.

3976

I had to look this one up too. From the docs for this method:

/// Retrieve the member declaration to which this expression refers.
///
/// The returned declaration will be a FieldDecl or (in C++) a VarDecl (for
/// static data members), a CXXMethodDecl, or an EnumConstantDecl.
ValueDecl *getMemberDecl() const { return MemberDecl; }

So, FieldDecl, VarDecl, CXXMethodDecl, and EnumConstantDecl are all valid return types.

[rtrieu]

Check array accesses.

[rtrieu]

test/Analysis/array-struct-region.cpp

38

Makes sense. The warning is now disabled via -Wno flags on the RUN lines.

test/SemaCXX/self-comparison.cpp

78

A little recursive checking and now we check array accesses.

87

All operator tests added.

[jfb]

test/SemaCXX/self-comparison.cpp

87

operator<=>?

Is there a separate test for user-defined operators as well? What makes sense in these cases? Technically a user-defined comparison could do anything... but I think this warning makes sense for them as well.

[rtrieu]

test/SemaCXX/self-comparison.cpp

87

operator<=> test was added to test/SemaCXX/compare-cxx2a.cpp because the operator requires header that this test doesn't include.

The warning currently doesn't check user-defined operators and I've include a test below to show that. If a warning for user-defined operators was created, it would not belong in -Wtautological-compare because we would not know the actual result. The best we could do is to say there was a self compare with a user-defined operator. It would also need to hook into the AST at a different point. Builtin compare operators are checked in BinaryOperator nodes. User-defined operators would need to be checked at CallExpr or CXXOperatorCallExpr instead. This is different enough that it should be in a different patch.

[jfb]

test/SemaCXX/self-comparison.cpp

87

Thanks, this looks great!

[NoQ]

Thanks from me as well.

lib/AST/Expr.cpp

4009

const auto *?

include/clang/AST/Expr.h

const Expr *skipRValueSubobjectAdjustments(
       SmallVectorImpl<const Expr *> &CommaLHS,
       SmallVectorImpl<SubobjectAdjustment> &Adjustments) const;
   const Expr *skipRValueSubobjectAdjustments() const {
     SmallVector<const Expr *, 8> CommaLHSs;
     SmallVector<SubobjectAdjustment, 8> Adjustments;
     return skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
   }
 
+  /// Checks that the two Expr's will refer to the same value as a comparison
+  /// operand.  The caller must ensure that the values referenced by the Expr's
+  /// are not modified between E1 and E2 or the result my be invalid.
+  static bool isSameComparisonOperand(const Expr* E1, const Expr* E2);
+
   static bool classof(const Stmt *T) {
     return T->getStmtClass() >= firstExprConstant &&
            T->getStmtClass() <= lastExprConstant;
   }
 };
 
 //===----------------------------------------------------------------------===//
 // Wrapper Expressions.

lib/AST/Expr.cpp

if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E))
     if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
       if (VD->getStorageClass() == SC_Register &&
           VD->hasAttr<AsmLabelAttr>() && !VD->isLocalVarDecl())
         return true;
 
   return false;
 }
 
+bool Expr::isSameComparisonOperand(const Expr* E1, const Expr* E2) {
+  E1 = E1->IgnoreParens();
+  E2 = E2->IgnoreParens();
+
+  if (E1->getStmtClass() != E2->getStmtClass())
+    return false;
+
+  switch (E1->getStmtClass()) {
+    default:
+      return false;
+    case CXXThisExprClass:
+      return true;
+    case DeclRefExprClass: {
+      // DeclRefExpr without an ImplicitCastExpr can happen for integral
+      // template parameters.
+      const auto *DRE1 = cast<DeclRefExpr>(E1);
+      const auto *DRE2 = cast<DeclRefExpr>(E2);
+      return DRE1->isRValue() && DRE2->isRValue() &&
+             DRE1->getDecl() == DRE2->getDecl();
+    }
+    case ImplicitCastExprClass: {
+      // Peel off implicit casts.
+      while (true) {
+        const auto *ICE1 = dyn_cast<ImplicitCastExpr>(E1);
+        const auto *ICE2 = dyn_cast<ImplicitCastExpr>(E2);
+        if (!ICE1 || !ICE2)
+          return false;
+        if (ICE1->getCastKind() != ICE2->getCastKind())
+          return false;
+        E1 = ICE1->getSubExpr()->IgnoreParens();
+        E2 = ICE2->getSubExpr()->IgnoreParens();
+        // The final cast must be one of these types.
+        if (ICE1->getCastKind() == CK_LValueToRValue ||
+            ICE1->getCastKind() == CK_ArrayToPointerDecay ||
+            ICE1->getCastKind() == CK_FunctionToPointerDecay) {
+          break;
+        }
+      }
+
+      const auto *DRE1 = dyn_cast<DeclRefExpr>(E1);
+      const auto *DRE2 = dyn_cast<DeclRefExpr>(E2);
+      if (DRE1 && DRE2)
+        return declaresSameEntity(DRE1->getDecl(), DRE2->getDecl());
+
+      const auto *Ivar1 = dyn_cast<ObjCIvarRefExpr>(E1);
+      const auto *Ivar2 = dyn_cast<ObjCIvarRefExpr>(E2);
+      if (Ivar1 && Ivar2) {
+        return Ivar1->isFreeIvar() && Ivar2->isFreeIvar() &&
+               declaresSameEntity(Ivar1->getDecl(), Ivar2->getDecl());
+      }
+
+      const auto *Array1 = dyn_cast<ArraySubscriptExpr>(E1);
+      const auto *Array2 = dyn_cast<ArraySubscriptExpr>(E2);
+      if (Array1 && Array2) {
+        if (!isSameComparisonOperand(Array1->getBase(), Array2->getBase()))
+          return false;
+
+        auto Idx1 = Array1->getIdx();
+        auto Idx2 = Array2->getIdx();
+        const auto Integer1 = dyn_cast<IntegerLiteral>(Idx1);
+        const auto Integer2 = dyn_cast<IntegerLiteral>(Idx2);
+        if (Integer1 && Integer2) {
+          if (Integer1->getValue() != Integer2->getValue())
+            return false;
+        } else {
+          if (!isSameComparisonOperand(Idx1, Idx2))
+            return false;
+        }
+
+        return true;
+      }
+
+      // Walk the MemberExpr chain.
+      while (isa<MemberExpr>(E1) && isa<MemberExpr>(E2)) {
+        const auto *ME1 = cast<MemberExpr>(E1);
+        const auto *ME2 = cast<MemberExpr>(E2);
+        if (!declaresSameEntity(ME1->getMemberDecl(), ME2->getMemberDecl()))
+          return false;
+        if (const auto *D = dyn_cast<VarDecl>(ME1->getMemberDecl()))
+          if (D->isStaticDataMember())
+            return true;
+        E1 = ME1->getBase()->IgnoreParenImpCasts();
+        E2 = ME2->getBase()->IgnoreParenImpCasts();
+      }
+
+      if (isa<CXXThisExpr>(E1) && isa<CXXThisExpr>(E2))
+        return true;
+
+      // A static member variable can end the MemberExpr chain with either
+      // a MemberExpr or a DeclRefExpr.
+      auto getAnyDecl = [](const Expr *E) -> const ValueDecl * {
+        if (auto *DRE = dyn_cast<DeclRefExpr>(E))
+          return DRE->getDecl();
+        if (auto *ME = dyn_cast<MemberExpr>(E))
+          return ME->getMemberDecl();
+        return nullptr;
+      };
+
+      const ValueDecl *VD1 = getAnyDecl(E1);
+      const ValueDecl *VD2 = getAnyDecl(E2);
+      return declaresSameEntity(VD1, VD2);
+    }
+  }
+}
+
 /// isArrow - Return true if the base expression is a pointer to vector,
 /// return false if the base expression is a vector.
 bool ExtVectorElementExpr::isArrow() const {
   return getBase()->getType()->isPointerType();
 }
 
 unsigned ExtVectorElementExpr::getNumElements() const {
   if (const VectorType *VT = getType()->getAs<VectorType>())

lib/Analysis/CFG.cpp

static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) {
   E = E->IgnoreParens();
   if (isa<IntegerLiteral>(E))
     return E;
   if (auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts()))
     return isa<EnumConstantDecl>(DR->getDecl()) ? DR : nullptr;
   return nullptr;
 }
 
-/// Tries to interpret a binary operator into `Decl Op Expr` form, if Expr is
-/// an integer literal or an enum constant.
+/// Tries to interpret a binary operator into `Expr Op NumExpr` form, if
+/// NumExpr is an integer literal or an enum constant.
 ///
 /// If this fails, at least one of the returned DeclRefExpr or Expr will be
 /// null.
-static std::tuple<const DeclRefExpr *, BinaryOperatorKind, const Expr *>
+static std::tuple<const Expr *, BinaryOperatorKind, const Expr *>
 tryNormalizeBinaryOperator(const BinaryOperator *B) {
   BinaryOperatorKind Op = B->getOpcode();
 
   const Expr *MaybeDecl = B->getLHS();
   const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS());
   // Expr looked like `0 == Foo` instead of `Foo == 0`
   if (Constant == nullptr) {
     // Flip the operator
     if (Op == BO_GT)
       Op = BO_LT;
     else if (Op == BO_GE)
       Op = BO_LE;
     else if (Op == BO_LT)
       Op = BO_GT;
     else if (Op == BO_LE)
       Op = BO_GE;
 
     MaybeDecl = B->getRHS();
     Constant = tryTransformToIntOrEnumConstant(B->getLHS());
   }
 
-  auto *D = dyn_cast<DeclRefExpr>(MaybeDecl->IgnoreParenImpCasts());
-  return std::make_tuple(D, Op, Constant);
+  return std::make_tuple(MaybeDecl, Op, Constant);
 }
 
 /// For an expression `x == Foo && x == Bar`, this determines whether the
 /// `Foo` and `Bar` are either of the same enumeration type, or both integer
 /// literals.
 ///
 /// It's an error to pass this arguments that are not either IntegerLiterals
 /// or DeclRefExprs (that have decls of type EnumConstantDecl)
TryResult checkIncorrectLogicOperator(const BinaryOperator *B) {
     const BinaryOperator *RHS =
         dyn_cast<BinaryOperator>(B->getRHS()->IgnoreParens());
     if (!LHS || !RHS)
       return {};
 
     if (!LHS->isComparisonOp() || !RHS->isComparisonOp())
       return {};
 
-    const DeclRefExpr *Decl1;
-    const Expr *Expr1;
+    const Expr *DeclExpr1;
+    const Expr *NumExpr1;
     BinaryOperatorKind BO1;
-    std::tie(Decl1, BO1, Expr1) = tryNormalizeBinaryOperator(LHS);
+    std::tie(DeclExpr1, BO1, NumExpr1) = tryNormalizeBinaryOperator(LHS);
 
-    if (!Decl1 || !Expr1)
+    if (!DeclExpr1 || !NumExpr1)
       return {};
 
-    const DeclRefExpr *Decl2;
-    const Expr *Expr2;
+    const Expr *DeclExpr2;
+    const Expr *NumExpr2;
     BinaryOperatorKind BO2;
-    std::tie(Decl2, BO2, Expr2) = tryNormalizeBinaryOperator(RHS);
+    std::tie(DeclExpr2, BO2, NumExpr2) = tryNormalizeBinaryOperator(RHS);
 
-    if (!Decl2 || !Expr2)
+    if (!DeclExpr2 || !NumExpr2)
       return {};
 
     // Check that it is the same variable on both sides.
-    if (Decl1->getDecl() != Decl2->getDecl())
+    if (!Expr::isSameComparisonOperand(DeclExpr1, DeclExpr2))
       return {};
 
     // Make sure the user's intent is clear (e.g. they're comparing against two
     // int literals, or two things from the same enum)
-    if (!areExprTypesCompatible(Expr1, Expr2))
+    if (!areExprTypesCompatible(NumExpr1, NumExpr2))
       return {};
 
     Expr::EvalResult L1Result, L2Result;
-    if (!Expr1->EvaluateAsInt(L1Result, *Context) ||
-        !Expr2->EvaluateAsInt(L2Result, *Context))
+    if (!NumExpr1->EvaluateAsInt(L1Result, *Context) ||
+        !NumExpr2->EvaluateAsInt(L2Result, *Context))
       return {};
 
     llvm::APSInt L1 = L1Result.Val.getInt();
     llvm::APSInt L2 = L2Result.Val.getInt();
 
     // Can't compare signed with unsigned or with different bit width.
     if (L1.isSigned() != L2.isSigned() || L1.getBitWidth() != L2.getBitWidth())
       return {};

lib/Sema/SemaExpr.cpp

static void diagnoseLogicalNotOnLHSofCheck(Sema &S, ExprResult &LHS,
   SecondClose = S.getLocForEndOfToken(SecondClose);
   if (SecondClose.isInvalid())
     SecondOpen = SourceLocation();
   S.Diag(UO->getOperatorLoc(), diag::note_logical_not_silence_with_parens)
       << FixItHint::CreateInsertion(SecondOpen, "(")
       << 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)) {
+// Returns true if E refers to a non-weak array.
+static bool checkForArray(const Expr *E) {
+  const ValueDecl *D = nullptr;
+  if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E)) {
+    D = DR->getDecl();
+  } else if (const MemberExpr *Mem = dyn_cast<MemberExpr>(E)) {
     if (Mem->isImplicitAccess())
-      return Mem->getMemberDecl();
+      D = Mem->getMemberDecl();
   }
-  return nullptr;
+  if (!D)
+    return false;
+  return D->getType()->isArrayType() && !D->isWeak();
 }
 
 /// Diagnose some forms of syntactically-obvious tautological comparison.
 static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc,
                                            Expr *LHS, Expr *RHS,
                                            BinaryOperatorKind Opc) {
   Expr *LHSStripped = LHS->IgnoreParenImpCasts();
   Expr *RHSStripped = RHS->IgnoreParenImpCasts();
static void diagnoseTautologicalComparison(Sema &S, SourceLocation Loc,
   // often indicate logic errors in the program.
   //
   // NOTE: Don't warn about comparison expressions resulting from macro
   // expansion. Also don't warn about comparisons which are only self
   // comparisons within a template instantiation. The warnings should catch
   // 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.
-  ValueDecl *DL = getCompareDecl(LHSStripped);
-  ValueDecl *DR = getCompareDecl(RHSStripped);
 
   // Used for indexing into %select in warn_comparison_always
   enum {
     AlwaysConstant,
     AlwaysTrue,
     AlwaysFalse,
     AlwaysEqual, // std::strong_ordering::equal from operator<=>
   };
-  if (DL && DR && declaresSameEntity(DL, DR)) {
+
+  if (Expr::isSameComparisonOperand(LHS, RHS)) {
     unsigned Result;
     switch (Opc) {
     case BO_EQ: case BO_LE: case BO_GE:
       Result = AlwaysTrue;
       break;
     case BO_NE: case BO_LT: case BO_GT:
       Result = AlwaysFalse;
       break;
     case BO_Cmp:
       Result = AlwaysEqual;
       break;
     default:
       Result = AlwaysConstant;
       break;
     }
     S.DiagRuntimeBehavior(Loc, nullptr,
                           S.PDiag(diag::warn_comparison_always)
                               << 0 /*self-comparison*/
                               << Result);
-  } else if (DL && DR &&
-             DL->getType()->isArrayType() && DR->getType()->isArrayType() &&
-             !DL->isWeak() && !DR->isWeak()) {
+  } else if (checkForArray(LHSStripped) && checkForArray(RHSStripped)) {
     // What is it always going to evaluate to?
     unsigned Result;
     switch(Opc) {
     case BO_EQ: // e.g. array1 == array2
       Result = AlwaysFalse;
       break;
     case BO_NE: // e.g. array1 != array2
       Result = AlwaysTrue;

test/Analysis/array-struct-region.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
 // RUN:                    -analyzer-checker=debug.ExprInspection -verify\
+// RUN:                    -Wno-tautological-compare\
 // RUN:                    -x c %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
 // RUN:                    -analyzer-checker=debug.ExprInspection -verify\
+// RUN:                    -Wno-tautological-compare\
 // RUN:                    -x c++ -std=c++14 %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
 // RUN:                    -analyzer-checker=debug.ExprInspection -verify\
+// RUN:                    -Wno-tautological-compare\
 // RUN:                    -x c++ -std=c++17 %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
 // RUN:                    -analyzer-checker=debug.ExprInspection -verify\
+// RUN:                    -Wno-tautological-compare\
 // RUN:                    -DINLINE -x c %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
 // RUN:                    -analyzer-checker=debug.ExprInspection -verify\
+// RUN:                    -Wno-tautological-compare\
 // RUN:                    -DINLINE -x c++ -std=c++14 %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
 // RUN:                    -analyzer-checker=debug.ExprInspection -verify\
+// RUN:                    -Wno-tautological-compare\
 // RUN:                    -DINLINE -x c++ -std=c++17 %s
 
 void clang_analyzer_eval(int);
 
 struct S {
   int field;
 
 #if __cplusplus
   const struct S *getThis() const { return this; }
   const struct S *operator +() const { return this; }
 
   bool check() const { return this == this; }
   bool operator !() const { return this != this; }
 
   int operator *() const { return field; }
 #endif
 };
 
 #if __cplusplus
 const struct S *operator -(const struct S &s) { return &s; }
 bool operator ~(const struct S &s) { return (&s) != &s; }
 #endif
 

test/Sema/warn-overlap.c

int returns(int x) {
   return x != 1 || x != 3;
   // expected-warning@-1{{overlapping comparisons always evaluate to true}}
   return x == 1 && x == 2;
   // expected-warning@-1{{overlapping comparisons always evaluate to false}}
 
   return x < 1 || x != 0;
   // expected-warning@-1{{overlapping comparisons always evaluate to true}}
 }
+
+struct A {
+  int x;
+  int y;
+};
+
+int struct_test(struct A a) {
+  return a.x > 5 && a.y < 1;  // no warning, different variables
+
+  return a.x > 5 && a.x < 1;
+  // expected-warning@-1{{overlapping comparisons always evaluate to false}}
+  return a.y == 1 || a.y != 1;
+  // expected-warning@-1{{overlapping comparisons always evaluate to true}}
+}

test/SemaCXX/compare-cxx2a.cpp

 // Force x86-64 because some of our heuristics are actually based
 // on integer sizes.
 
 // RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -fsyntax-only -pedantic -verify -Wsign-compare -std=c++2a %s
 
 #include "Inputs/std-compare.h"
 
 #define ASSERT_TYPE(...) static_assert(__is_same(__VA_ARGS__))
 #define ASSERT_EXPR_TYPE(Expr, Expect) static_assert(__is_same(decltype(Expr), Expect));
 
+struct S {
+  static int x[5];
+};
+
 void self_compare() {
   int a;
   int *b = nullptr;
+  S s;
 
   (void)(a <=> a); // expected-warning {{self-comparison always evaluates to 'std::strong_ordering::equal'}}
   (void)(b <=> b); // expected-warning {{self-comparison always evaluates to 'std::strong_ordering::equal'}}
+  (void)(s.x[a] <=> S::x[a]); // expected-warning {{self-comparison always evaluates to 'std::strong_ordering::equal'}}
 }
 
 void test0(long a, unsigned long b) {
   enum EnumA : int {A};
   enum EnumB {B};
   enum EnumC {C = 0x10000};
 
   (void)((short)a <=> (unsigned short)b);

test/SemaCXX/self-comparison.cpp

 template<typename T, typename U>
 bool g() {
   // FIXME: Ideally we'd produce a self-comparison warning on the first of these.
   return
     Y<T>::n == Y<T>::n ||
     Y<T>::n == Y<U>::n;
 }
 template bool g<int, int>(); // should not produce any warnings
+
+namespace member_tests {
+struct B {
+  int field;
+  static int static_field;
+  int test(B b) {
+    return field == field;  // expected-warning {{self-comparison always evaluates to true}}
+    return static_field == static_field;  // expected-warning {{self-comparison always evaluates to true}}
+    return static_field == b.static_field;  // expected-warning {{self-comparison always evaluates to true}}
+    return B::static_field == this->static_field;  // expected-warning {{self-comparison always evaluates to true}}
+    return this == this;  // expected-warning {{self-comparison always evaluates to true}}
+
+    return field == b.field;
+    return this->field == b.field;
+  }
+};
+
+enum {
+  I0,
+  I1,
+  I2,
+};
+
+struct S {
+  int field;
+  static int static_field;
+  int array[4];
+};
+
+struct T {
+  int field;
+  static int static_field;
+  int array[4];
+  S s;
+};
+
+int struct_test(S s1, S s2, S *s3, T t) {
+  return s1.field == s1.field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s2.field == s2.field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.static_field == s2.static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return S::static_field == s1.static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array == s1.array;  // expected-warning {{self-comparison always evaluates to true}}
+  return t.s.static_field == S::static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s3->field == s3->field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s3->static_field == S::static_field;  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array[0] == s1.array[0];  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array[I1] == s1.array[I1];  // expected-warning {{self-comparison always evaluates to true}}
+  return s1.array[s2.array[0]] == s1.array[s2.array[0]];  // expected-warning {{self-comparison always evaluates to true}}
+  return s3->array[t.field] == s3->array[t.field];  // expected-warning {{self-comparison always evaluates to true}}
+
+  // Try all operators
+  return t.field == t.field;  // expected-warning {{self-comparison always evaluates to true}}
+  return t.field <= t.field;  // expected-warning {{self-comparison always evaluates to true}}
+  return t.field >= t.field;  // expected-warning {{self-comparison always evaluates to true}}
+
+  return t.field != t.field;  // expected-warning {{self-comparison always evaluates to false}}
+  return t.field < t.field;  // expected-warning {{self-comparison always evaluates to false}}
+  return t.field > t.field;  // expected-warning {{self-comparison always evaluates to false}}
+
+  // no warning
+  return s1.field == s2.field;
+  return s2.array == s1.array;
+  return s2.array[0] == s1.array[0];
+  return s1.array[I1] == s1.array[I2];
+
+  return s1.static_field == t.static_field;
+};
+
+struct U {
+  bool operator!=(const U&);
+};
+
+bool operator==(const U&, const U&);
+
+// May want to warn on this in the future.
+int user_defined(U u) {
+  return u == u;
+  return u != u;
+}
+
+} // namespace member_tests