Skip to content

Commit 0d4aa3b

Browse files
committed
Refactor && Change warning mesage
1 parent 352d118 commit 0d4aa3b

8 files changed

+261
-252
lines changed

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

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@
1313
#include <optional>
1414
#include <utility>
1515

16-
// TODO: change warning message
17-
1816
using namespace clang::ast_matchers;
1917

2018
namespace clang::tidy::misc {
@@ -86,37 +84,48 @@ static llvm::StringRef translate(llvm::StringRef Value) {
8684
return {};
8785
}
8886

89-
static bool isBooleanBitwise(const BinaryOperator *BinOp, ASTContext *AC, std::optional<bool>& rootAssignsToBoolean) {
87+
static bool isBooleanBitwise(const BinaryOperator *BinOp, ASTContext *AC,
88+
std::optional<bool> &rootAssignsToBoolean) {
9089
if (!BinOp)
9190
return false;
9291

9392
for (const auto &[Bitwise, _] : OperatorsTransformation) {
9493
if (BinOp->getOpcodeStr() == Bitwise) {
95-
const bool lhsBoolean = BinOp->getLHS()
96-
->IgnoreImpCasts()
97-
->getType()
98-
.getDesugaredType(*AC)
99-
->isBooleanType();
100-
const bool rhsBoolean = BinOp->getRHS()
101-
->IgnoreImpCasts()
102-
->getType()
103-
.getDesugaredType(*AC)
104-
->isBooleanType();
105-
if (lhsBoolean && rhsBoolean) {
106-
rootAssignsToBoolean = rootAssignsToBoolean.value_or(false);
107-
return true;
108-
}
109-
if (assignsToBoolean(BinOp, AC) || rootAssignsToBoolean.value_or(false)) {
110-
rootAssignsToBoolean = rootAssignsToBoolean.value_or(true);
111-
return true;
112-
}
113-
if (BinOp->isCompoundAssignmentOp() && lhsBoolean) {
114-
rootAssignsToBoolean = rootAssignsToBoolean.value_or(true);
115-
return true;
116-
}
117-
if (std::optional<bool> DummyFlag = false; (lhsBoolean || isBooleanBitwise(dyn_cast<BinaryOperator>(BinOp->getLHS()->IgnoreParenImpCasts()), AC, DummyFlag)) && (rhsBoolean || isBooleanBitwise(dyn_cast<BinaryOperator>(BinOp->getRHS()->IgnoreParenImpCasts()), AC, DummyFlag))) {
118-
rootAssignsToBoolean = rootAssignsToBoolean.value_or(false);
119-
return true;
94+
bool lhsBoolean = BinOp->getLHS()
95+
->IgnoreImpCasts()
96+
->getType()
97+
.getDesugaredType(*AC)
98+
->isBooleanType();
99+
bool rhsBoolean = BinOp->getRHS()
100+
->IgnoreImpCasts()
101+
->getType()
102+
.getDesugaredType(*AC)
103+
->isBooleanType();
104+
for (int i = 0; i < 2; ++i) {
105+
if (lhsBoolean && rhsBoolean) {
106+
rootAssignsToBoolean = rootAssignsToBoolean.value_or(false);
107+
return true;
108+
}
109+
if (assignsToBoolean(BinOp, AC) ||
110+
rootAssignsToBoolean.value_or(false)) {
111+
rootAssignsToBoolean = rootAssignsToBoolean.value_or(true);
112+
return true;
113+
}
114+
if (BinOp->isCompoundAssignmentOp() && lhsBoolean) {
115+
rootAssignsToBoolean = rootAssignsToBoolean.value_or(true);
116+
return true;
117+
}
118+
std::optional<bool> DummyFlag = false;
119+
lhsBoolean =
120+
lhsBoolean ||
121+
isBooleanBitwise(dyn_cast<BinaryOperator>(
122+
BinOp->getLHS()->IgnoreParenImpCasts()),
123+
AC, DummyFlag);
124+
rhsBoolean =
125+
rhsBoolean ||
126+
isBooleanBitwise(dyn_cast<BinaryOperator>(
127+
BinOp->getRHS()->IgnoreParenImpCasts()),
128+
AC, DummyFlag);
120129
}
121130
}
122131
}
@@ -151,14 +160,17 @@ void BoolBitwiseOperationCheck::emitWarningAndChangeOperatorsIfPossible(
151160
const NamedDecl *ND = getLHSNamedDeclIfCompoundAssign(BinOp);
152161
return diag(BinOp->getOperatorLoc(),
153162
"use logical operator '%0' for boolean %select{variable "
154-
"%2|values}1 instead of bitwise operator '%3'")
163+
"%2|semantics}1 instead of bitwise operator '%3'")
155164
<< translate(BinOp->getOpcodeStr()) << (ND == nullptr) << ND
156165
<< BinOp->getOpcodeStr();
157166
};
158167

159168
const bool HasVolatileOperand = llvm::any_of(
160169
std::array{BinOp->getLHS(), BinOp->getRHS()}, [&](const Expr *E) {
161-
return E->IgnoreImpCasts()->getType().getDesugaredType(Ctx).isVolatileQualified();
170+
return E->IgnoreImpCasts()
171+
->getType()
172+
.getDesugaredType(Ctx)
173+
.isVolatileQualified();
162174
});
163175
if (HasVolatileOperand)
164176
return static_cast<void>(DiagEmitter());
@@ -250,8 +262,7 @@ void BoolBitwiseOperationCheck::emitWarningAndChangeOperatorsIfPossible(
250262
void BoolBitwiseOperationCheck::visitBinaryTreesNode(
251263
const BinaryOperator *BinOp, const BinaryOperator *ParentBinOp,
252264
const clang::SourceManager &SM, clang::ASTContext &Ctx,
253-
std::optional<bool>& rootAssignsToBoolean) {
254-
//llvm::outs() << "ENTER " << rootAssignsToBoolean << "\n";
265+
std::optional<bool> &rootAssignsToBoolean) {
255266
if (!BinOp)
256267
return;
257268

@@ -264,8 +275,6 @@ void BoolBitwiseOperationCheck::visitBinaryTreesNode(
264275
visitBinaryTreesNode(
265276
dyn_cast<BinaryOperator>(BinOp->getRHS()->IgnoreParenImpCasts()), BinOp,
266277
SM, Ctx, rootAssignsToBoolean);
267-
268-
//llvm::outs() << "LEAVE\n";
269278
}
270279

271280
void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {

clang-tools-extra/test/clang-tidy/checkers/misc/bool-bitwise-operation-change-possible-side-effect.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ void bad_possible_side_effects() {
88
bool a = true, b = false;
99

1010
a | function_with_possible_side_effects();
11-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
11+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
1212
// CHECK-FIXES: a || function_with_possible_side_effects();
1313

1414
a & function_with_possible_side_effects();
15-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
15+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
1616
// CHECK-FIXES: a && function_with_possible_side_effects();
1717

1818
a |= function_with_possible_side_effects();
@@ -47,11 +47,11 @@ void bad_definitely_side_effects() {
4747
int acc = 0;
4848

4949
a | (acc++, b);
50-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
50+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
5151
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
5252

5353
a & (acc++, b);
54-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
54+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
5555
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
5656

5757
a |= (acc++, b);

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ void bad_in_macro() {
1616

1717
// change operator - GOOD
1818
IDENT(a) | b;
19-
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
19+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
2020
// CHECK-FIXES: IDENT(a) || b;
2121
a & IDENT(b);
22-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
22+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
2323
// CHECK-FIXES: a && IDENT(b);
2424
IDENT(a) & IDENT(b);
25-
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [misc-bool-bitwise-operation]
25+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
2626
// CHECK-FIXES: IDENT(a) && IDENT(b);
2727

2828
// insert `)` - BAD
@@ -31,15 +31,15 @@ void bad_in_macro() {
3131

3232
// insert `)` - GOOD
3333
a && b | c IDENT(&& e);
34-
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
34+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
3535
// CHECK-FIXES: a && (b || c) IDENT(&& e);
3636

3737
// insert `(` - BAD
3838
a IDENT(&& b) | c && e;
3939

4040
// insert `(` - GOOD
4141
IDENT(a &&) b | c && e;
42-
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [misc-bool-bitwise-operation]
42+
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
4343
// CHECK-FIXES: IDENT(a &&) (b || c) && e;
4444

4545
bool ab = false;

0 commit comments

Comments
 (0)