[clang] c783ca0 - Revert "Missing tautological compare warnings due to unary operators"
Aaron Ballman via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 2 06:41:23 PDT 2022
Author: Aaron Ballman
Date: 2022-08-02T09:39:36-04:00
New Revision: c783ca0de1e1e00f364cf4745b8444a020ddd29b
URL: https://github.com/llvm/llvm-project/commit/c783ca0de1e1e00f364cf4745b8444a020ddd29b
DIFF: https://github.com/llvm/llvm-project/commit/c783ca0de1e1e00f364cf4745b8444a020ddd29b.diff
LOG: 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);
}
```
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Analysis/CFG.cpp
clang/test/Sema/warn-bitwise-compare.c
clang/test/SemaCXX/warn-unreachable.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f3d8200c0d1a8..782659346e52e 100644
--- 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 282e0066642b6..5c45264896027 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -964,44 +964,43 @@ class CFGBuilder {
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 b96881fe296e2..08a8b084fe795 100644
--- 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 0d9468c514835..c664c1912899d 100644
--- 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();
More information about the cfe-commits
mailing list