Skip to content

Commit dfa7ce4

Browse files
denzor200denzor200
committed
Implement handling of members
1 parent 0d4aa3b commit dfa7ce4

File tree

3 files changed

+204
-12
lines changed

3 files changed

+204
-12
lines changed

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

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,10 @@ static bool assignsToBoolean(const BinaryOperator *BinOp, ASTContext *AC) {
5858
static const NamedDecl *
5959
getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
6060
if (BO->isCompoundAssignmentOp()) {
61-
const auto *DeclRefLHS =
62-
dyn_cast<DeclRefExpr>(BO->getLHS()->IgnoreImpCasts());
63-
return DeclRefLHS ? DeclRefLHS->getDecl() : nullptr;
61+
if (const auto *DeclRefLHS = dyn_cast<DeclRefExpr>(BO->getLHS()->IgnoreImpCasts()))
62+
return DeclRefLHS->getDecl();
63+
else if (const auto *MemberLHS = dyn_cast<MemberExpr>(BO->getLHS()->IgnoreImpCasts()))
64+
return MemberLHS->getMemberDecl();
6465
}
6566
return nullptr;
6667
}
@@ -132,6 +133,17 @@ static bool isBooleanBitwise(const BinaryOperator *BinOp, ASTContext *AC,
132133
return false;
133134
}
134135

136+
static const Expr* getValidCompoundsLHS(const BinaryOperator* BinOp) {
137+
assert(BinOp->isCompoundAssignmentOp());
138+
139+
if (const auto *DeclRefLHS = dyn_cast<DeclRefExpr>(BinOp->getLHS()->IgnoreImpCasts()))
140+
return DeclRefLHS;
141+
else if (const auto *MemberLHS = dyn_cast<MemberExpr>(BinOp->getLHS()->IgnoreImpCasts()))
142+
return MemberLHS;
143+
144+
return nullptr;
145+
}
146+
135147
BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
136148
ClangTidyContext *Context)
137149
: ClangTidyCheck(Name, Context),
@@ -159,9 +171,8 @@ void BoolBitwiseOperationCheck::emitWarningAndChangeOperatorsIfPossible(
159171
auto DiagEmitter = [BinOp, this] {
160172
const NamedDecl *ND = getLHSNamedDeclIfCompoundAssign(BinOp);
161173
return diag(BinOp->getOperatorLoc(),
162-
"use logical operator '%0' for boolean %select{variable "
163-
"%2|semantics}1 instead of bitwise operator '%3'")
164-
<< translate(BinOp->getOpcodeStr()) << (ND == nullptr) << ND
174+
"use logical operator '%0' for boolean %select{%select{member|variable}2 %3|semantics}1 instead of bitwise operator '%4'")
175+
<< translate(BinOp->getOpcodeStr()) << (ND == nullptr) << (!isa<MemberExpr>(BinOp->getLHS()->IgnoreImpCasts())) << ND
165176
<< BinOp->getOpcodeStr();
166177
};
167178

