Skip to content

Commit 14dcf62

Browse files
committed
Implement more cases with unsigned flags
1 parent 5bc7f73 commit 14dcf62

File tree

3 files changed

+71
-10
lines changed

3 files changed

+71
-10
lines changed

clang-tools-extra/clang-tidy/misc/BoolBitwiseOperationCheck.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,19 @@ static llvm::StringRef translate(llvm::StringRef Value) {
8787
return {};
8888
}
8989

90-
static bool isBooleanBitwise(const BinaryOperator *BinOp, ASTContext *AC) {
90+
static bool isBooleanBitwise(const BinaryOperator *BinOp, ASTContext *AC, std::optional<bool>& rootAssignsToBoolean) {
9191
for (const auto &[Bitwise, _] : OperatorsTransformation) {
9292
if (BinOp->getOpcodeStr() == Bitwise) {
9393
const bool hasBooleanOperands = llvm::all_of(
9494
std::array{BinOp->getLHS(), BinOp->getRHS()}, [](const Expr *E) {
9595
return E->IgnoreImpCasts()->getType().getTypePtr()->isBooleanType();
9696
});
9797
if (hasBooleanOperands) {
98+
rootAssignsToBoolean = rootAssignsToBoolean.value_or(false);
9899
return true;
99100
}
100-
if (assignsToBoolean(BinOp, AC)) {
101+
if (assignsToBoolean(BinOp, AC) || rootAssignsToBoolean.value_or(false)) {
102+
rootAssignsToBoolean = rootAssignsToBoolean.value_or(true);
101103
return true;
102104
}
103105
}
@@ -231,28 +233,30 @@ void BoolBitwiseOperationCheck::emitWarningAndChangeOperatorsIfPossible(
231233

232234
void BoolBitwiseOperationCheck::visitBinaryTreesNode(
233235
const BinaryOperator *BinOp, const BinaryOperator *ParentBinOp,
234-
const clang::SourceManager &SM, clang::ASTContext &Ctx) {
236+
const clang::SourceManager &SM, clang::ASTContext &Ctx,
237+
std::optional<bool>& rootAssignsToBoolean) {
235238
if (!BinOp) {
236239
return;
237240
}
238241

239-
if (isBooleanBitwise(BinOp, &Ctx)) {
242+
if (isBooleanBitwise(BinOp, &Ctx, rootAssignsToBoolean)) {
240243
emitWarningAndChangeOperatorsIfPossible(BinOp, ParentBinOp, SM, Ctx);
241244
}
242245

243246
visitBinaryTreesNode(
244247
dyn_cast<BinaryOperator>(BinOp->getLHS()->IgnoreParenImpCasts()), BinOp,
245-
SM, Ctx);
248+
SM, Ctx, rootAssignsToBoolean);
246249
visitBinaryTreesNode(
247250
dyn_cast<BinaryOperator>(BinOp->getRHS()->IgnoreParenImpCasts()), BinOp,
248-
SM, Ctx);
251+
SM, Ctx, rootAssignsToBoolean);
249252
}
250253

251254
void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
252255
const auto *binOpRoot = Result.Nodes.getNodeAs<BinaryOperator>("binOpRoot");
253256
const SourceManager &SM = *Result.SourceManager;
254257
ASTContext &Ctx = *Result.Context;
255-
visitBinaryTreesNode(binOpRoot, nullptr, SM, Ctx);
258+
std::optional<bool> rootAssignsToBoolean = std::nullopt;
259+
visitBinaryTreesNode(binOpRoot, nullptr, SM, Ctx, rootAssignsToBoolean);
256260
}
257261

258262
} // namespace clang::tidy::misc

clang-tools-extra/clang-tidy/misc/BoolBitwiseOperationCheck.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ class BoolBitwiseOperationCheck : public ClangTidyCheck {
3939
void visitBinaryTreesNode(const BinaryOperator *BinOp,
4040
const BinaryOperator *ParentBinOp,
4141
const clang::SourceManager &SM,
42-
clang::ASTContext &Ctx);
42+
clang::ASTContext &Ctx,
43+
std::optional<bool>& rootAssignsToBoolean);
4344

4445
private:
4546
bool StrictMode;

clang-tools-extra/test/clang-tidy/checkers/misc/bool-bitwise-operation-not-only-bool-operand.cpp

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
void general(unsigned flags, bool value) {
66
(flags << 1) | value;
77
flags = (flags << 1) | value;
8+
flags = (flags << 1) | (flags << 2) | value;
9+
flags = (flags << 1) | (flags << 2) | (flags << 4) | value;
810
}
911

10-
void take(bool value) {}
12+
// TODO: compound operators
1113

12-
// TODO: (flags << 1) | (flags << 2) | value
14+
void take(bool value) {}
1315

1416
template<bool bb = true | 1>
1517
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
@@ -39,6 +41,15 @@ void assign_to_boolean(unsigned flags, bool value) {
3941
take((flags << 1) | value);
4042
// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
4143
// CHECK-FIXES: take((flags << 1) || value);
44+
result = (flags << 1) | (flags << 2) | value;
45+
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
46+
// CHECK-MESSAGES: :[[@LINE-2]]:42: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
47+
// CHECK-FIXES: result = (flags << 1) || (flags << 2) || value;
48+
result = (flags << 1) | (flags << 2) | (flags << 4) | value;
49+
// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
50+
// CHECK-MESSAGES: :[[@LINE-2]]:42: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
51+
// CHECK-MESSAGES: :[[@LINE-3]]:57: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
52+
// CHECK-FIXES: result = (flags << 1) || (flags << 2) || (flags << 4) || value;
4253
}
4354

4455
template<bool bb = (true | 1)>
@@ -69,6 +80,15 @@ void assign_to_boolean_parens(unsigned flags, bool value) {
6980
take(((flags << 1) | value));
7081
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
7182
// CHECK-FIXES: take(((flags << 1) || value));
83+
result = ((flags << 1) | (flags << 2) | value);
84+
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
85+
// CHECK-MESSAGES: :[[@LINE-2]]:43: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
86+
// CHECK-FIXES: result = ((flags << 1) || (flags << 2) || value);
87+
result = ((flags << 1) | (flags << 2) | (flags << 4) | value);
88+
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
89+
// CHECK-MESSAGES: :[[@LINE-2]]:43: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
90+
// CHECK-MESSAGES: :[[@LINE-3]]:58: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
91+
// CHECK-FIXES: result = ((flags << 1) || (flags << 2) || (flags << 4) || value);
7292
}
7393

7494
template<bool bb = ((true | 1))>
@@ -99,6 +119,15 @@ void assign_to_boolean_parens2(unsigned flags, bool value) {
99119
take((((flags << 1) | value)));
100120
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
101121
// CHECK-FIXES: take((((flags << 1) || value)));
122+
result = (((flags << 1) | (flags << 2) | value));
123+
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
124+
// CHECK-MESSAGES: :[[@LINE-2]]:44: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
125+
// CHECK-FIXES: result = (((flags << 1) || (flags << 2) || value));
126+
result = (((flags << 1) | (flags << 2) | (flags << 4) | value));
127+
// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
128+
// CHECK-MESSAGES: :[[@LINE-2]]:44: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
129+
// CHECK-MESSAGES: :[[@LINE-3]]:59: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
130+
// CHECK-FIXES: result = (((flags << 1) || (flags << 2) || (flags << 4) || value));
102131
}
103132

104133
// functional cast
@@ -130,6 +159,15 @@ void assign_to_boolean_fcast(unsigned flags, bool value) {
130159
take(bool((flags << 1) | value));
131160
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
132161
// CHECK-FIXES: take(bool((flags << 1) || value));
162+
result = bool((flags << 1) | (flags << 2) | value);
163+
// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
164+
// CHECK-MESSAGES: :[[@LINE-2]]:47: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
165+
// CHECK-FIXES: result = bool((flags << 1) || (flags << 2) || value);
166+
result = bool((flags << 1) | (flags << 2) | (flags << 4) | value);
167+
// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
168+
// CHECK-MESSAGES: :[[@LINE-2]]:47: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
169+
// CHECK-MESSAGES: :[[@LINE-3]]:62: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
170+
// CHECK-FIXES: result = bool((flags << 1) || (flags << 2) || (flags << 4) || value);
133171
}
134172

135173
// C-style cast
@@ -161,6 +199,15 @@ void assign_to_boolean_ccast(unsigned flags, bool value) {
161199
take(bool((flags << 1) | value));
162200
// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
163201
// CHECK-FIXES: take(bool((flags << 1) || value));
202+
result = (bool)((flags << 1) | (flags << 2) | value);
203+
// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
204+
// CHECK-MESSAGES: :[[@LINE-2]]:49: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
205+
// CHECK-FIXES: result = (bool)((flags << 1) || (flags << 2) || value);
206+
result = (bool)((flags << 1) | (flags << 2) | (flags << 4) | value);
207+
// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
208+
// CHECK-MESSAGES: :[[@LINE-2]]:49: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
209+
// CHECK-MESSAGES: :[[@LINE-3]]:64: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
210+
// CHECK-FIXES: result = (bool)((flags << 1) || (flags << 2) || (flags << 4) || value);
164211
}
165212

166213
// static_cast
@@ -192,6 +239,15 @@ void assign_to_boolean_scast(unsigned flags, bool value) {
192239
take(static_cast<bool>((flags << 1) | value));
193240
// CHECK-MESSAGES: :[[@LINE-1]]:41: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
194241
// CHECK-FIXES: take(static_cast<bool>((flags << 1) || value));
242+
result = static_cast<bool>((flags << 1) | (flags << 2) | value);
243+
// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
244+
// CHECK-MESSAGES: :[[@LINE-2]]:60: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
245+
// CHECK-FIXES: result = static_cast<bool>((flags << 1) || (flags << 2) || value);
246+
result = static_cast<bool>((flags << 1) | (flags << 2) | (flags << 4) | value);
247+
// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
248+
// CHECK-MESSAGES: :[[@LINE-2]]:60: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
249+
// CHECK-MESSAGES: :[[@LINE-3]]:75: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
250+
// CHECK-FIXES: result = static_cast<bool>((flags << 1) || (flags << 2) || (flags << 4) || value);
195251
}
196252

197253

0 commit comments

Comments
 (0)