sberg created this revision.
sberg added reviewers: jordan_rose, rtrieu.
Herald added a project: clang.
sberg requested review of this revision.
...to C++ constant expression operands.
(I had been puzzled why Clang had not found <https://git.libreoffice.org/core/+/
55f3a4595891a8cc22272225d1c82419f28d4ef9%5E!/> "cid#1465512 Wrong operator used"
in the LibreOffice source code, only found by Coverity Scan. At least building
LibreOffice with this change caused no false positives.)
In C, values of const-qualified variables are never constant expressions, so
nothing should change for C code.
To avoid potential further false positives, restrict this change only to the
"bitwise or with non-zero value" warnings while keeping all other
-Wtautological-bitwise-compare warnings as-is, at least for now.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D85287
Files:
clang/lib/Analysis/CFG.cpp
clang/test/Sema/warn-bitwise-compare.c
clang/test/SemaCXX/warn-bitwise-compare.cpp
Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===================================================================
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -1,6 +1,12 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s
// RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
+int f() { return 3; }
+
+const int const1 = 1;
+constexpr int const2 = 2;
+const int const3 = f();
+
void test(int x) {
bool b1 = (8 & x) == 3;
// expected-warning@-1 {{bitwise comparison always evaluates to false}}
@@ -10,4 +16,15 @@
// expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
bool b4 = !!(x | 5);
// expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+ bool b5 = (x | const1);
+ // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+ bool b6 = (x | const2);
+ // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
+ bool b7 = (x | const3);
+
+ // No warnings at least for now:
+ bool b8 = (x & const1) == 8;
+ bool b9 = (x | const1) == 4;
+ bool b10 = (x & const2) == 8;
+ bool b11 = (x | const2) == 4;
}
Index: clang/test/Sema/warn-bitwise-compare.c
===================================================================
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -8,6 +8,8 @@
ONE,
};
+const int const1 = 1;
+
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}}
@@ -34,6 +36,9 @@
if ((x & mydefine) == 8) {}
if ((x | mydefine) == 4) {}
+
+ if ((x & const1) == 8) {}
+ if ((x | const1) == 4) {}
}
void g(int x) {
@@ -45,4 +50,6 @@
if (x | ONE) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}}
if (x | ZERO) {}
+
+ if (x | const1) {}
}
Index: clang/lib/Analysis/CFG.cpp
===================================================================
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -93,15 +93,22 @@
return isa<IntegerLiteral>(E);
}
-/// Helper for tryNormalizeBinaryOperator. Attempts to extract an IntegerLiteral
-/// constant expression or EnumConstantDecl from the given Expr. If it fails,
+/// Helper for tryNormalizeBinaryOperator. Attempts to extract a suitable
+/// integer or enum constant expression from the given Expr. If it fails,
/// returns nullptr.
-static const Expr *tryTransformToIntOrEnumConstant(const Expr *E) {
+static const Expr *tryTransformToIntOrEnumConstant(const Expr *E,
+ ASTContext &Ctx) {
E = E->IgnoreParens();
if (IsIntegerLiteralConstantExpr(E))
return E;
- if (auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts()))
- return isa<EnumConstantDecl>(DR->getDecl()) ? DR : nullptr;
+ if (auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts())) {
+ auto *D = DR->getDecl();
+ if (isa<EnumConstantDecl>(D))
+ return DR;
+ if (auto *VD = dyn_cast<VarDecl>(D))
+ if (VD->isUsableInConstantExpressions(Ctx))
+ return DR;
+ }
return nullptr;
}
@@ -111,11 +118,11 @@
/// If this fails, at least one of the returned DeclRefExpr or Expr will be
/// null.
static std::tuple<const Expr *, BinaryOperatorKind, const Expr *>
-tryNormalizeBinaryOperator(const BinaryOperator *B) {
+tryNormalizeBinaryOperator(const BinaryOperator *B, ASTContext &Ctx) {
BinaryOperatorKind Op = B->getOpcode();
const Expr *MaybeDecl = B->getLHS();
- const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS());
+ const Expr *Constant = tryTransformToIntOrEnumConstant(B->getRHS(), Ctx);
// Expr looked like `0 == Foo` instead of `Foo == 0`
if (Constant == nullptr) {
// Flip the operator
@@ -129,7 +136,7 @@
Op = BO_GE;
MaybeDecl = B->getRHS();
- Constant = tryTransformToIntOrEnumConstant(B->getLHS());
+ Constant = tryTransformToIntOrEnumConstant(B->getLHS(), Ctx);
}
return std::make_tuple(MaybeDecl, Op, Constant);
@@ -140,7 +147,7 @@
/// literals.
///
/// It's an error to pass this arguments that are not either IntegerLiterals
-/// or DeclRefExprs (that have decls of type EnumConstantDecl)
+/// or DeclRefExprs
static bool areExprTypesCompatible(const Expr *E1, const Expr *E2) {
// User intent isn't clear if they're mixing int literals with enum
// constants.
@@ -151,18 +158,21 @@
if (!isa<DeclRefExpr>(E1))
return true;
- // IntegerLiterals are handled above and only EnumConstantDecls are expected
+ // IntegerLiterals are handled above and only certain Decls are expected
// beyond this point
assert(isa<DeclRefExpr>(E1) && isa<DeclRefExpr>(E2));
auto *Decl1 = cast<DeclRefExpr>(E1)->getDecl();
auto *Decl2 = cast<DeclRefExpr>(E2)->getDecl();
- assert(isa<EnumConstantDecl>(Decl1) && isa<EnumConstantDecl>(Decl2));
- const DeclContext *DC1 = Decl1->getDeclContext();
- const DeclContext *DC2 = Decl2->getDeclContext();
+ if (isa<EnumConstantDecl>(Decl1) && isa<EnumConstantDecl>(Decl2)) {
+ const DeclContext *DC1 = Decl1->getDeclContext();
+ const DeclContext *DC2 = Decl2->getDeclContext();
+
+ assert(isa<EnumDecl>(DC1) && isa<EnumDecl>(DC2));
+ return DC1 == DC2;
+ }
- assert(isa<EnumDecl>(DC1) && isa<EnumDecl>(DC2));
- return DC1 == DC2;
+ return false;
}
namespace {
@@ -1046,7 +1056,8 @@
const Expr *DeclExpr1;
const Expr *NumExpr1;
BinaryOperatorKind BO1;
- std::tie(DeclExpr1, BO1, NumExpr1) = tryNormalizeBinaryOperator(LHS);
+ std::tie(DeclExpr1, BO1, NumExpr1) =
+ tryNormalizeBinaryOperator(LHS, *Context);
if (!DeclExpr1 || !NumExpr1)
return {};
@@ -1054,7 +1065,8 @@
const Expr *DeclExpr2;
const Expr *NumExpr2;
BinaryOperatorKind BO2;
- std::tie(DeclExpr2, BO2, NumExpr2) = tryNormalizeBinaryOperator(RHS);
+ std::tie(DeclExpr2, BO2, NumExpr2) =
+ tryNormalizeBinaryOperator(RHS, *Context);
if (!DeclExpr2 || !NumExpr2)
return {};
@@ -1140,10 +1152,10 @@
/// A bitwise-or with a non-zero constant always evaluates to true.
TryResult checkIncorrectBitwiseOrOperator(const BinaryOperator *B) {
- const Expr *LHSConstant =
- tryTransformToIntOrEnumConstant(B->getLHS()->IgnoreParenImpCasts());
- const Expr *RHSConstant =
- tryTransformToIntOrEnumConstant(B->getRHS()->IgnoreParenImpCasts());
+ const Expr *LHSConstant = tryTransformToIntOrEnumConstant(
+ B->getLHS()->IgnoreParenImpCasts(), *Context);
+ const Expr *RHSConstant = tryTransformToIntOrEnumConstant(
+ B->getRHS()->IgnoreParenImpCasts(), *Context);
if ((LHSConstant && RHSConstant) || (!LHSConstant && !RHSConstant))
return {};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
