Skip to content

Commit 710b7b4

Browse files
committed
Stock files were taken
1 parent 7b91bb2 commit 710b7b4

12 files changed

+1261
-5
lines changed
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
//===--- BoolBitwiseOperationCheck.cpp - clang-tidy -----------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "BoolBitwiseOperationCheck.h"
10+
#include "clang/ASTMatchers/ASTMatchFinder.h"
11+
#include "clang/Lex/Lexer.h"
12+
#include <array>
13+
#include <optional>
14+
#include <utility>
15+
16+
using namespace clang::ast_matchers;
17+
18+
namespace clang::tidy::misc {
19+
20+
static const NamedDecl *
21+
getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
22+
if (BO->isCompoundAssignmentOp()) {
23+
const auto *DeclRefLHS =
24+
dyn_cast<DeclRefExpr>(BO->getLHS()->IgnoreImpCasts());
25+
return DeclRefLHS ? DeclRefLHS->getDecl() : nullptr;
26+
}
27+
return nullptr;
28+
}
29+
30+
constexpr std::array<std::pair<llvm::StringRef, llvm::StringRef>, 8U>
31+
OperatorsTransformation{{{"|", "||"},
32+
{"|=", "||"},
33+
{"&", "&&"},
34+
{"&=", "&&"},
35+
{"bitand", "and"},
36+
{"and_eq", "and"},
37+
{"bitor", "or"},
38+
{"or_eq", "or"}}};
39+
40+
static llvm::StringRef translate(llvm::StringRef Value) {
41+
for (const auto &[Bitwise, Logical] : OperatorsTransformation) {
42+
if (Value == Bitwise)
43+
return Logical;
44+
}
45+
46+
return {};
47+
}
48+
49+
BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
50+
ClangTidyContext *Context)
51+
: ClangTidyCheck(Name, Context),
52+
StrictMode(Options.get("StrictMode", false)),
53+
IgnoreMacros(Options.get("IgnoreMacros", false)) {}
54+
55+
void BoolBitwiseOperationCheck::storeOptions(
56+
ClangTidyOptions::OptionMap &Opts) {
57+
Options.store(Opts, "StrictMode", StrictMode);
58+
Options.store(Opts, "IgnoreMacros", IgnoreMacros);
59+
}
60+
61+
void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
62+
Finder->addMatcher(
63+
binaryOperator(
64+
unless(isExpansionInSystemHeader()),
65+
hasAnyOperatorName("|", "&", "|=", "&="),
66+
hasEitherOperand(expr(hasType(booleanType()))),
67+
optionally(hasParent( // to simple implement transformations like
68+
// `a&&b|c` -> `a&&(b||c)`
69+
binaryOperator().bind("p"))))
70+
.bind("op"),
71+
this);
72+
}
73+
74+
void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
75+
const auto *MatchedExpr = Result.Nodes.getNodeAs<BinaryOperator>("op");
76+
77+
auto DiagEmitter = [MatchedExpr, this] {
78+
const NamedDecl *ND = getLHSNamedDeclIfCompoundAssign(MatchedExpr);
79+
return diag(MatchedExpr->getOperatorLoc(),
80+
"use logical operator '%0' for boolean %select{variable "
81+
"%2|values}1 instead of bitwise operator '%3'")
82+
<< translate(MatchedExpr->getOpcodeStr()) << (ND == nullptr) << ND
83+
<< MatchedExpr->getOpcodeStr();
84+
};
85+
86+
const bool HasVolatileOperand = llvm::any_of(
87+
std::array{MatchedExpr->getLHS(), MatchedExpr->getRHS()},
88+
[](const Expr *E) {
89+
return E->IgnoreImpCasts()->getType().isVolatileQualified();
90+
});
91+
if (HasVolatileOperand)
92+
return static_cast<void>(DiagEmitter());
93+
94+
const bool HasSideEffects = MatchedExpr->getRHS()->HasSideEffects(
95+
*Result.Context, /*IncludePossibleEffects=*/!StrictMode);
96+
if (HasSideEffects)
97+
return static_cast<void>(DiagEmitter());
98+
99+
SourceLocation Loc = MatchedExpr->getOperatorLoc();
100+
101+
if (Loc.isInvalid() || Loc.isMacroID())
102+
return static_cast<void>(IgnoreMacros || DiagEmitter());
103+
104+
Loc = Result.SourceManager->getSpellingLoc(Loc);
105+
if (Loc.isInvalid() || Loc.isMacroID())
106+
return static_cast<void>(IgnoreMacros || DiagEmitter());
107+
108+
const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
109+
if (TokenRange.isInvalid())
110+
return static_cast<void>(IgnoreMacros || DiagEmitter());
111+
112+
const StringRef FixSpelling = translate(Lexer::getSourceText(
113+
TokenRange, *Result.SourceManager, Result.Context->getLangOpts()));
114+
115+
if (FixSpelling.empty())
116+
return static_cast<void>(DiagEmitter());
117+
118+
FixItHint InsertEqual;
119+
if (MatchedExpr->isCompoundAssignmentOp()) {
120+
const auto *DeclRefLHS =
121+
dyn_cast<DeclRefExpr>(MatchedExpr->getLHS()->IgnoreImpCasts());
122+
if (!DeclRefLHS)
123+
return static_cast<void>(DiagEmitter());
124+
const SourceLocation LocLHS = DeclRefLHS->getEndLoc();
125+
if (LocLHS.isInvalid() || LocLHS.isMacroID())
126+
return static_cast<void>(IgnoreMacros || DiagEmitter());
127+
const SourceLocation InsertLoc = clang::Lexer::getLocForEndOfToken(
128+
LocLHS, 0, *Result.SourceManager, Result.Context->getLangOpts());
129+
if (InsertLoc.isInvalid() || InsertLoc.isMacroID())
130+
return static_cast<void>(IgnoreMacros || DiagEmitter());
131+
InsertEqual = FixItHint::CreateInsertion(
132+
InsertLoc, " = " + DeclRefLHS->getDecl()->getNameAsString());
133+
}
134+
135+
auto ReplaceOperator = FixItHint::CreateReplacement(TokenRange, FixSpelling);
136+
137+
const auto *Parent = Result.Nodes.getNodeAs<BinaryOperator>("p");
138+
std::optional<BinaryOperatorKind> ParentOpcode;
139+
if (Parent)
140+
ParentOpcode = Parent->getOpcode();
141+
142+
const auto *RHS =
143+
dyn_cast<BinaryOperator>(MatchedExpr->getRHS()->IgnoreImpCasts());
144+
std::optional<BinaryOperatorKind> RHSOpcode;
145+
if (RHS)
146+
RHSOpcode = RHS->getOpcode();
147+
148+
const Expr *SurroundedExpr = nullptr;
149+
if ((MatchedExpr->getOpcode() == BO_Or && ParentOpcode == BO_LAnd) ||
150+
(MatchedExpr->getOpcode() == BO_And &&
151+
llvm::is_contained({BO_Xor, BO_Or}, ParentOpcode))) {
152+
const Expr *Side = Parent->getLHS()->IgnoreParenImpCasts() == MatchedExpr
153+
? Parent->getLHS()
154+
: Parent->getRHS();
155+
SurroundedExpr = Side->IgnoreImpCasts();
156+
assert(SurroundedExpr->IgnoreParens() == MatchedExpr);
157+
} else if (MatchedExpr->getOpcode() == BO_AndAssign && RHSOpcode == BO_LOr)
158+
SurroundedExpr = RHS;
159+
160+
if (SurroundedExpr && isa<ParenExpr>(SurroundedExpr))
161+
SurroundedExpr = nullptr;
162+
163+
FixItHint InsertBrace1;
164+
FixItHint InsertBrace2;
165+
if (SurroundedExpr) {
166+
const SourceLocation InsertFirstLoc = SurroundedExpr->getBeginLoc();
167+
const SourceLocation InsertSecondLoc = clang::Lexer::getLocForEndOfToken(
168+
SurroundedExpr->getEndLoc(), 0, *Result.SourceManager,
169+
Result.Context->getLangOpts());
170+
if (InsertFirstLoc.isInvalid() || InsertFirstLoc.isMacroID() ||
171+
InsertSecondLoc.isInvalid() || InsertSecondLoc.isMacroID())
172+
return static_cast<void>(IgnoreMacros || DiagEmitter());
173+
InsertBrace1 = FixItHint::CreateInsertion(InsertFirstLoc, "(");
174+
InsertBrace2 = FixItHint::CreateInsertion(InsertSecondLoc, ")");
175+
}
176+
177+
DiagEmitter() << InsertEqual << ReplaceOperator << InsertBrace1
178+
<< InsertBrace2;
179+
}
180+
181+
} // namespace clang::tidy::misc
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
//===--- BoolBitwiseOperationCheck.h - clang-tidy ---------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOLBITWISEOPERATIONCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOLBITWISEOPERATIONCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang::tidy::misc {
15+
16+
/// Finds potentially inefficient use of bitwise operators such as ``&``, ``|``
17+
/// and their compound analogues on Boolean values where logical operators like
18+
/// ``&&`` and ``||`` would be more appropriate.
19+
///
20+
/// For the user-facing documentation see:
21+
/// http://clang.llvm.org/extra/clang-tidy/checks/misc/bool-bitwise-operation.html
22+
class BoolBitwiseOperationCheck : public ClangTidyCheck {
23+
public:
24+
BoolBitwiseOperationCheck(StringRef Name, ClangTidyContext *Context);
25+
void storeOptions(ClangTidyOptions::OptionMap &Options) override;
26+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
27+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
28+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
29+
return LangOpts.CPlusPlus || LangOpts.C99;
30+
}
31+
std::optional<TraversalKind> getCheckTraversalKind() const override {
32+
return TK_IgnoreUnlessSpelledInSource;
33+
}
34+
35+
private:
36+
bool StrictMode;
37+
bool IgnoreMacros;
38+
};
39+
40+
} // namespace clang::tidy::misc
41+
42+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_BOOLBITWISEOPERATIONCHECK_H

