Revert "Missing tautological compare warnings due to unary operators"

This reverts commit 0cc3c184c784d5f0d55de8ad0a9eeee876acd149.

The changes did not account for templated code where one instantiation
may trigger the diagnostic but other instantiations will not, as in:
```
template <int I, class T>
void foo(int x) {
    bool b1 = (x & sizeof(T)) == 8;
    bool b2 = (x & I) == 8;
    bool b3 = (x & 4) == 8;
}

void run(int x) {
    foo<4, int>(8);
}
```

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f3d8200..7826593 100644 (file)
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -49,9 +49,6 @@ Major New Features
 
 Bug Fixes
 ---------
-- ``-Wtautological-compare`` missed warnings for tautological comparisons
-  involving a negative integer literal. This fixes
-  `Issue 42918 <https://github.com/llvm/llvm-project/issues/42918>`_.
 - Fixes an accepts-invalid bug in C when using a ``_Noreturn`` function
   specifier on something other than a function declaration. This fixes
   `Issue 56800 <https://github.com/llvm/llvm-project/issues/56800>`_.

diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 282e006..5c45264 100644 (file)
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -964,44 +964,43 @@ private:
     const Expr *LHSExpr = B->getLHS()->IgnoreParens();
     const Expr *RHSExpr = B->getRHS()->IgnoreParens();
 
-    const Expr *BoolExpr = nullptr; // To store the expression.
-    Expr::EvalResult IntExprResult; // If integer literal then will save value.
+    const IntegerLiteral *IntLiteral = dyn_cast<IntegerLiteral>(LHSExpr);
+    const Expr *BoolExpr = RHSExpr;
 
-    if (LHSExpr->EvaluateAsInt(IntExprResult, *Context))
-      BoolExpr = RHSExpr;
-    else if (RHSExpr->EvaluateAsInt(IntExprResult, *Context))
+    if (!IntLiteral) {
+      IntLiteral = dyn_cast<IntegerLiteral>(RHSExpr);
       BoolExpr = LHSExpr;
-    else
-      return TryResult();
+    }
 
-    llvm::APInt L1 = IntExprResult.Val.getInt();
+    if (!IntLiteral)
+      return TryResult();
 
     const BinaryOperator *BitOp = dyn_cast<BinaryOperator>(BoolExpr);
