Codesbyusman updated this revision to Diff 452797.
Codesbyusman added a comment.

updating for the missing tautological compare warnings.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130510/new/

https://reviews.llvm.org/D130510

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-unreachable.cpp

Index: clang/test/SemaCXX/warn-unreachable.cpp
===================================================================
--- clang/test/SemaCXX/warn-unreachable.cpp
+++ clang/test/SemaCXX/warn-unreachable.cpp
@@ -396,16 +396,18 @@
   if (y == -1 && y != -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}}
+    calledFun(); // expected-warning {{will never be executed}}
 
-  if (x != -1)
+  if (x != -1)   // expected-note {{silence}}
     calledFun();
   else
+    calledFun(); // expected-warning {{will never be executed}}
+    
+  // TODO: Extend warning to the following code:
+  if (x < -1)
     calledFun();
+ 
   if (-1 > x)
     calledFun();
   else
Index: clang/test/Sema/warn-bitwise-compare.c
===================================================================
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 #define mydefine 2
+#define mydefine2 -2
 
 enum {
   ZERO,
@@ -11,29 +12,64 @@
 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) {
Index: clang/lib/Analysis/CFG.cpp
===================================================================
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -964,43 +964,44 @@
     const Expr *LHSExpr = B->getLHS()->IgnoreParens();
     const Expr *RHSExpr = B->getRHS()->IgnoreParens();
 
-    const IntegerLiteral *IntLiteral = dyn_cast<IntegerLiteral>(LHSExpr);
+    Optional<llvm::APInt> IntLiteral1 =
+        getIntegerLiteralSubexpressionValue(LHSExpr);
     const Expr *BoolExpr = RHSExpr;
 
-    if (!IntLiteral) {
-      IntLiteral = dyn_cast<IntegerLiteral>(RHSExpr);
+    if (!IntLiteral1) {
+      IntLiteral1 = getIntegerLiteralSubexpressionValue(RHSExpr);
       BoolExpr = LHSExpr;
     }
 
-    if (!IntLiteral)
+    if (!IntLiteral1)
       return TryResult();
 
-    const BinaryOperator *BitOp = dyn_cast<BinaryOperator>(BoolExpr);
+    const BinaryOperator *BitOp = dyn_cast<BinaryOperator>(BoolExpr->IgnoreParens());
     if (BitOp && (BitOp->getOpcode() == BO_And ||
                   BitOp->getOpcode() == BO_Or)) {
       const Expr *LHSExpr2 = BitOp->getLHS()->IgnoreParens();
       const Expr *RHSExpr2 = BitOp->getRHS()->IgnoreParens();
 
-      const IntegerLiteral *IntLiteral2 = dyn_cast<IntegerLiteral>(LHSExpr2);
+      Optional<llvm::APInt> IntLiteral2 =
+          getIntegerLiteralSubexpressionValue(LHSExpr2);
 
       if (!IntLiteral2)
-        IntLiteral2 = dyn_cast<IntegerLiteral>(RHSExpr2);
+        IntLiteral2 = getIntegerLiteralSubexpressionValue(RHSExpr2);
 
       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)) {
+      if ((BitOp->getOpcode() == BO_And &&
+           (*IntLiteral2 & *IntLiteral1) != *IntLiteral1) ||
+          (BitOp->getOpcode() == BO_Or &&
+           (*IntLiteral2 | *IntLiteral1) != *IntLiteral1)) {
         if (BuildOpts.Observer)
           BuildOpts.Observer->compareBitwiseEquality(B,
                                                      B->getOpcode() != BO_EQ);
         TryResult(B->getOpcode() != BO_EQ);
       }
     } else if (BoolExpr->isKnownToHaveBooleanValue()) {
-      llvm::APInt IntValue = IntLiteral->getValue();
-      if ((IntValue == 1) || (IntValue == 0)) {
+      if ((*IntLiteral1 == 1) || (*IntLiteral1 == 0)) {
         return TryResult();
       }
       return TryResult(B->getOpcode() != BO_EQ);
@@ -1009,6 +1010,51 @@
     return TryResult();
   }
 
+  Optional<llvm::APInt> getIntegerLiteralSubexpressionValue(const Expr *E) {
+    // The value to return;
+    Optional<llvm::APInt> Value = llvm::None;
+
+    const IntegerLiteral *IntLiteral = nullptr;
+    const UnaryOperator *UnOp = dyn_cast<UnaryOperator>(E->IgnoreParens());
+
+    // If unary.
+    if (UnOp) {
+      // Get the sub expression of the unary expression and get the Integer
+      // Literal.
+      const Expr *SubExpr = UnOp->getSubExpr();
+      IntLiteral = dyn_cast<IntegerLiteral>(SubExpr->IgnoreParens());
+
+      if (IntLiteral) {
+
+        Value = IntLiteral->getValue();
+        // Getting the Operator Code to perform them manually.
+        const UnaryOperatorKind OpCode = UnOp->getOpcode();
+
+        // Perform the operation manually.
+        switch (OpCode) {
+        case UO_Plus:
+          return Value;
+        case UO_Minus:
+          return -*Value;
+        case UO_Not:
+          return ~*Value;
+        case UO_LNot:
+          return *Value = !*Value;
+        default:
+          return Value;
+        }
+      }
+
+    } else {
+
+      IntLiteral = dyn_cast<IntegerLiteral>(E->IgnoreParens());
+      if (IntLiteral)
+        return Value = IntLiteral->getValue();
+    }
+
+    return Value;
+  }
+
   TryResult analyzeLogicOperatorCondition(BinaryOperatorKind Relation,
                                           const llvm::APSInt &Value1,
                                           const llvm::APSInt &Value2) {
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -60,7 +60,10 @@
 - No longer assert/miscompile when trying to make a vectorized ``_BitInt`` type
   using the ``ext_vector_type`` attribute (the ``vector_size`` attribute was
   already properly diagnosing this case).
-
+- ``-Wtautological-compare`` missed warnings for tautological comparisons
+  involving a negative integer literal. This fixes
+  `Issue 42918 <https://github.com/llvm/llvm-project/issues/42918>`_.
+  
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Clang will now correctly diagnose as ill-formed a constant expression where an
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to