Skip to content

Commit 3872a69

Browse files
committed
Add IgnoreMacros option
1 parent a573e50 commit 3872a69

File tree

5 files changed

+146
-32
lines changed

5 files changed

+146
-32
lines changed

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

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "BoolBitwiseOperationCheck.h"
1010
#include "clang/ASTMatchers/ASTMatchFinder.h"
1111
#include "clang/Lex/Lexer.h"
12+
#include "llvm/ADT/ScopeExit.h"
1213
#include <array>
1314
#include <optional>
1415
#include <utility>
@@ -83,11 +84,13 @@ static llvm::StringRef translate(llvm::StringRef Value) {
8384
BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
8485
ClangTidyContext *Context)
8586
: ClangTidyCheck(Name, Context),
86-
StrictMode(Options.get("StrictMode", true)) {}
87+
StrictMode(Options.get("StrictMode", true)),
88+
IgnoreMacros(Options.get("IgnoreMacros", false)) {}
8789

8890
void BoolBitwiseOperationCheck::storeOptions(
8991
ClangTidyOptions::OptionMap &Opts) {
9092
Options.store(Opts, "StrictMode", StrictMode);
93+
Options.store(Opts, "IgnoreMacros", IgnoreMacros);
9194
}
9295

9396
void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
@@ -106,11 +109,14 @@ void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
106109
void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
107110
const auto *MatchedExpr = Result.Nodes.getNodeAs<BinaryOperator>("op");
108111

109-
auto Diag = diag(MatchedExpr->getOperatorLoc(),
110-
"use logical operator '%0' for boolean %1 instead of "
111-
"bitwise operator '%2'")
112-
<< translate(MatchedExpr->getOpcodeStr())
113-
<< tryPrintVariable(MatchedExpr) << MatchedExpr->getOpcodeStr();
112+
auto DiagEmitterProc = [MatchedExpr, this] {
113+
return diag(MatchedExpr->getOperatorLoc(),
114+
"use logical operator '%0' for boolean %1 instead of "
115+
"bitwise operator '%2'")
116+
<< translate(MatchedExpr->getOpcodeStr())
117+
<< tryPrintVariable(MatchedExpr) << MatchedExpr->getOpcodeStr();
118+
};
119+
auto DiagEmitter = llvm::make_scope_exit(DiagEmitterProc);
114120

115121
if (isInTemplateFunction(MatchedExpr, *Result.Context))
116122
return;
@@ -127,16 +133,25 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
127133

128134
SourceLocation Loc = MatchedExpr->getOperatorLoc();
129135

130-
if (Loc.isInvalid() || Loc.isMacroID())
136+
if (Loc.isInvalid() || Loc.isMacroID()) {
137+
if (IgnoreMacros)
138+
DiagEmitter.release();
131139
return;
140+
}
132141

133142
Loc = Result.SourceManager->getSpellingLoc(Loc);
134-
if (Loc.isInvalid() || Loc.isMacroID())
143+
if (Loc.isInvalid() || Loc.isMacroID()) {
144+
if (IgnoreMacros)
145+
DiagEmitter.release();
135146
return;
147+
}
136148

137149
const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
138-
if (TokenRange.isInvalid())
150+
if (TokenRange.isInvalid()) {
151+
if (IgnoreMacros)
152+
DiagEmitter.release();
139153
return;
154+
}
140155

