Skip to content

Commit 0a1b47f

Browse files
committed
Fix realworld case with unsigned flags
1 parent 710b7b4 commit 0a1b47f

File tree

4 files changed

+311
-45
lines changed

4 files changed

+311
-45
lines changed

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

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,68 @@ using namespace clang::ast_matchers;
1717

1818
namespace clang::tidy::misc {
1919

20+
static const Stmt* ignoreParenExpr(const Stmt* S) {
21+
while (const auto* PE = dyn_cast<ParenExpr>(S)) {
22+
S = PE->getSubExpr();
23+
}
24+
return S;
25+
}
26+
27+
template<typename R>
28+
static bool
29+
containsNodeIgnoringParenExpr(R&& children, const Stmt* Node) {
30+
for (const Stmt* Child : children) {
31+
if (ignoreParenExpr(Child) == Node) {
32+
return true;
33+
}
34+
}
35+
return false;
36+
}
37+
38+
static bool isBooleanImplicitCastStmt(const Stmt* S) {
39+
const auto* ICE = dyn_cast<ImplicitCastExpr>(S);
40+
return ICE && ICE->getType()->isBooleanType();
41+
}
42+
43+
namespace {
44+
AST_MATCHER(BinaryOperator, assignsToBoolean) {
45+
auto Parents = Finder->getASTContext().getParents(Node);
46+
47+
for (const auto& Parent : Parents) {
48+
// Check for Decl parents
49+
if (const auto* D = Parent.template get<Decl>()) {
50+
const Expr* InitExpr = nullptr;
51+
52+
if (const auto* VD = dyn_cast<VarDecl>(D)) {
53+
InitExpr = VD->getInit();
54+
} else if (const auto* FD = dyn_cast<FieldDecl>(D)) {
55+
InitExpr = FD->getInClassInitializer();
56+
} else if (const auto* NTTPD = dyn_cast<NonTypeTemplateParmDecl>(D)) {
57+
if (NTTPD->getType()->isBooleanType()) {
58+
return true;
59+
}
60+
}
61+
62+
if (InitExpr && isBooleanImplicitCastStmt(InitExpr)) {
63+
return true;
64+
}
65+
}
66+
67+
// Check for Stmt parents
68+
if (const auto* S = Parent.template get<Stmt>()) {
69+
for (const Stmt* Child : S->children()) {
70+
if (isBooleanImplicitCastStmt(Child) && containsNodeIgnoringParenExpr(Child->children(), &Node)) {
71+
return true;
72+
}
73+
}
74+
}
75+
}
76+
77+
return false;
78+
}
79+
80+
} // namespace
81+
2082
static const NamedDecl *
2183
getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
2284
if (BO->isCompoundAssignmentOp()) {
@@ -27,6 +89,8 @@ getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
2789
return nullptr;
2890
}
2991

92+
constexpr std::array<llvm::StringRef, 4U> OperatorsNames{"|", "&", "|=", "&="};
93+
3094
constexpr std::array<std::pair<llvm::StringRef, llvm::StringRef>, 8U>
3195
OperatorsTransformation{{{"|", "||"},
3296
{"|=", "||"},
@@ -62,8 +126,12 @@ void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
62126
Finder->addMatcher(
63127
binaryOperator(
64128
unless(isExpansionInSystemHeader()),
65-
hasAnyOperatorName("|", "&", "|=", "&="),
66-
hasEitherOperand(expr(hasType(booleanType()))),
129+
hasAnyOperatorName(OperatorsNames),
130+
anyOf(
131+
hasOperands(expr(hasType(booleanType())), expr(hasType(booleanType()))),
132+
assignsToBoolean()
133+
),
134+
// isBooleanLikeOperands(),
67135
optionally(hasParent( // to simple implement transformations like
68136
// `a&&b|c` -> `a&&(b||c)`
69137
binaryOperator().bind("p"))))

clang-tools-extra/test/clang-tidy/checkers/misc/bool-bitwise-operation-nontraditional.cpp

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -167,25 +167,26 @@ void bad_with_priors() {
167167

168168
void bad_with_priors2() {
169169
bool a = false, b = true, c = true;
170+
bool r;
170171
a xor b bitand c;
171172
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
172173
// CHECK-FIXES: a xor (b and c);
173174

174175
// braces added in the first change
175-
a bitor b bitand c;
176-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
177-
// CHECK-MESSAGES: :[[@LINE-2]]:15: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
178-
// CHECK-FIXES: a or (b and c);
176+
r = a bitor b bitand c;
177+
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
178+
// CHECK-MESSAGES: :[[@LINE-2]]:19: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
179+
// CHECK-FIXES: r = a or (b and c);
179180

180-
b bitand c xor a;
181-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
182-
// CHECK-FIXES: (b and c) xor a;
181+
r = b bitand c xor a;
182+
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
183+
// CHECK-FIXES: r = (b and c) xor a;
183184

184185
// braces added in the first change
185-
b bitand c bitor a;
186-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
187-
// CHECK-MESSAGES: :[[@LINE-2]]:16: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
188-
// CHECK-FIXES: (b and c) or a;
186+
r = b bitand c bitor a;
187+
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
188+
// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
189+
// CHECK-FIXES: r = (b and c) or a;
189190
}
190191

191192
template<typename T>
@@ -204,13 +205,13 @@ void bad_has_ancestor() {
204205
// CHECK-FIXES: a xor ident(b and c or a);
205206

206207
a bitor ident(a ? b bitand c : c);
207-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
208-
// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
208+
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
209209
// CHECK-FIXES: a bitor ident(a ? b and c : c);
210210
}
211211

212212
void bad_with_priors_already_braced() {
213213
bool a = false, b = true, c = true;
214+
bool r;
214215
a and (b bitor c);
215216
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
216217
// CHECK-FIXES: a and (b or c);
@@ -228,19 +229,19 @@ void bad_with_priors_already_braced() {
228229
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
229230
// CHECK-FIXES: a xor (b and c);
230231

231-
a bitor (b bitand c);
232-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
233-
// CHECK-MESSAGES: :[[@LINE-2]]:16: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
234-
// CHECK-FIXES: a or (b and c);
232+
r = a bitor (b bitand c);
233+
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
234+
// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
235+
// CHECK-FIXES: r = a or (b and c);
235236

236237
(b bitand c) xor a;
237238
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
238239
// CHECK-FIXES: (b and c) xor a;
239240

240-
(b bitand c) bitor a;
241-
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
242-
// CHECK-MESSAGES: :[[@LINE-2]]:18: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
243-
// CHECK-FIXES: (b and c) or a;
241+
r = (b bitand c) bitor a;
242+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
243+
// CHECK-MESSAGES: :[[@LINE-2]]:22: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
244+
// CHECK-FIXES: r = (b and c) or a;
244245
}
245246

246247
void bad_with_priors_compound() {

0 commit comments

Comments
 (0)