clang-tools-extra/clang-tidy/misc/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ add_custom_target(genconfusable DEPENDS Confusables.inc)
1818
set_target_properties(genconfusable PROPERTIES FOLDER "Clang Tools Extra/Sourcegenning")
1919

2020
add_clang_library(clangTidyMiscModule STATIC
21+
BoolBitwiseOperationCheck.cpp
2122
ConstCorrectnessCheck.cpp
2223
CoroutineHostileRAIICheck.cpp
2324
DefinitionsInHeadersCheck.cpp

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "../ClangTidy.h"
1010
#include "../ClangTidyModule.h"
1111
#include "../ClangTidyModuleRegistry.h"
12+
#include "BoolBitwiseOperationCheck.h"
1213
#include "ConfusableIdentifierCheck.h"
1314
#include "ConstCorrectnessCheck.h"
1415
#include "CoroutineHostileRAIICheck.h"
@@ -40,6 +41,8 @@ namespace misc {
4041
class MiscModule : public ClangTidyModule {
4142
public:
4243
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
44+
CheckFactories.registerCheck<BoolBitwiseOperationCheck>(
45+
"misc-bool-bitwise-operation");
4346
CheckFactories.registerCheck<ConfusableIdentifierCheck>(
4447
"misc-confusable-identifiers");
4548
CheckFactories.registerCheck<ConstCorrectnessCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,13 @@ New checks
210210
Finds calls to STL library iterator algorithms that could be replaced with
211211
LLVM range-based algorithms from ``llvm/ADT/STLExtras.h``.
212212

213+
- New :doc:`misc-bool-bitwise-operation
214+
<clang-tidy/checks/misc/bool-bitwise-operation>` check.
215+
216+
Finds potentially inefficient use of bitwise operators such as ``&``, ``|``
217+
and their compound analogues on Boolean values where logical operators like
218+
``&&`` and ``||`` would be more appropriate.
219+
213220
- New :doc:`misc-override-with-different-visibility
214221
<clang-tidy/checks/misc/override-with-different-visibility>` check.
215222

clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,9 @@ Clang-Tidy Checks
174174
:doc:`bugprone-use-after-move <bugprone/use-after-move>`,
175175
:doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes"
176176
:doc:`cert-dcl58-cpp <cert/dcl58-cpp>`,
177+
:doc:`cert-env33-c <cert/env33-c>`,
177178
:doc:`cert-err33-c <cert/err33-c>`,
179+
:doc:`cert-err52-cpp <cert/err52-cpp>`,
178180
:doc:`cert-err60-cpp <cert/err60-cpp>`,
179181
:doc:`cert-flp30-c <cert/flp30-c>`,
180182
:doc:`cert-mem57-cpp <cert/mem57-cpp>`,
@@ -261,6 +263,7 @@ Clang-Tidy Checks
261263
:doc:`llvmlibc-implementation-in-namespace <llvmlibc/implementation-in-namespace>`,
262264
:doc:`llvmlibc-inline-function-decl <llvmlibc/inline-function-decl>`, "Yes"
263265
:doc:`llvmlibc-restrict-system-libc-headers <llvmlibc/restrict-system-libc-headers>`, "Yes"
266+
:doc:`misc-bool-bitwise-operation <misc/bool-bitwise-operation>`, "Yes"
264267
:doc:`misc-confusable-identifiers <misc/confusable-identifiers>`,
265268
:doc:`misc-const-correctness <misc/const-correctness>`, "Yes"
266269
:doc:`misc-coroutine-hostile-raii <misc/coroutine-hostile-raii>`,
@@ -372,7 +375,7 @@ Clang-Tidy Checks
372375
:doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`,
373376
:doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, "Yes"
374377
:doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`,
375-
:doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes"
378+
:doc:`readability-braces-around-statements <readability/braces-around-statements>`,
376379
:doc:`readability-const-return-type <readability/const-return-type>`, "Yes"
377380
:doc:`readability-container-contains <readability/container-contains>`, "Yes"
378381
:doc:`readability-container-data-pointer <readability/container-data-pointer>`, "Yes"
@@ -442,9 +445,7 @@ Check aliases
442445
:doc:`cert-dcl54-cpp <cert/dcl54-cpp>`, :doc:`misc-new-delete-overloads <misc/new-delete-overloads>`,
443446
:doc:`cert-dcl59-cpp <cert/dcl59-cpp>`, :doc:`google-build-namespaces <google/build-namespaces>`,
444447
:doc:`cert-err09-cpp <cert/err09-cpp>`, :doc:`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference>`,
445-
:doc:`cert-env33-c <cert/env33-c>`, :doc:`bugprone-command-processor <bugprone/command-processor>`,
446448
:doc:`cert-err34-c <cert/err34-c>`, :doc:`bugprone-unchecked-string-to-number-conversion <bugprone/unchecked-string-to-number-conversion>`,
447-
:doc:`cert-err52-cpp <cert/err52-cpp>`, :doc:`modernize-avoid-setjmp-longjmp <modernize/avoid-setjmp-longjmp>`,
448449
:doc:`cert-err58-cpp <cert/err58-cpp>`, :doc:`bugprone-throwing-static-initialization <bugprone/throwing-static-initialization>`,
449450
:doc:`cert-err61-cpp <cert/err61-cpp>`, :doc:`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference>`,
450451
:doc:`cert-exp42-c <cert/exp42-c>`, :doc:`bugprone-suspicious-memory-comparison <bugprone/suspicious-memory-comparison>`,
@@ -571,12 +572,12 @@ Check aliases
571572
:doc:`cppcoreguidelines-non-private-member-variables-in-classes <cppcoreguidelines/non-private-member-variables-in-classes>`, :doc:`misc-non-private-member-variables-in-classes <misc/non-private-member-variables-in-classes>`,
572573
:doc:`cppcoreguidelines-use-default-member-init <cppcoreguidelines/use-default-member-init>`, :doc:`modernize-use-default-member-init <modernize/use-default-member-init>`, "Yes"
573574
:doc:`fuchsia-header-anon-namespaces <fuchsia/header-anon-namespaces>`, :doc:`google-build-namespaces <google/build-namespaces>`,
574-
:doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes"
575+
:doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`,
575576
:doc:`google-readability-function-size <google/readability-function-size>`, :doc:`readability-function-size <readability/function-size>`,
576577
:doc:`google-readability-namespace-comments <google/readability-namespace-comments>`, :doc:`llvm-namespace-comment <llvm/namespace-comment>`,
577578
:doc:`hicpp-avoid-c-arrays <hicpp/avoid-c-arrays>`, :doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`,
578579
:doc:`hicpp-avoid-goto <hicpp/avoid-goto>`, :doc:`cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto>`,
579-
:doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes"
580+
:doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`,
580581
:doc:`hicpp-deprecated-headers <hicpp/deprecated-headers>`, :doc:`modernize-deprecated-headers <modernize/deprecated-headers>`, "Yes"
581582
:doc:`hicpp-explicit-conversions <hicpp/explicit-conversions>`, :doc:`google-explicit-constructor <google/explicit-constructor>`, "Yes"
582583
:doc:`hicpp-function-size <hicpp/function-size>`, :doc:`readability-function-size <readability/function-size>`,
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
.. title:: clang-tidy - misc-bool-bitwise-operation
2+
3+
misc-bool-bitwise-operation
4+
===========================
5+
6+
Finds potentially inefficient use of bitwise operators such as ``&``, ``|``
7+
and their compound analogues on Boolean values where logical operators like
8+
``&&`` and ``||`` would be more appropriate.
9+
10+
Bitwise operations on Booleans can incur unnecessary performance overhead due
11+
to implicit integer conversions and missed short-circuit evaluation.
12+
13+
.. code-block:: c++
14+
15+
bool invalid = false;
16+
invalid |= x > limit.x; // warning: use logical operator '||' for boolean variable 'invalid' instead of bitwise operator '|='
17+
invalid |= y > limit.y; // warning: use logical operator '||' for boolean variable 'invalid' instead of bitwise operator '|='
18+
invalid |= z > limit.z; // warning: use logical operator '||' for boolean variable 'invalid' instead of bitwise operator '|='
19+
if (invalid) {
20+
// error handling
21+
}
22+
23+
These 3 warnings suggest assigning the result of a logical ``||`` operation
24+
instead of using the ``|=`` operator:
25+
26+
.. code-block:: c++
27+
28+
bool invalid = false;
29+
invalid = invalid || x > limit.x;
30+
invalid = invalid || y > limit.x;
31+
invalid = invalid || z > limit.z;
32+
if (invalid) {
33+
// error handling
34+
}
35+
36+
Limitations
37+
-----------
38+
39+
* Bitwise operators inside templates aren't matched.
40+
41+
.. code-block:: c++
42+
43+
template <typename X>
44+
void f(X a, X b) {
45+
a | b;
46+
}
47+
48+
// even 'f(true, false)' (or similar) won't trigger the warning.
49+
50+
Options
51+
-------
52+
53+
.. option:: StrictMode
54+
55+
Enabling this option promotes more fix-it hints even when they might
56+
change evaluation order or skip side effects. Default value is `false`.
57+
58+
.. option:: IgnoreMacros
59+
60+
This option disables the warning if a macro inside the expression body
61+
prevents replacing a bitwise operator with a logical one. Default value
62+
is `false`.

0 commit comments

Comments
 (0)