141156
StringRef Spelling = Lexer::getSourceText(TokenRange, *Result.SourceManager,
142157
Result.Context->getLangOpts());
@@ -154,11 +169,16 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
154169
if (!DelcRefLHS)
155170
return;
156171
const SourceLocation LocLHS = DelcRefLHS->getEndLoc();
157-
if (LocLHS.isInvalid() || LocLHS.isMacroID())
172+
if (LocLHS.isInvalid() || LocLHS.isMacroID()) {
173+
if (IgnoreMacros)
174+
DiagEmitter.release();
158175
return;
176+
}
159177
const SourceLocation InsertLoc = clang::Lexer::getLocForEndOfToken(
160178
LocLHS, 0, *Result.SourceManager, Result.Context->getLangOpts());
161179
if (InsertLoc.isInvalid() || InsertLoc.isMacroID()) {
180+
if (IgnoreMacros)
181+
DiagEmitter.release();
162182
return;
163183
}
164184
InsertEqual = FixItHint::CreateInsertion(
@@ -201,13 +221,18 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
201221
SurroundedExpr->getEndLoc(), 0, *Result.SourceManager,
202222
Result.Context->getLangOpts());
203223
if (InsertFirstLoc.isInvalid() || InsertFirstLoc.isMacroID() ||
204-
InsertSecondLoc.isInvalid() || InsertSecondLoc.isMacroID())
224+
InsertSecondLoc.isInvalid() || InsertSecondLoc.isMacroID()) {
225+
if (IgnoreMacros)
226+
DiagEmitter.release();
205227
return;
228+
}
206229
InsertBrace1 = FixItHint::CreateInsertion(InsertFirstLoc, "(");
207230
InsertBrace2 = FixItHint::CreateInsertion(InsertSecondLoc, ")");
208231
}
209232

210-
Diag << InsertEqual << ReplaceOperator << InsertBrace1 << InsertBrace2;
233+
DiagEmitter.release();
234+
DiagEmitterProc() << InsertEqual << ReplaceOperator << InsertBrace1
235+
<< InsertBrace2;
211236
}
212237

213238
} // namespace clang::tidy::performance

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class BoolBitwiseOperationCheck : public ClangTidyCheck {
3131

3232
private:
3333
bool StrictMode;
34+
bool IgnoreMacros;
3435
};
3536