-    if (BitOp &&
-        (BitOp->getOpcode() == BO_And || BitOp->getOpcode() == BO_Or)) {
+    if (BitOp && (BitOp->getOpcode() == BO_And ||
+                  BitOp->getOpcode() == BO_Or)) {
+      const Expr *LHSExpr2 = BitOp->getLHS()->IgnoreParens();
+      const Expr *RHSExpr2 = BitOp->getRHS()->IgnoreParens();
 
-      // If integer literal in expression identified then will save value.
-      Expr::EvalResult IntExprResult2;
+      const IntegerLiteral *IntLiteral2 = dyn_cast<IntegerLiteral>(LHSExpr2);
 
-      if (BitOp->getLHS()->EvaluateAsInt(IntExprResult2, *Context))
-        ; // LHS is a constant expression.
-      else if (BitOp->getRHS()->EvaluateAsInt(IntExprResult2, *Context))
-        ; // RHS is a constant expression.
-      else
-        return TryResult(); // Neither is a constant expression, bail out.
+      if (!IntLiteral2)
+        IntLiteral2 = dyn_cast<IntegerLiteral>(RHSExpr2);
 
-      llvm::APInt L2 = IntExprResult2.Val.getInt();
+      if (!IntLiteral2)
+        return TryResult();
 
+      llvm::APInt L1 = IntLiteral->getValue();
+      llvm::APInt L2 = IntLiteral2->getValue();
       if ((BitOp->getOpcode() == BO_And && (L2 & L1) != L1) ||
-          (BitOp->getOpcode() == BO_Or && (L2 | L1) != L1)) {
+          (BitOp->getOpcode() == BO_Or  && (L2 | L1) != L1)) {
         if (BuildOpts.Observer)
           BuildOpts.Observer->compareBitwiseEquality(B,
                                                      B->getOpcode() != BO_EQ);
         TryResult(B->getOpcode() != BO_EQ);
       }
     } else if (BoolExpr->isKnownToHaveBooleanValue()) {
-      llvm::APInt IntValue = IntExprResult.Val.getInt(); // Getting the value.
-      if ((L1 == 1) || (L1 == 0)) {
+      llvm::APInt IntValue = IntLiteral->getValue();
+      if ((IntValue == 1) || (IntValue == 0)) {
         return TryResult();
       }
       return TryResult(B->getOpcode() != BO_EQ);

diff --git a/clang/test/Sema/warn-bitwise-compare.c b/clang/test/Sema/warn-bitwise-compare.c
index b96881f..08a8b08 100644 (file)
--- a/clang/test/Sema/warn-bitwise-compare.c
+++ b/clang/test/Sema/warn-bitwise-compare.c
@@ -2,7 +2,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 #define mydefine 2
-#define mydefine2 -2
 
 enum {
   ZERO,
@@ -12,67 +11,29 @@ enum {
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
-  if ((-8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
-  if ((x & -8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
-
   if ((x & 8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((2 & x) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
-  if ((x & -8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
-  if ((-2 & x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
-
   if ((x | 4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x | 3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
-
-  if ((x | -4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
-  if ((x | -3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
-  if ((-5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
-
-
   if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
-  if ((x & 0xFFEB) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
-
   if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
-  if ((0xFFDD | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
 
   if (!!((8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
-  if (!!((-8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
-
   int y = ((8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
-      y = ((-8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
-      y = ((3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
-      y = ((-3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
 
   if ((x & 8) == 8) {}
   if ((x & 8) != 8) {}
   if ((x | 4) == 4) {}
   if ((x | 4) != 4) {}
 
-  if ((-2 & x) != 4) {}
-  if ((x & -8) == -8) {}
-  if ((x & -8) != -8) {}
-  if ((x | -4) == -4) {}
-  if ((x | -4) != -4) {}
-
-
   if ((x & 9) == 8) {}
   if ((x & 9) != 8) {}
   if ((x | 4) == 5) {}
   if ((x | 4) != 5) {}
 
-  if ((x & -9) == -10) {}
-  if ((x & -9) != -10) {}
-  if ((x | -4) == -3) {}
-  if ((x | -4) != -3) {}
-
-  if ((x^0) == 0) {}
-
   if ((x & mydefine) == 8) {}
   if ((x | mydefine) == 4) {}
-
-  if ((x & mydefine2) == 8) {}
-  if ((x | mydefine2) == 4) {}
-
 }
 
 void g(int x) {

diff --git a/clang/test/SemaCXX/warn-unreachable.cpp b/clang/test/SemaCXX/warn-unreachable.cpp
index 0d9468c..c664c19 100644 (file)
--- a/clang/test/SemaCXX/warn-unreachable.cpp
+++ b/clang/test/SemaCXX/warn-unreachable.cpp
@@ -396,16 +396,15 @@ void tautological_compare(bool x, int y) {
   if (y == -1 && y != -1)  // expected-note {{silence}}
     calledFun();        // expected-warning {{will never be executed}}
 
-  if (x == -1)   // expected-note {{silence}}
-    calledFun(); // expected-warning {{will never be executed}}
+  // TODO: Extend warning to the following code:
+  if (x < -1)
+    calledFun();
+  if (x == -1)
+    calledFun();
 
-  if (x != -1)   // expected-note {{silence}}
+  if (x != -1)
     calledFun();
   else
-    calledFun(); // expected-warning {{will never be executed}}
-
-  // TODO: Extend warning to the following code:
-  if (x < -1)
     calledFun();
   if (-1 > x)
     calledFun();