@@ -201,19 +212,22 @@ void BoolBitwiseOperationCheck::emitWarningAndChangeOperatorsIfPossible(
201212

202213
FixItHint InsertEqual;
203214
if (BinOp->isCompoundAssignmentOp()) {
204-
const auto *DeclRefLHS =
205-
dyn_cast<DeclRefExpr>(BinOp->getLHS()->IgnoreImpCasts());
206-
if (!DeclRefLHS)
215+
const auto *LHS = getValidCompoundsLHS(BinOp);
216+
if (!LHS)
207217
return static_cast<void>(DiagEmitter());
208-
const SourceLocation LocLHS = DeclRefLHS->getEndLoc();
218+
const SourceLocation LocLHS = LHS->getEndLoc();
209219
if (LocLHS.isInvalid() || LocLHS.isMacroID())
210220
return static_cast<void>(IgnoreMacros || DiagEmitter());
211221
const SourceLocation InsertLoc =
212222
clang::Lexer::getLocForEndOfToken(LocLHS, 0, SM, Ctx.getLangOpts());
213223
if (InsertLoc.isInvalid() || InsertLoc.isMacroID())
214224
return static_cast<void>(IgnoreMacros || DiagEmitter());
215-
InsertEqual = FixItHint::CreateInsertion(
216-
InsertLoc, " = " + DeclRefLHS->getDecl()->getNameAsString());
225+
auto SourceText = static_cast<std::string>(Lexer::getSourceText(
226+
CharSourceRange::getTokenRange(LHS->getSourceRange()),
227+
SM, Ctx.getLangOpts()
228+
));
229+
llvm::erase_if(SourceText, [](unsigned char ch) { return std::isspace(ch); });
230+
InsertEqual = FixItHint::CreateInsertion(InsertLoc, " = " + SourceText);
217231
}
218232

219233
auto ReplaceOperator = FixItHint::CreateReplacement(TokenRange, FixSpelling);

‎clang-tools-extra/test/clang-tidy/checkers/misc/bool-bitwise-operation-static-members.cpp‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %check_clang_tidy %s misc-bool-bitwise-operation %t
2+
3+
struct A {
4+
static int first;
5+
static bool second;
6+
};
7+
8+
int A::first = 100;
9+
bool A::second = false;
10+
11+
void normal() {
12+
int b = 200;
13+
14+
A::first | b;
15+
A::first & b;
16+
A::first |= b;
17+
A::first &= b;
18+
}
19+
20+
void bad() {
21+
bool b = false;
22+
23+
A::second | b;
24+
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
25+
// CHECK-FIXES: A::second || b;
26+
A::second & b;
27+
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
28+
// CHECK-FIXES: A::second && b;
29+
A::second |= b;
30+
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator '||' for boolean variable 'second' instead of bitwise operator '|=' [misc-bool-bitwise-operation]
31+
// CHECK-FIXES: A::second = A::second || b;
32+
A::second &= b;
33+
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator '&&' for boolean variable 'second' instead of bitwise operator '&=' [misc-bool-bitwise-operation]
34+
// CHECK-FIXES: A::second = A::second && b;
35+
}

‎clang-tools-extra/test/clang-tidy/checkers/misc/bool-bitwise-operation-struct-members.cpp‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
// RUN: %check_clang_tidy %s misc-bool-bitwise-operation %t
2+
3+
struct A {
4+
int first;
5+
bool second;
6+
};
7+
8+
void normal() {
9+
A a {100, false};
10+
int b = 200;
11+
12+
a.first | b;
13+
a.first & b;
14+
a.first |= b;
15+
a.first &= b;
16+
}
17+
18+
void bad() {
19+
A a {-1, true};
20+
bool b = false;
21+
22+
a.second | b;
23+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
24+
// CHECK-FIXES: a.second || b;
25+
a.second & b;
26+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
27+
// CHECK-FIXES: a.second && b;
28+
a.second |= b;
29+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean member 'second' instead of bitwise operator '|=' [misc-bool-bitwise-operation]
30+
// CHECK-FIXES: a.second = a.second || b;
31+
a.second &= b;
32+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean member 'second' instead of bitwise operator '&=' [misc-bool-bitwise-operation]
33+
// CHECK-FIXES: a.second = a.second && b;
34+
}
35+
36+
void bad_two_lines() {
37+
A a {-1, true};
38+
bool b = false;
39+
40+
a.
41+
second | b;
42+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
43+
// CHECK-FIXES: second || b;
44+
a.
45+
second & b;
46+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
47+
// CHECK-FIXES: second && b;
48+
a.
49+
second |= b;
50+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '||' for boolean member 'second' instead of bitwise operator '|=' [misc-bool-bitwise-operation]
51+
// CHECK-FIXES: second = a.second || b;
52+
a
53+
.
54+
second &= b;
55+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator '&&' for boolean member 'second' instead of bitwise operator '&=' [misc-bool-bitwise-operation]
56+
// CHECK-FIXES: second = a.second && b;
57+
a
58+
.second &= b;
59+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '&&' for boolean member 'second' instead of bitwise operator '&=' [misc-bool-bitwise-operation]
60+
// CHECK-FIXES: .second = a.second && b;
61+
}
62+
63+
void bad_side_effects_volatile() {
64+
volatile A a {-1, true};
65+
bool b = false;
66+
67+
a.second | b;
68+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
69+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
70+
a.second & b;
71+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
72+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
73+
a.second |= b;
74+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean member 'second' instead of bitwise operator '|=' [misc-bool-bitwise-operation]
75+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
76+
a.second &= b;
77+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean member 'second' instead of bitwise operator '&=' [misc-bool-bitwise-operation]
78+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
79+
}
80+
81+
struct VolatileA {
82+
volatile bool second;
83+
};
84+
85+
void bad_side_effects_volatile2() {
86+
VolatileA a {true};
87+
bool b = false;
88+
89+
a.second | b;
90+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
91+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
92+
a.second & b;
93+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
94+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
95+
a.second |= b;
96+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean member 'second' instead of bitwise operator '|=' [misc-bool-bitwise-operation]
97+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
98+
a.second &= b;
99+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean member 'second' instead of bitwise operator '&=' [misc-bool-bitwise-operation]
100+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
101+
}
102+
103+
void bad_arrow() {
104+
A a {-1, true};
105+
auto* pa = &a;
106+
bool b = false;
107+
108+
pa->second | b;
109+
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|' [misc-bool-bitwise-operation]
110+
// CHECK-FIXES: pa->second || b;
111+
pa->second & b;
112+
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&' [misc-bool-bitwise-operation]
113+
// CHECK-FIXES: pa->second && b;
114+
pa->second |= b;
115+
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use logical operator '||' for boolean member 'second' instead of bitwise operator '|=' [misc-bool-bitwise-operation]
116+
// CHECK-FIXES: pa->second = pa->second || b;
117+
pa->second &= b;
118+
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use logical operator '&&' for boolean member 'second' instead of bitwise operator '&=' [misc-bool-bitwise-operation]
119+
// CHECK-FIXES: pa->second = pa->second && b;
120+
}
121+
122+
struct B {
123+
bool& access();
124+
};
125+
126+
void bad_no_fixit() {
127+
B b;
128+
bool c = false;
129+
b.access() |= c;
130+
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|=' [misc-bool-bitwise-operation]
131+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
132+
b.access() &= c;
133+
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&=' [misc-bool-bitwise-operation]
134+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
135+
136+
auto* pb = &b;
137+
pb->access() |= c;
138+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: use logical operator '||' for boolean semantics instead of bitwise operator '|=' [misc-bool-bitwise-operation]
139+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
140+
pb->access() &= c;
141+
// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: use logical operator '&&' for boolean semantics instead of bitwise operator '&=' [misc-bool-bitwise-operation]
142+
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
143+
}

0 commit comments

Comments
 (0)