3637
} // namespace clang::tidy::performance
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// RUN: %check_clang_tidy %s performance-bool-bitwise-operation %t \
2+
// RUN: -config="{CheckOptions: { \
3+
// RUN: performance-bool-bitwise-operation.IgnoreMacros: true }}"
4+
5+
#define MY_OR |
6+
#define MY_AND &
7+
#define MY_OR_ASSIGN |=
8+
#define MY_AND_ASSIGN &=
9+
#define MY_LOG_AND &&
10+
11+
#define CAT(a, b) a ## b
12+
#define IDENT(a) a
13+
14+
void bad_in_macro() {
15+
bool a = true, b = false;
16+
17+
// change operator - BAD
18+
IDENT(a |) b;
19+
a IDENT(& b);
20+
IDENT(a |=) b;
21+
a IDENT(&= b);
22+
23+
// change operator - GOOD
24+
IDENT(a) | b;
25+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
26+
// CHECK-FIXES: IDENT(a) || b;
27+
a & IDENT(b);
28+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [performance-bool-bitwise-operation]
29+
// CHECK-FIXES: a && IDENT(b);
30+
IDENT(a) & IDENT(b);
31+
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [performance-bool-bitwise-operation]
32+
// CHECK-FIXES: IDENT(a) && IDENT(b);
33+
34+
// insert `)` - BAD
35+
bool c = true, e = false;
36+
a && b | IDENT(c &&) e;
37+
38+
// insert `)` - GOOD
39+
a && b | c IDENT(&& e);
40+
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
41+
// CHECK-FIXES: a && (b || c) IDENT(&& e);
42+
43+
// insert `(` - BAD
44+
a IDENT(&& b) | c && e;
45+
46+
// insert `(` - GOOD
47+
IDENT(a &&) b | c && e;
48+
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
49+
// CHECK-FIXES: IDENT(a &&) (b || c) && e;
50+
51+
bool ab = false;
52+
// insert ` = a` - BAD
53+
CAT(a, b) &= b;
54+
55+
// insert ` = a`- GOOD
56+
b &= CAT(a, b);
57+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean variable 'b' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
58+
// CHECK-FIXES: b = b && CAT(a, b);
59+
}
60+

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

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -289,19 +289,16 @@ void bad_in_macro() {
289289
bool a = true, b = false;
290290

291291
// change operator - BAD
292-
a MY_OR b;
293-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
294-
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
295-
a MY_AND b;
296-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [performance-bool-bitwise-operation]
292+
IDENT(a bitor) b;
293+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
297294
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
298-
a MY_OR_ASSIGN b;
299-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean variable 'a' instead of bitwise operator '|=' [performance-bool-bitwise-operation]
295+
a IDENT(bitand b);
296+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [performance-bool-bitwise-operation]
300297
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
301-
a MY_AND_ASSIGN b;
302-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean variable 'a' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
298+
IDENT(a or_eq) b;
299+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '||' for boolean variable 'a' instead of bitwise operator '|=' [performance-bool-bitwise-operation]
303300
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
304-
IDENT(a and_eq b);
301+
a IDENT(and_eq b);
305302
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '&&' for boolean variable 'a' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
306303
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
307304

@@ -349,6 +346,23 @@ void bad_in_macro() {
349346
// CHECK-FIXES: b = b and CAT(a, b);
350347
}
351348

349+
void bad_in_macro_fixit() {
350+
bool a = true, b = false;
351+
352+
// FIXME: implement fixit for all of these cases
353+
354+
a MY_OR b;
355+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
356+
a MY_AND b;
357+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [performance-bool-bitwise-operation]
358+
a MY_OR_ASSIGN b;
359+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean variable 'a' instead of bitwise operator '|=' [performance-bool-bitwise-operation]
360+
a MY_AND_ASSIGN b;
361+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean variable 'a' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
362+
IDENT(a and_eq b);
363+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '&&' for boolean variable 'a' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
364+
}
365+
352366
template<typename T>
353367
void good_in_unreachable_template(T a, T b) {
354368
a bitor b;

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

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -289,19 +289,16 @@ void bad_in_macro() {
289289
bool a = true, b = false;
290290

291291
// change operator - BAD
292-
a MY_OR b;
293-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
292+
IDENT(a |) b;
293+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
294294
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
295-
a MY_AND b;
296-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [performance-bool-bitwise-operation]
295+
a IDENT(& b);
296+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [performance-bool-bitwise-operation]
297297
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
298-
a MY_OR_ASSIGN b;
299-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean variable 'a' instead of bitwise operator '|=' [performance-bool-bitwise-operation]
298+
IDENT(a |=) b;
299+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '||' for boolean variable 'a' instead of bitwise operator '|=' [performance-bool-bitwise-operation]
300300
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
301-
a MY_AND_ASSIGN b;
302-
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean variable 'a' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
303-
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
304-
IDENT(a &= b);
301+
a IDENT(&= b);
305302
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '&&' for boolean variable 'a' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
306303
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
307304

@@ -349,6 +346,23 @@ void bad_in_macro() {
349346
// CHECK-FIXES: b = b && CAT(a, b);
350347
}
351348

349+
void bad_in_macro_fixit() {
350+
bool a = true, b = false;
351+
352+
// FIXME: implement fixit for all of these cases
353+
354+
a MY_OR b;
355+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean values instead of bitwise operator '|' [performance-bool-bitwise-operation]
356+
a MY_AND b;
357+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean values instead of bitwise operator '&' [performance-bool-bitwise-operation]
358+
a MY_OR_ASSIGN b;
359+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '||' for boolean variable 'a' instead of bitwise operator '|=' [performance-bool-bitwise-operation]
360+
a MY_AND_ASSIGN b;
361+
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator '&&' for boolean variable 'a' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
362+
IDENT(a &= b);
363+
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator '&&' for boolean variable 'a' instead of bitwise operator '&=' [performance-bool-bitwise-operation]
364+
}
365+
352366
template<typename T>
353367
void good_in_unreachable_template(T a, T b) {
354368
a | b;

0 commit comments

Comments
 (0)