[clang-tools-extra] [clang-tidy] Add performance-bool-bitwise-operation check (PR #142324)
Denis Mikhailov via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 1 17:00:33 PDT 2025
https://github.com/denzor200 created https://github.com/llvm/llvm-project/pull/142324
Add new clang-tidy check that finds potentially inefficient use of bitwise operators such as ``&``, ``|`` and their compound analogues on boolean values where logical operators like ``&&`` and ``||`` would be more appropriate.
Bitwise operations on booleans can incur unnecessary performance overhead due to implicit integer conversions and missed short-circuit evaluation.
Small example:
```
bool invalid = false;
invalid |= x > limit.x; // warning: use logical operator instead of bitwise one for bool
invalid |= y > limit.y; // warning: use logical operator instead of bitwise one for bool
invalid |= z > limit.z; // warning: use logical operator instead of bitwise one for bool
if (invalid) {
// error handling
}
```
Fixes: https://github.com/llvm/llvm-project/issues/40307
>From abf12a9f8bf8c8c9826094fcbf20674ddb18b7fd Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Fri, 30 May 2025 00:41:02 +0300
Subject: [PATCH 01/12] Firs step for bool-bitwise-operation-check
---
.../performance/BoolBitwiseOperationCheck.cpp | 129 ++++++++++++++
.../performance/BoolBitwiseOperationCheck.h | 33 ++++
.../clang-tidy/performance/CMakeLists.txt | 1 +
.../performance/PerformanceTidyModule.cpp | 3 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 +
.../docs/clang-tidy/checks/list.rst | 1 +
.../performance/bool-bitwise-operation.rst | 6 +
.../performance/bool-bitwise-operation.cpp | 167 ++++++++++++++++++
8 files changed, 345 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
new file mode 100644
index 0000000000000..1147e2186e6f6
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -0,0 +1,129 @@
+//===--- BoolBitwiseOperationCheck.cpp - clang-tidy -----------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "BoolBitwiseOperationCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::performance {
+namespace {
+std::string getSpellingOpcode(const BinaryOperator &Expr, const SourceManager &SM,
+ const clang::LangOptions &LO) {
+ SourceLocation Loc = Expr.getOperatorLoc();
+ if (Loc.isValid()) {
+ // TODO: bear it in mind
+ // SourceLocation expansionLoc = Result.SourceManager->getExpansionLoc(Loc);
+ // if (expansionLoc.isValid()) {
+ Loc = SM.getSpellingLoc(Loc);
+ if (Loc.isValid() && !Loc.isMacroID()) {
+ const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
+ if (TokenRange.isValid()) {
+ return Lexer::getSourceText(TokenRange, SM, LO).str();
+ }
+ }
+ // }
+ }
+ return "";
+}
+
+std::string changeOpcode(llvm::StringRef Spelling) {
+ if (Spelling == "|" || Spelling == "|=")
+ return "||";
+ else if (Spelling == "&" || Spelling == "&=")
+ return "&&";
+ else if (Spelling == "bitand" || Spelling == "and_eq")
+ return "and";
+ else if (Spelling == "bitor" || Spelling == "or_eq")
+ return "or";
+ return Spelling.str();
+}
+}
+
+void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(binaryOperator(
+ unless(isExpansionInSystemHeader()),
+ hasAnyOperatorName("|", "&", "|=", "&="),
+ hasEitherOperand(expr(ignoringImpCasts(hasType(booleanType())))),
+ optionally(hasEitherOperand(
+ expr(ignoringImpCasts(hasType(isVolatileQualified())))
+ .bind("vol"))),
+ optionally(hasAncestor(
+ binaryOperator().bind("p"))), // TODO: check operator name later
+ optionally(hasRHS(ignoringParenCasts(
+ binaryOperator().bind("r")))), // TODO: check operator name later
+ optionally(hasLHS(ignoringParenCasts(
+ declRefExpr().bind("l")
+ )))
+ )
+ .bind("op"), this);
+}
+
+// TODO: test for nontraditional xor
+void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedExpr = Result.Nodes.getNodeAs<BinaryOperator>("op");
+ const SourceManager &SM = *Result.SourceManager;
+ const clang::LangOptions &LO = Result.Context->getLangOpts();
+
+ auto Diag = diag(MatchedExpr->getOperatorLoc(), "use logical operator instead of bitwise one for bool");
+
+ const auto *VolatileOperand = Result.Nodes.getNodeAs<Expr>("vol");
+ if (VolatileOperand)
+ return;
+
+ SourceLocation Loc = MatchedExpr->getOperatorLoc();
+ if (Loc.isValid()) {
+ // TODO: bear it in mind
+ // SourceLocation expansionLoc = Result.SourceManager->getExpansionLoc(Loc);
+ // if (expansionLoc.isValid()) {
+ Loc = SM.getSpellingLoc(Loc);
+ if (Loc.isValid() && !Loc.isMacroID()) {
+ const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
+ if (TokenRange.isValid()) {
+ const StringRef SpellingOpc = Lexer::getSourceText(TokenRange, SM, LO);
+ if (SpellingOpc == "&=" || SpellingOpc == "|=" || SpellingOpc == "and_eq" || SpellingOpc == "or_eq") {
+ const auto *DelcRefLHS = Result.Nodes.getNodeAs<DeclRefExpr>("l");
+ if (!DelcRefLHS)
+ return;
+ const clang::SourceLocation EndLoc = // TODO: naming
+ clang::Lexer::getLocForEndOfToken(DelcRefLHS->getEndLoc(), 0,
+ SM, LO);
+ if (EndLoc.isInvalid()) {
+ return;
+ }
+ Diag << FixItHint::CreateInsertion(EndLoc,
+ " = " + DelcRefLHS->getDecl()->getNameAsString());
+ }
+ Diag << FixItHint::CreateReplacement(TokenRange, changeOpcode(SpellingOpc));
+ const auto *Parent = Result.Nodes.getNodeAs<BinaryOperator>("p");
+ const auto *RHS = Result.Nodes.getNodeAs<BinaryOperator>("r");
+ // TODO: here must be not spelling ops
+ const std::string ParentSpellingOpc = Parent ? getSpellingOpcode(*Parent, SM, LO) : "";
+ const std::string RightSpellingOpc = RHS ? getSpellingOpcode(*RHS, SM, LO) : "";
+ if ((SpellingOpc == "|" && ParentSpellingOpc == "&&") ||
+ (SpellingOpc == "&" && ParentSpellingOpc == "^")) {
+ const clang::SourceLocation StartLoc = MatchedExpr->getBeginLoc();
+ const clang::SourceLocation EndLoc = // TODO: check for valid
+ clang::Lexer::getLocForEndOfToken(MatchedExpr->getEndLoc(), 0, SM, LO);
+ Diag << FixItHint::CreateInsertion(StartLoc, "(")
+ << FixItHint::CreateInsertion(EndLoc, ")");
+ } else if (SpellingOpc == "&=" && RightSpellingOpc == "||") {
+ const clang::SourceLocation StartLoc = RHS->getBeginLoc();
+ const clang::SourceLocation EndLoc = // TODO: check for valid
+ clang::Lexer::getLocForEndOfToken(RHS->getEndLoc(), 0, SM, LO);
+ Diag << FixItHint::CreateInsertion(StartLoc, "(")
+ << FixItHint::CreateInsertion(EndLoc, ")");
+ }
+ }
+ }
+ // }
+ }
+}
+
+} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
new file mode 100644
index 0000000000000..d3a348116bf24
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
@@ -0,0 +1,33 @@
+//===--- BoolBitwiseOperationCheck.h - clang-tidy ---------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_BOOLBITWISEOPERATIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_BOOLBITWISEOPERATIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::performance {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance/bool-bitwise-operation.html
+class BoolBitwiseOperationCheck : public ClangTidyCheck {
+public:
+ BoolBitwiseOperationCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+};
+
+} // namespace clang::tidy::performance
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_BOOLBITWISEOPERATIONCHECK_H
diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index c6e547c5089fb..5a600e988d65e 100644
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyPerformanceModule STATIC
AvoidEndlCheck.cpp
+ BoolBitwiseOperationCheck.cpp
EnumSizeCheck.cpp
FasterStringFindCheck.cpp
ForRangeCopyCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 9e0fa6f88b36a..971951213aac1 100644
--- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -10,6 +10,7 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "AvoidEndlCheck.h"
+#include "BoolBitwiseOperationCheck.h"
#include "EnumSizeCheck.h"
#include "FasterStringFindCheck.h"
#include "ForRangeCopyCheck.h"
@@ -36,6 +37,8 @@ class PerformanceModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<AvoidEndlCheck>("performance-avoid-endl");
+ CheckFactories.registerCheck<BoolBitwiseOperationCheck>(
+ "performance-bool-bitwise-operation");
CheckFactories.registerCheck<EnumSizeCheck>("performance-enum-size");
CheckFactories.registerCheck<FasterStringFindCheck>(
"performance-faster-string-find");
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e0f81a032c38d..183038598235b 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -136,6 +136,11 @@ New checks
Finds unintended character output from ``unsigned char`` and ``signed char``
to an ``ostream``.
+- New :doc:`performance-bool-bitwise-operation
+ <clang-tidy/checks/performance/bool-bitwise-operation>` check.
+
+ FIXME: Write a short description.
+
- New :doc:`portability-avoid-pragma-once
<clang-tidy/checks/portability/avoid-pragma-once>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5a79d61b1fd7e..19f96afb06cc0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -333,6 +333,7 @@ Clang-Tidy Checks
:doc:`openmp-exception-escape <openmp/exception-escape>`,
:doc:`openmp-use-default-none <openmp/use-default-none>`,
:doc:`performance-avoid-endl <performance/avoid-endl>`, "Yes"
+ :doc:`performance-bool-bitwise-operation <performance/bool-bitwise-operation>`, "Yes"
:doc:`performance-enum-size <performance/enum-size>`,
:doc:`performance-faster-string-find <performance/faster-string-find>`, "Yes"
:doc:`performance-for-range-copy <performance/for-range-copy>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
new file mode 100644
index 0000000000000..9b236f95504ad
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - performance-bool-bitwise-operation
+
+performance-bool-bitwise-operation
+==================================
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
new file mode 100644
index 0000000000000..62122d6f7516d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
@@ -0,0 +1,167 @@
+// RUN: %check_clang_tidy %s performance-bool-bitwise-operation %t
+
+bool& normal() {
+ int a = 100, b = 200;
+
+ a | b;
+ a & b;
+ a |= b;
+ a &= b;
+
+ a bitor b;
+ a bitand b;
+ a or_eq b;
+ a and_eq b;
+
+ static bool st = false;
+ return st;
+}
+
+void bad() {
+ bool a = true, b = false;
+ a | b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || b;
+ a & b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && b;
+ a |= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a || b;
+ a &= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && b;
+}
+
+void bad_volatile_bool() {
+ bool a = true;
+ volatile bool b = false;
+ a | b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a & b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a |= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a &= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+}
+
+void bad_nontraditional() {
+ bool a = true, b = false;
+ a bitor b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or b;
+ a bitand b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and b;
+ a or_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a or b;
+ a and_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a and b;
+}
+
+void bad_with_priors() {
+ bool a = false, b = true, c = true;
+ a && b | c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && (b || c);
+ a && b & c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && b && c;
+ a || b & c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || b && c;
+ a || b | c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || b || c;
+ b | c && a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b || c) && a;
+}
+
+void bad_with_priors2() {
+ bool a = false, b = true, c = true;
+ a ^ b & c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a ^ (b && c);
+ a | b & c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || b && c;
+ b & c ^ a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b && c) ^ a;
+}
+
+void bad_with_priors_compound() {
+ bool a = false, b = true, c = true;
+ a &= b || c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && (b || c);
+ a |= b || c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a || b || c;
+ a &= b && c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && b && c;
+ a |= b && c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a || b && c;
+}
+
+void bad_with_priors_compound2() {
+ // TODO: ^ and |
+}
+
+void bad_no_fixit() {
+ bool b = false;
+ normal() |= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ normal() &= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+}
+
+// TODO: all the same tests as in math-parentheses check
+void bad_in_macro___() {
+#if 0
+ const std::string Input = R"cc(
+#define M(a,b) (true & a) * (true | b)
+ int f() { return M(false, false); }
+ )cc";
+#endif
+// TODO: implement this(make sure no fix were provided)
+#if 0
+ const std::string Input = R"cc(
+#define M(a,b) (true a) * (true b)
+ int f() { return M(& false, | false); }
+ )cc";
+ const std::string Expected = R"cc(
+#define M(a,b) (true a) * (true b)
+ int f() { return M(&& false, || false); }
+ )cc";
+#endif
+// TODO: implement this
+}
+
+template<typename T>
+void good_in_unreachable_template(T a, T b) {
+ a | b;
+ a & b;
+ a |= b;
+ a &= b;
+}
+
+// TODO: test for in template
+// TODO: test for type in typedef
+// TODO: test for expressions in parentheses
+
+
>From 4f70d9fba7659491c3633e9dacd9b148bde5f7fe Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Fri, 30 May 2025 17:12:43 +0300
Subject: [PATCH 02/12] WIP
---
.../performance/BoolBitwiseOperationCheck.cpp | 42 ++++---
.../performance/bool-bitwise-operation.cpp | 105 ++++++++++++++++++
2 files changed, 132 insertions(+), 15 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
index 1147e2186e6f6..b1f30466a3fd9 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -14,6 +14,19 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
namespace {
+bool isFullyInsideMacro(const BinaryOperator *BO, const SourceManager &SM) {
+ SourceLocation Begin = BO->getBeginLoc();
+ SourceLocation End = BO->getEndLoc();
+
+ // Если хотя бы одна часть оператора не в макросе — считаем его "не макросным"
+ if (!Begin.isMacroID() || !End.isMacroID()) {
+ return false;
+ }
+
+ // Проверяем, что начало и конец находятся в одном макросе
+ return SM.getImmediateMacroCallerLoc(Begin) == SM.getImmediateMacroCallerLoc(End);
+}
+
std::string getSpellingOpcode(const BinaryOperator &Expr, const SourceManager &SM,
const clang::LangOptions &LO) {
SourceLocation Loc = Expr.getOperatorLoc();
@@ -55,9 +68,9 @@ void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
expr(ignoringImpCasts(hasType(isVolatileQualified())))
.bind("vol"))),
optionally(hasAncestor(
- binaryOperator().bind("p"))), // TODO: check operator name later
+ binaryOperator().bind("p"))),
optionally(hasRHS(ignoringParenCasts(
- binaryOperator().bind("r")))), // TODO: check operator name later
+ binaryOperator().bind("r")))),
optionally(hasLHS(ignoringParenCasts(
declRefExpr().bind("l")
)))
@@ -65,7 +78,6 @@ void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
.bind("op"), this);
}
-// TODO: test for nontraditional xor
void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
const auto *MatchedExpr = Result.Nodes.getNodeAs<BinaryOperator>("op");
const SourceManager &SM = *Result.SourceManager;
@@ -74,19 +86,20 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
auto Diag = diag(MatchedExpr->getOperatorLoc(), "use logical operator instead of bitwise one for bool");
const auto *VolatileOperand = Result.Nodes.getNodeAs<Expr>("vol");
- if (VolatileOperand)
+ if (VolatileOperand || isFullyInsideMacro(MatchedExpr, SM))
return;
SourceLocation Loc = MatchedExpr->getOperatorLoc();
if (Loc.isValid()) {
// TODO: bear it in mind
- // SourceLocation expansionLoc = Result.SourceManager->getExpansionLoc(Loc);
- // if (expansionLoc.isValid()) {
- Loc = SM.getSpellingLoc(Loc);
- if (Loc.isValid() && !Loc.isMacroID()) {
- const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
+ //SourceLocation expansionLoc = Result.SourceManager->getExpansionLoc(Loc);
+ SourceLocation expansionLoc = Loc;
+ if (expansionLoc.isValid()) {
+ expansionLoc = SM.getFileLoc(expansionLoc);
+ if (expansionLoc.isValid() && !expansionLoc.isMacroID()) {
+ const CharSourceRange TokenRange = CharSourceRange::getTokenRange(expansionLoc);
if (TokenRange.isValid()) {
- const StringRef SpellingOpc = Lexer::getSourceText(TokenRange, SM, LO);
+ const std::string SpellingOpc = getSpellingOpcode(*MatchedExpr, SM, LO);
if (SpellingOpc == "&=" || SpellingOpc == "|=" || SpellingOpc == "and_eq" || SpellingOpc == "or_eq") {
const auto *DelcRefLHS = Result.Nodes.getNodeAs<DeclRefExpr>("l");
if (!DelcRefLHS)
@@ -103,17 +116,16 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
Diag << FixItHint::CreateReplacement(TokenRange, changeOpcode(SpellingOpc));
const auto *Parent = Result.Nodes.getNodeAs<BinaryOperator>("p");
const auto *RHS = Result.Nodes.getNodeAs<BinaryOperator>("r");
- // TODO: here must be not spelling ops
const std::string ParentSpellingOpc = Parent ? getSpellingOpcode(*Parent, SM, LO) : "";
const std::string RightSpellingOpc = RHS ? getSpellingOpcode(*RHS, SM, LO) : "";
- if ((SpellingOpc == "|" && ParentSpellingOpc == "&&") ||
- (SpellingOpc == "&" && ParentSpellingOpc == "^")) {
+ if (((SpellingOpc == "|" || SpellingOpc == "bitor") && (ParentSpellingOpc == "&&" || ParentSpellingOpc == "and")) ||
+ ((SpellingOpc == "&" || SpellingOpc == "bitand") && (ParentSpellingOpc == "^" || ParentSpellingOpc == "xor"))) {
const clang::SourceLocation StartLoc = MatchedExpr->getBeginLoc();
const clang::SourceLocation EndLoc = // TODO: check for valid
clang::Lexer::getLocForEndOfToken(MatchedExpr->getEndLoc(), 0, SM, LO);
Diag << FixItHint::CreateInsertion(StartLoc, "(")
<< FixItHint::CreateInsertion(EndLoc, ")");
- } else if (SpellingOpc == "&=" && RightSpellingOpc == "||") {
+ } else if ((SpellingOpc == "&=" || SpellingOpc == "and_eq") && (RightSpellingOpc == "||" || RightSpellingOpc == "or")) {
const clang::SourceLocation StartLoc = RHS->getBeginLoc();
const clang::SourceLocation EndLoc = // TODO: check for valid
clang::Lexer::getLocForEndOfToken(RHS->getEndLoc(), 0, SM, LO);
@@ -122,7 +134,7 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
}
}
}
- // }
+ }
}
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
index 62122d6f7516d..68ce50584a400 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
@@ -120,6 +120,55 @@ void bad_with_priors_compound2() {
// TODO: ^ and |
}
+void bad_with_priors_nontraditional() {
+ bool a = false, b = true, c = true;
+ a and b bitor c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and (b or c);
+ a and b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and b and c;
+ a or b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or b and c;
+ a or b bitor c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or b or c;
+ b bitor c and a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b or c) and a;
+}
+
+void bad_with_priors2_nontraditional() {
+ bool a = false, b = true, c = true;
+ a xor b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a xor (b and c);
+ a bitor b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or b and c;
+ b bitand c xor a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b and c) xor a;
+}
+
+void bad_with_priors_compound_nontraditional() {
+ bool a = false, b = true, c = true;
+ a and_eq b or c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a and (b or c);
+ a or_eq b or c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a or b or c;
+ a and_eq b and c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a and b and c;
+ a or_eq b and c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a or b and c;
+}
+
void bad_no_fixit() {
bool b = false;
normal() |= b;
@@ -130,6 +179,62 @@ void bad_no_fixit() {
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
}
+// TODO: nontraditional
+#define MY_OR |
+#define MY_AND &
+#define MY_OR_ASSIGN |=
+#define MY_AND_ASSIGN &=
+
+#define MY_OR_FUNC(a, b) ((a) | (b))
+#define MY_AND_FUNC(a, b) ((a) & (b))
+#define MY_OR_ASSIGN_FUNC(a, b) ((a) |= (b))
+#define MY_AND_ASSIGN_FUNC(a, b) ((a) &= (b))
+
+#define IDENT(a) (a)
+
+// TODO: check that braces will not be settled inside the macro
+// TODO: check that =a will not be sellted inside the macro(for both cases)
+
+void bad_in_macro() {
+ bool a = true, b = false;
+
+ // TODO: same for braces as in math check
+
+ a MY_OR b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || b;
+ a MY_AND b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && b;
+ a MY_OR_ASSIGN b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a || b;
+ a MY_AND_ASSIGN b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && b;
+
+ // TODO: same but for partial(with hints and with no hints)
+ MY_OR_FUNC(a, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ MY_AND_FUNC(a, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ MY_OR_ASSIGN_FUNC(a, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ MY_AND_ASSIGN_FUNC(a, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ IDENT(a | b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: IDENT(a || b);
+ IDENT(a & b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: IDENT(a && b);
+}
+
// TODO: all the same tests as in math-parentheses check
void bad_in_macro___() {
#if 0
>From 0d59bf97ca32e3341d7aae6b864d951a513617b4 Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Fri, 30 May 2025 23:22:00 +0300
Subject: [PATCH 03/12] Simplify a lot
---
.../performance/BoolBitwiseOperationCheck.cpp | 165 +++++++++---------
.../performance/bool-bitwise-operation.cpp | 97 ++++++----
2 files changed, 144 insertions(+), 118 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
index b1f30466a3fd9..ebd05b4d6cfd1 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -9,53 +9,32 @@
#include "BoolBitwiseOperationCheck.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
+#include <array>
+#include <utility>
+#include <optional>
using namespace clang::ast_matchers;
namespace clang::tidy::performance {
namespace {
-bool isFullyInsideMacro(const BinaryOperator *BO, const SourceManager &SM) {
- SourceLocation Begin = BO->getBeginLoc();
- SourceLocation End = BO->getEndLoc();
-
- // Если хотя бы одна часть оператора не в макросе — считаем его "не макросным"
- if (!Begin.isMacroID() || !End.isMacroID()) {
- return false;
- }
-
- // Проверяем, что начало и конец находятся в одном макросе
- return SM.getImmediateMacroCallerLoc(Begin) == SM.getImmediateMacroCallerLoc(End);
-}
-std::string getSpellingOpcode(const BinaryOperator &Expr, const SourceManager &SM,
- const clang::LangOptions &LO) {
- SourceLocation Loc = Expr.getOperatorLoc();
- if (Loc.isValid()) {
- // TODO: bear it in mind
- // SourceLocation expansionLoc = Result.SourceManager->getExpansionLoc(Loc);
- // if (expansionLoc.isValid()) {
- Loc = SM.getSpellingLoc(Loc);
- if (Loc.isValid() && !Loc.isMacroID()) {
- const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
- if (TokenRange.isValid()) {
- return Lexer::getSourceText(TokenRange, SM, LO).str();
- }
- }
- // }
+constexpr std::array<std::pair<llvm::StringRef, llvm::StringRef>, 8U>
+ OperatorsTransformation{{{"|", "||"},
+ {"|=", "||"},
+ {"&", "&&"},
+ {"&=", "&&"},
+ {"bitand", "and"},
+ {"and_eq", "and"},
+ {"bitor", "or"},
+ {"or_eq", "or"}}};
+
+llvm::StringRef translate(llvm::StringRef Value) {
+ for (const auto &[Bitwise, Logical] : OperatorsTransformation) {
+ if (Value == Bitwise)
+ return Logical;
}
- return "";
-}
-std::string changeOpcode(llvm::StringRef Spelling) {
- if (Spelling == "|" || Spelling == "|=")
- return "||";
- else if (Spelling == "&" || Spelling == "&=")
- return "&&";
- else if (Spelling == "bitand" || Spelling == "and_eq")
- return "and";
- else if (Spelling == "bitor" || Spelling == "or_eq")
- return "or";
- return Spelling.str();
+ return {};
}
}
@@ -80,61 +59,75 @@ void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
const auto *MatchedExpr = Result.Nodes.getNodeAs<BinaryOperator>("op");
- const SourceManager &SM = *Result.SourceManager;
- const clang::LangOptions &LO = Result.Context->getLangOpts();
auto Diag = diag(MatchedExpr->getOperatorLoc(), "use logical operator instead of bitwise one for bool");
const auto *VolatileOperand = Result.Nodes.getNodeAs<Expr>("vol");
- if (VolatileOperand || isFullyInsideMacro(MatchedExpr, SM))
+ if (VolatileOperand)
return;
SourceLocation Loc = MatchedExpr->getOperatorLoc();
- if (Loc.isValid()) {
- // TODO: bear it in mind
- //SourceLocation expansionLoc = Result.SourceManager->getExpansionLoc(Loc);
- SourceLocation expansionLoc = Loc;
- if (expansionLoc.isValid()) {
- expansionLoc = SM.getFileLoc(expansionLoc);
- if (expansionLoc.isValid() && !expansionLoc.isMacroID()) {
- const CharSourceRange TokenRange = CharSourceRange::getTokenRange(expansionLoc);
- if (TokenRange.isValid()) {
- const std::string SpellingOpc = getSpellingOpcode(*MatchedExpr, SM, LO);
- if (SpellingOpc == "&=" || SpellingOpc == "|=" || SpellingOpc == "and_eq" || SpellingOpc == "or_eq") {
- const auto *DelcRefLHS = Result.Nodes.getNodeAs<DeclRefExpr>("l");
- if (!DelcRefLHS)
- return;
- const clang::SourceLocation EndLoc = // TODO: naming
- clang::Lexer::getLocForEndOfToken(DelcRefLHS->getEndLoc(), 0,
- SM, LO);
- if (EndLoc.isInvalid()) {
- return;
- }
- Diag << FixItHint::CreateInsertion(EndLoc,
- " = " + DelcRefLHS->getDecl()->getNameAsString());
- }
- Diag << FixItHint::CreateReplacement(TokenRange, changeOpcode(SpellingOpc));
- const auto *Parent = Result.Nodes.getNodeAs<BinaryOperator>("p");
- const auto *RHS = Result.Nodes.getNodeAs<BinaryOperator>("r");
- const std::string ParentSpellingOpc = Parent ? getSpellingOpcode(*Parent, SM, LO) : "";
- const std::string RightSpellingOpc = RHS ? getSpellingOpcode(*RHS, SM, LO) : "";
- if (((SpellingOpc == "|" || SpellingOpc == "bitor") && (ParentSpellingOpc == "&&" || ParentSpellingOpc == "and")) ||
- ((SpellingOpc == "&" || SpellingOpc == "bitand") && (ParentSpellingOpc == "^" || ParentSpellingOpc == "xor"))) {
- const clang::SourceLocation StartLoc = MatchedExpr->getBeginLoc();
- const clang::SourceLocation EndLoc = // TODO: check for valid
- clang::Lexer::getLocForEndOfToken(MatchedExpr->getEndLoc(), 0, SM, LO);
- Diag << FixItHint::CreateInsertion(StartLoc, "(")
- << FixItHint::CreateInsertion(EndLoc, ")");
- } else if ((SpellingOpc == "&=" || SpellingOpc == "and_eq") && (RightSpellingOpc == "||" || RightSpellingOpc == "or")) {
- const clang::SourceLocation StartLoc = RHS->getBeginLoc();
- const clang::SourceLocation EndLoc = // TODO: check for valid
- clang::Lexer::getLocForEndOfToken(RHS->getEndLoc(), 0, SM, LO);
- Diag << FixItHint::CreateInsertion(StartLoc, "(")
- << FixItHint::CreateInsertion(EndLoc, ")");
- }
- }
- }
+
+ // TODO: not only operator must be checked for a macro
+ if (Loc.isInvalid() || Loc.isMacroID())
+ return;
+
+ Loc = Result.SourceManager->getSpellingLoc(Loc);
+ if (Loc.isInvalid() || Loc.isMacroID())
+ return;
+
+ const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
+ if (TokenRange.isInvalid())
+ return;
+
+ StringRef Spelling = Lexer::getSourceText(TokenRange, *Result.SourceManager,
+ Result.Context->getLangOpts());
+ StringRef TranslatedSpelling = translate(Spelling);
+
+ if (TranslatedSpelling.empty())
+ return;
+
+ std::string FixSpelling = TranslatedSpelling.str();
+
+ if (MatchedExpr->isCompoundAssignmentOp()) {
+ const auto *DelcRefLHS = Result.Nodes.getNodeAs<DeclRefExpr>("l");
+ if (!DelcRefLHS)
+ return;
+ const clang::SourceLocation InsertLoc =
+ clang::Lexer::getLocForEndOfToken(DelcRefLHS->getEndLoc(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts());
+ if (InsertLoc.isInvalid()) {
+ return;
}
+ Diag << FixItHint::CreateInsertion(InsertLoc,
+ " = " + DelcRefLHS->getDecl()->getNameAsString());
+ }
+ Diag << FixItHint::CreateReplacement(TokenRange, FixSpelling);
+
+ std::optional<BinaryOperatorKind> ParentOpcode;
+ if (const auto *Parent = Result.Nodes.getNodeAs<BinaryOperator>("p"); Parent)
+ ParentOpcode = Parent->getOpcode();
+
+ const auto *RHS = Result.Nodes.getNodeAs<BinaryOperator>("r");
+ std::optional<BinaryOperatorKind> RHSOpcode;
+ if (RHS)
+ RHSOpcode = RHS->getOpcode();
+
+ const BinaryOperator* SurroundedExpr = nullptr;
+ if ((MatchedExpr->getOpcode() == BO_Or && ParentOpcode.has_value() && *ParentOpcode == BO_LAnd) ||
+ (MatchedExpr->getOpcode() == BO_And && ParentOpcode.has_value() && *ParentOpcode == BO_Xor)) {
+ SurroundedExpr = MatchedExpr;
+ } else if (MatchedExpr->getOpcode() == BO_AndAssign && RHSOpcode.has_value() && *RHSOpcode == BO_LOr) {
+ SurroundedExpr = RHS;
+ }
+
+ if (SurroundedExpr) {
+ const SourceLocation InsertFirstLoc = SurroundedExpr->getBeginLoc(); // TODO: check for correct??
+ const SourceLocation InsertSecondLoc = // TODO: check for valid
+ clang::Lexer::getLocForEndOfToken(SurroundedExpr->getEndLoc(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts());
+ Diag << FixItHint::CreateInsertion(InsertFirstLoc, "(")
+ << FixItHint::CreateInsertion(InsertSecondLoc, ")");
}
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
index 68ce50584a400..ecbb184908014 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
@@ -184,6 +184,7 @@ void bad_no_fixit() {
#define MY_AND &
#define MY_OR_ASSIGN |=
#define MY_AND_ASSIGN &=
+#define MY_LOG_AND &&
#define MY_OR_FUNC(a, b) ((a) | (b))
#define MY_AND_FUNC(a, b) ((a) & (b))
@@ -192,28 +193,67 @@ void bad_no_fixit() {
#define IDENT(a) (a)
-// TODO: check that braces will not be settled inside the macro
-// TODO: check that =a will not be sellted inside the macro(for both cases)
+bool global_flag = false;
+int sink(int);
+#define FUN(ARG) (sink(ARG))
+#define FUN2(ARG) sink((ARG))
+#define FUN3(ARG) sink(ARG)
+#define FUN4(ARG) sink(true | ARG)
+#define FUN5(ARG) sink(false && ARG)
+#define FUN6(ARG) sink(global_flag |= ARG)
+#define M1(a,b) (true & a) && (true | b)
+#define M2(a,b) (true a) && (true b)
+
+// FIXME: implement fixit hint for simple macro cases
void bad_in_macro() {
bool a = true, b = false;
- // TODO: same for braces as in math check
-
a MY_OR b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a || b;
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
a MY_AND b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a && b;
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
a MY_OR_ASSIGN b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a = a || b;
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
a MY_AND_ASSIGN b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a = a && b;
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ // TODO: bool q = true MY_AND false MY_OR true MY_AND false MY_OR true MY_AND false MY_AND true MY_OR false;
+
+ bool q = (true MY_LOG_AND false MY_OR true) MY_LOG_AND (false MY_OR true MY_LOG_AND (false MY_LOG_AND true MY_OR false));
+ // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:67: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-3]]:112: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-4]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ bool r = FUN(true | false && true);
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ bool s = FUN2(true | false && true);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ bool t = FUN3(true | false && true);
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ bool u = FUN4(true && false);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ bool v = FUN5(false | true);
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ bool vv = FUN6(true && false);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- // TODO: same but for partial(with hints and with no hints)
MY_OR_FUNC(a, b);
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
@@ -229,32 +269,25 @@ void bad_in_macro() {
IDENT(a | b);
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: IDENT(a || b);
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
IDENT(a & b);
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: IDENT(a && b);
-}
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ bool abc = false;
+ IDENT(abc |= b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ IDENT(abc &= b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
-// TODO: all the same tests as in math-parentheses check
-void bad_in_macro___() {
-#if 0
- const std::string Input = R"cc(
-#define M(a,b) (true & a) * (true | b)
- int f() { return M(false, false); }
- )cc";
-#endif
-// TODO: implement this(make sure no fix were provided)
-#if 0
- const std::string Input = R"cc(
-#define M(a,b) (true a) * (true b)
- int f() { return M(& false, | false); }
- )cc";
- const std::string Expected = R"cc(
-#define M(a,b) (true a) * (true b)
- int f() { return M(&& false, || false); }
- )cc";
-#endif
-// TODO: implement this
+ M1(false, false);
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ M2(& false, | false);
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-3]]:{{.*}}: note: FIX-IT applied suggested code changes
}
template<typename T>
>From 72a7785c3f2be532b8a9cc33e8e3a891053d0998 Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Sat, 31 May 2025 01:32:35 +0300
Subject: [PATCH 04/12] completed tests for macro
---
.../performance/BoolBitwiseOperationCheck.cpp | 29 +++--
.../performance/bool-bitwise-operation.cpp | 116 ++++++------------
2 files changed, 55 insertions(+), 90 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
index ebd05b4d6cfd1..91150f7f1e332 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -68,7 +68,6 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
SourceLocation Loc = MatchedExpr->getOperatorLoc();
- // TODO: not only operator must be checked for a macro
if (Loc.isInvalid() || Loc.isMacroID())
return;
@@ -89,20 +88,24 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
std::string FixSpelling = TranslatedSpelling.str();
+ FixItHint InsertEqual, ReplaceOperator, InsertBrace1, InsertBrace2;
if (MatchedExpr->isCompoundAssignmentOp()) {
const auto *DelcRefLHS = Result.Nodes.getNodeAs<DeclRefExpr>("l");
if (!DelcRefLHS)
return;
- const clang::SourceLocation InsertLoc =
- clang::Lexer::getLocForEndOfToken(DelcRefLHS->getEndLoc(), 0, *Result.SourceManager,
- Result.Context->getLangOpts());
- if (InsertLoc.isInvalid()) {
+ const SourceLocation LocLHS = DelcRefLHS->getEndLoc();
+ if (LocLHS.isInvalid() || LocLHS.isMacroID())
+ return;
+ const SourceLocation InsertLoc = clang::Lexer::getLocForEndOfToken(
+ LocLHS, 0, *Result.SourceManager,
+ Result.Context->getLangOpts());
+ if (InsertLoc.isInvalid() || InsertLoc.isMacroID()) {
return;
}
- Diag << FixItHint::CreateInsertion(InsertLoc,
+ InsertEqual = FixItHint::CreateInsertion(InsertLoc,
" = " + DelcRefLHS->getDecl()->getNameAsString());
}
- Diag << FixItHint::CreateReplacement(TokenRange, FixSpelling);
+ ReplaceOperator = FixItHint::CreateReplacement(TokenRange, FixSpelling);
std::optional<BinaryOperatorKind> ParentOpcode;
if (const auto *Parent = Result.Nodes.getNodeAs<BinaryOperator>("p"); Parent)
@@ -122,13 +125,17 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
}
if (SurroundedExpr) {
- const SourceLocation InsertFirstLoc = SurroundedExpr->getBeginLoc(); // TODO: check for correct??
- const SourceLocation InsertSecondLoc = // TODO: check for valid
+ const SourceLocation InsertFirstLoc = SurroundedExpr->getBeginLoc();
+ const SourceLocation InsertSecondLoc =
clang::Lexer::getLocForEndOfToken(SurroundedExpr->getEndLoc(), 0, *Result.SourceManager,
Result.Context->getLangOpts());
- Diag << FixItHint::CreateInsertion(InsertFirstLoc, "(")
- << FixItHint::CreateInsertion(InsertSecondLoc, ")");
+ if (InsertFirstLoc.isInvalid() || InsertFirstLoc.isMacroID() || InsertSecondLoc.isInvalid() || InsertSecondLoc.isMacroID())
+ return;
+ InsertBrace1 = FixItHint::CreateInsertion(InsertFirstLoc, "(");
+ InsertBrace2 = FixItHint::CreateInsertion(InsertSecondLoc, ")");
}
+
+ Diag << InsertEqual << ReplaceOperator << InsertBrace1 << InsertBrace2;
}
} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
index ecbb184908014..3615569073be2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
@@ -116,10 +116,6 @@ void bad_with_priors_compound() {
// CHECK-FIXES: a = a || b && c;
}
-void bad_with_priors_compound2() {
- // TODO: ^ and |
-}
-
void bad_with_priors_nontraditional() {
bool a = false, b = true, c = true;
a and b bitor c;
@@ -179,36 +175,19 @@ void bad_no_fixit() {
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
}
-// TODO: nontraditional
#define MY_OR |
#define MY_AND &
#define MY_OR_ASSIGN |=
#define MY_AND_ASSIGN &=
#define MY_LOG_AND &&
-#define MY_OR_FUNC(a, b) ((a) | (b))
-#define MY_AND_FUNC(a, b) ((a) & (b))
-#define MY_OR_ASSIGN_FUNC(a, b) ((a) |= (b))
-#define MY_AND_ASSIGN_FUNC(a, b) ((a) &= (b))
-
-#define IDENT(a) (a)
-
-bool global_flag = false;
-int sink(int);
-#define FUN(ARG) (sink(ARG))
-#define FUN2(ARG) sink((ARG))
-#define FUN3(ARG) sink(ARG)
-#define FUN4(ARG) sink(true | ARG)
-#define FUN5(ARG) sink(false && ARG)
-#define FUN6(ARG) sink(global_flag |= ARG)
+#define CAT(a, b) a ## b
+#define IDENT(a) a
-#define M1(a,b) (true & a) && (true | b)
-#define M2(a,b) (true a) && (true b)
-
-// FIXME: implement fixit hint for simple macro cases
void bad_in_macro() {
bool a = true, b = false;
+ // change operator - BAD
a MY_OR b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
@@ -221,73 +200,52 @@ void bad_in_macro() {
a MY_AND_ASSIGN b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
-
- // TODO: bool q = true MY_AND false MY_OR true MY_AND false MY_OR true MY_AND false MY_AND true MY_OR false;
-
- bool q = (true MY_LOG_AND false MY_OR true) MY_LOG_AND (false MY_OR true MY_LOG_AND (false MY_LOG_AND true MY_OR false));
- // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES: :[[@LINE-2]]:67: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES: :[[@LINE-3]]:112: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-4]]:{{.*}}: note: FIX-IT applied suggested code changes
-
- bool r = FUN(true | false && true);
- // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
-
- bool s = FUN2(true | false && true);
- // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
-
- bool t = FUN3(true | false && true);
- // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ IDENT(a &= b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- bool u = FUN4(true && false);
+ // change operator - GOOD
+ IDENT(a) | b;
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ // CHECK-FIXES: IDENT(a) || b;
+ a & IDENT(b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && IDENT(b);
+ IDENT(a) & IDENT(b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: IDENT(a) && IDENT(b);
- bool v = FUN5(false | true);
- // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // insert `)` - BAD
+ bool c = true, e = false;
+ a && b | IDENT(c &&) e;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- bool vv = FUN6(true && false);
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ // insert `)` - GOOD
+ a && b | c IDENT(&& e);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && (b || c) IDENT(&& e);
- MY_OR_FUNC(a, b);
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- MY_AND_FUNC(a, b);
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- MY_OR_ASSIGN_FUNC(a, b);
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- MY_AND_ASSIGN_FUNC(a, b);
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // insert `(` - BAD
+ a IDENT(&& b) | c && e;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- IDENT(a | b);
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- IDENT(a & b);
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- bool abc = false;
- IDENT(abc |= b);
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- IDENT(abc &= b);
+ // insert `(` - GOOD
+ IDENT(a &&) b | c && e;
+ // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: IDENT(a &&) (b || c) && e;
+
+ bool ab = false;
+ // insert ` = a` - BAD
+ CAT(a, b) &= b;
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- M1(false, false);
- // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
- M2(& false, | false);
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES-NOT: :[[@LINE-3]]:{{.*}}: note: FIX-IT applied suggested code changes
+ // insert ` = a`- GOOD
+ b &= CAT(a, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: b = b && CAT(a, b);
}
template<typename T>
>From bc5c91c7d52ffba9904db0c429dd2b151cfc176d Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Sat, 31 May 2025 03:39:33 +0300
Subject: [PATCH 05/12] Completed all tests
---
.../performance/BoolBitwiseOperationCheck.cpp | 26 ++++++
.../performance/bool-bitwise-operation.cpp | 81 ++++++++++++++++++-
2 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
index 91150f7f1e332..164373dab02eb 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -17,6 +17,20 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
namespace {
+template<typename AstNode>
+bool isInTemplateFunction(const AstNode* AN, ASTContext& Context) {
+ auto Parents = Context.getParents(*AN);
+ for (const auto& Parent : Parents) {
+ if (const auto* FD = Parent.template get<FunctionDecl>()) {
+ return FD->isTemplateInstantiation() ||
+ FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate;
+ }
+ if (const auto* S = Parent.template get<Stmt>()) {
+ return isInTemplateFunction(S, Context);
+ }
+ }
+ return false;
+}
constexpr std::array<std::pair<llvm::StringRef, llvm::StringRef>, 8U>
OperatorsTransformation{{{"|", "||"},
@@ -38,6 +52,7 @@ llvm::StringRef translate(llvm::StringRef Value) {
}
}
+// TODO: simplify the matcher
void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(binaryOperator(
unless(isExpansionInSystemHeader()),
@@ -62,6 +77,17 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
auto Diag = diag(MatchedExpr->getOperatorLoc(), "use logical operator instead of bitwise one for bool");
+ if (isInTemplateFunction(MatchedExpr, *Result.Context))
+ return;
+
+ // if (MatchedExpr->isInstantiationDependent())
+ // return;
+ // if
+ // (removeCVRef(MatchedExpr->getLHS()->IgnoreImpCasts()->getType())->isTemplateTypeParmType()
+ // &&
+ // removeCVRef(MatchedExpr->getRHS()->IgnoreImpCasts()->getType())->isTemplateTypeParmType())
+ // return;
+
const auto *VolatileOperand = Result.Nodes.getNodeAs<Expr>("vol");
if (VolatileOperand)
return;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
index 3615569073be2..5f1b7cf80896d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
@@ -17,7 +17,7 @@ bool& normal() {
return st;
}
-void bad() {
+bool bad() {
bool a = true, b = false;
a | b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
@@ -31,6 +31,34 @@ void bad() {
a &= b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: a = a && b;
+
+ return true;
+}
+
+bool global_1 = bad() | bad();
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+// CHECK-FIXES: bool global_1 = bad() || bad();
+bool global_2 = bad() & bad();
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+// CHECK-FIXES: bool global_2 = bad() && bad();
+
+using Boolean = bool;
+
+bool bad_typedef() {
+ Boolean a = true, b = false;
+ a | b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || b;
+ a & b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && b;
+ a |= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a || b;
+ a &= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && b;
+ return true;
}
void bad_volatile_bool() {
@@ -84,6 +112,12 @@ void bad_with_priors() {
b | c && a;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: (b || c) && a;
+
+ bool q = (true && false | true) && (false | true && (false && true | false));
+ // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:47: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-3]]:72: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: bool q = (true && (false || true)) && ((false || true) && (false && (true || false)));
}
void bad_with_priors2() {
@@ -256,8 +290,47 @@ void good_in_unreachable_template(T a, T b) {
a &= b;
}
-// TODO: test for in template
-// TODO: test for type in typedef
-// TODO: test for expressions in parentheses
+template<typename T>
+int bad_in_template(T a, T b) {
+ bool c = false;
+ a | b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a & b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a |= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a &= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ c &= a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ return 0;
+}
+
+template<typename T>
+int bad_in_template_lamnda_captured(T a, T b) {
+ [=] mutable {
+ bool c = false;
+ a | b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a & b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a |= b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ b &= a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ }();
+ return 0;
+}
+
+int dummy = bad_in_template(true, false) + bad_in_template_lamnda_captured(false, true);
>From 5ca7121eb98c48655e3fbd5d1e429f05da14848e Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Sat, 31 May 2025 17:05:07 +0300
Subject: [PATCH 06/12] WIP
---
.../performance/BoolBitwiseOperationCheck.cpp | 29 +++++--------------
.../performance/bool-bitwise-operation.cpp | 13 +++++++++
2 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
index 164373dab02eb..e60730be9e5a4 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -52,22 +52,13 @@ llvm::StringRef translate(llvm::StringRef Value) {
}
}
-// TODO: simplify the matcher
void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(binaryOperator(
unless(isExpansionInSystemHeader()),
hasAnyOperatorName("|", "&", "|=", "&="),
hasEitherOperand(expr(ignoringImpCasts(hasType(booleanType())))),
- optionally(hasEitherOperand(
- expr(ignoringImpCasts(hasType(isVolatileQualified())))
- .bind("vol"))),
optionally(hasAncestor(
- binaryOperator().bind("p"))),
- optionally(hasRHS(ignoringParenCasts(
- binaryOperator().bind("r")))),
- optionally(hasLHS(ignoringParenCasts(
- declRefExpr().bind("l")
- )))
+ binaryOperator().bind("p")))
)
.bind("op"), this);
}
@@ -80,16 +71,9 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
if (isInTemplateFunction(MatchedExpr, *Result.Context))
return;
- // if (MatchedExpr->isInstantiationDependent())
- // return;
- // if
- // (removeCVRef(MatchedExpr->getLHS()->IgnoreImpCasts()->getType())->isTemplateTypeParmType()
- // &&
- // removeCVRef(MatchedExpr->getRHS()->IgnoreImpCasts()->getType())->isTemplateTypeParmType())
- // return;
-
- const auto *VolatileOperand = Result.Nodes.getNodeAs<Expr>("vol");
- if (VolatileOperand)
+ const bool HasVolatileOperand = MatchedExpr->getLHS()->IgnoreImpCasts()->getType().isVolatileQualified() ||
+ MatchedExpr->getRHS()->IgnoreImpCasts()->getType().isVolatileQualified();
+ if (HasVolatileOperand)
return;
SourceLocation Loc = MatchedExpr->getOperatorLoc();
@@ -116,7 +100,7 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
FixItHint InsertEqual, ReplaceOperator, InsertBrace1, InsertBrace2;
if (MatchedExpr->isCompoundAssignmentOp()) {
- const auto *DelcRefLHS = Result.Nodes.getNodeAs<DeclRefExpr>("l");
+ const auto *DelcRefLHS = dyn_cast<DeclRefExpr>(MatchedExpr->getLHS()->IgnoreImpCasts());
if (!DelcRefLHS)
return;
const SourceLocation LocLHS = DelcRefLHS->getEndLoc();
@@ -137,12 +121,13 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Parent = Result.Nodes.getNodeAs<BinaryOperator>("p"); Parent)
ParentOpcode = Parent->getOpcode();
- const auto *RHS = Result.Nodes.getNodeAs<BinaryOperator>("r");
+ const auto *RHS = dyn_cast<BinaryOperator>(MatchedExpr->getRHS()->IgnoreParenCasts());
std::optional<BinaryOperatorKind> RHSOpcode;
if (RHS)
RHSOpcode = RHS->getOpcode();
const BinaryOperator* SurroundedExpr = nullptr;
+ // TODO: `*ParentOpcode == BO_Xor` - forgot about OR operator
if ((MatchedExpr->getOpcode() == BO_Or && ParentOpcode.has_value() && *ParentOpcode == BO_LAnd) ||
(MatchedExpr->getOpcode() == BO_And && ParentOpcode.has_value() && *ParentOpcode == BO_Xor)) {
SurroundedExpr = MatchedExpr;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
index 5f1b7cf80896d..74f8be33c4303 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
@@ -118,6 +118,11 @@ void bad_with_priors() {
// CHECK-MESSAGES: :[[@LINE-2]]:47: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES: :[[@LINE-3]]:72: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: bool q = (true && (false || true)) && ((false || true) && (false && (true || false)));
+
+ // TODO: ?? a && (b | c);
+
+ // TODO: ?? a && (q ^ (b | c));
+
}
void bad_with_priors2() {
@@ -132,6 +137,12 @@ void bad_with_priors2() {
b & c ^ a;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: (b && c) ^ a;
+
+ // TODO: make a test case from it
+ // bool d = false;
+ // d ^ (a && b & c);
+
+ // TODO: is there a hidden problem with priority when for example `|` surrounded by `||` changed to `||`
}
void bad_with_priors_compound() {
@@ -148,6 +159,8 @@ void bad_with_priors_compound() {
a |= b && c;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: a = a || b && c;
+
+ // TODO: test for already braced, `a &= (b || c);`
}
void bad_with_priors_nontraditional() {
>From ffc51ddb9bc6b946cc31d095bcad36915a6bb55a Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Mon, 2 Jun 2025 00:10:53 +0300
Subject: [PATCH 07/12] WIP
---
.../performance/BoolBitwiseOperationCheck.cpp | 32 +++-
.../performance/bool-bitwise-operation.cpp | 149 +++++++++++++++---
2 files changed, 150 insertions(+), 31 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
index e60730be9e5a4..0b8dca733ba3e 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -17,6 +17,24 @@ using namespace clang::ast_matchers;
namespace clang::tidy::performance {
namespace {
+bool hasExplicitParentheses(const Expr* E, const SourceManager& SM, const LangOptions& LangOpts) {
+ if (!E)
+ return false;
+
+ const SourceLocation Start = E->getBeginLoc();
+ const SourceLocation End = E->getEndLoc();
+
+ if (Start.isMacroID() || End.isMacroID() || !Start.isValid() || !End.isValid()) {
+ return false;
+ }
+
+ const std::optional<Token> PrevTok = Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional<Token> NextTok = Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ (NextTok && NextTok->is(tok::r_paren));
+}
+
template<typename AstNode>
bool isInTemplateFunction(const AstNode* AN, ASTContext& Context) {
auto Parents = Context.getParents(*AN);
@@ -57,10 +75,9 @@ void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
unless(isExpansionInSystemHeader()),
hasAnyOperatorName("|", "&", "|=", "&="),
hasEitherOperand(expr(ignoringImpCasts(hasType(booleanType())))),
- optionally(hasAncestor(
+ optionally(hasAncestor( // to simple implement transformations like `a&&b|c` -> `a&&(b||c)`
binaryOperator().bind("p")))
- )
- .bind("op"), this);
+ ).bind("op"), this);
}
void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
@@ -73,7 +90,8 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
const bool HasVolatileOperand = MatchedExpr->getLHS()->IgnoreImpCasts()->getType().isVolatileQualified() ||
MatchedExpr->getRHS()->IgnoreImpCasts()->getType().isVolatileQualified();
- if (HasVolatileOperand)
+ const bool HasSideEffects = MatchedExpr->getRHS()->HasSideEffects(*Result.Context); // TODO: `IncludePossibleEffects` option
+ if (HasVolatileOperand || HasSideEffects)
return;
SourceLocation Loc = MatchedExpr->getOperatorLoc();
@@ -127,14 +145,16 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
RHSOpcode = RHS->getOpcode();
const BinaryOperator* SurroundedExpr = nullptr;
- // TODO: `*ParentOpcode == BO_Xor` - forgot about OR operator
if ((MatchedExpr->getOpcode() == BO_Or && ParentOpcode.has_value() && *ParentOpcode == BO_LAnd) ||
- (MatchedExpr->getOpcode() == BO_And && ParentOpcode.has_value() && *ParentOpcode == BO_Xor)) {
+ (MatchedExpr->getOpcode() == BO_And && ParentOpcode.has_value() && llvm::is_contained({BO_Xor, BO_Or}, *ParentOpcode))) {
SurroundedExpr = MatchedExpr;
} else if (MatchedExpr->getOpcode() == BO_AndAssign && RHSOpcode.has_value() && *RHSOpcode == BO_LOr) {
SurroundedExpr = RHS;
}
+ if (hasExplicitParentheses(SurroundedExpr, *Result.SourceManager,Result.Context->getLangOpts()))
+ SurroundedExpr = nullptr;
+
if (SurroundedExpr) {
const SourceLocation InsertFirstLoc = SurroundedExpr->getBeginLoc();
const SourceLocation InsertSecondLoc =
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
index 74f8be33c4303..3207c9a5f125b 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
@@ -17,7 +17,7 @@ bool& normal() {
return st;
}
-bool bad() {
+bool bad() noexcept __attribute__((pure)) {
bool a = true, b = false;
a | b;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
@@ -61,7 +61,73 @@ bool bad_typedef() {
return true;
}
-void bad_volatile_bool() {
+bool function_with_possible_side_effects();
+
+void bad_side_effects() {
+ bool a = true, b = false;
+
+ a | function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a & function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ function_with_possible_side_effects() | a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: function_with_possible_side_effects() || a;
+
+ function_with_possible_side_effects() & a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: function_with_possible_side_effects() && a;
+ a |= function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a &= function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ // count of evaluation with side effect remains the same, so the fixit will be provided
+ bool c = true;
+
+ a || function_with_possible_side_effects() | c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || function_with_possible_side_effects() || c;
+
+ function_with_possible_side_effects() || b | c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: function_with_possible_side_effects() || b || c;
+
+ a && function_with_possible_side_effects() & c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && function_with_possible_side_effects() && c;
+
+ function_with_possible_side_effects() && b & c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: function_with_possible_side_effects() && b && c;
+
+ // but here the count of evaluation migh be changed - no fix must be provided
+
+ a &= function_with_possible_side_effects() && c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a &= b && function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a |= function_with_possible_side_effects() || c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a |= b || function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+}
+
+void bad_side_effects_volatile() {
bool a = true;
volatile bool b = false;
a | b;
@@ -118,11 +184,6 @@ void bad_with_priors() {
// CHECK-MESSAGES: :[[@LINE-2]]:47: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES: :[[@LINE-3]]:72: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: bool q = (true && (false || true)) && ((false || true) && (false && (true || false)));
-
- // TODO: ?? a && (b | c);
-
- // TODO: ?? a && (q ^ (b | c));
-
}
void bad_with_priors2() {
@@ -130,19 +191,62 @@ void bad_with_priors2() {
a ^ b & c;
// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: a ^ (b && c);
+
+ // braces added in the first change
a | b & c;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-MESSAGES: :[[@LINE-2]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a || b && c;
+ // CHECK-FIXES: a || (b && c);
+
b & c ^ a;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: (b && c) ^ a;
- // TODO: make a test case from it
- // bool d = false;
- // d ^ (a && b & c);
+ // braces added in the first change
+ b & c | a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b && c) || a;
- // TODO: is there a hidden problem with priority when for example `|` surrounded by `||` changed to `||`
+ // case to check `hasAncestor` works as we expected:
+ bool d = false;
+ d ^ (a && b & c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: d ^ (a && b && c);
+}
+
+void bad_with_priors_already_braced() {
+ bool a = false, b = true, c = true;
+ a && (b | c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && (b || c);
+ (b | c) && a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b || c) && a;
+
+ bool q = (true && (false | true)) && ((false | true) && (false && (true | false)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:50: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-3]]:77: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: bool q = (true && (false || true)) && ((false || true) && (false && (true || false)));
+
+ a ^ (b & c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a ^ (b && c);
+
+ a | (b & c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || (b && c);
+
+ (b & c) ^ a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b && c) ^ a;
+
+ (b & c) | a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b && c) || a;
}
void bad_with_priors_compound() {
@@ -159,8 +263,13 @@ void bad_with_priors_compound() {
a |= b && c;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
// CHECK-FIXES: a = a || b && c;
-
- // TODO: test for already braced, `a &= (b || c);`
+}
+
+void bad_with_priors_compound_already_braced() {
+ bool a = false, b = true, c = true;
+ a &= (b || c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && (b || c);
}
void bad_with_priors_nontraditional() {
@@ -183,17 +292,7 @@ void bad_with_priors_nontraditional() {
}
void bad_with_priors2_nontraditional() {
- bool a = false, b = true, c = true;
- a xor b bitand c;
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a xor (b and c);
- a bitor b bitand c;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a or b and c;
- b bitand c xor a;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: (b and c) xor a;
+ // TODO: implement this
}
void bad_with_priors_compound_nontraditional() {
>From 6a282c0ae0853cb08b49998a440d23b0c99594dc Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Mon, 2 Jun 2025 00:49:56 +0300
Subject: [PATCH 08/12] Implement ChangePossibleSideEffects option
---
.../performance/BoolBitwiseOperationCheck.cpp | 10 ++-
.../performance/BoolBitwiseOperationCheck.h | 7 +-
...-operation-change-possible-side-effect.cpp | 83 +++++++++++++++++++
3 files changed, 97 insertions(+), 3 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation-change-possible-side-effect.cpp
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
index 0b8dca733ba3e..17bb3d024304e 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -70,6 +70,14 @@ llvm::StringRef translate(llvm::StringRef Value) {
}
}
+BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ ChangePossibleSideEffects(Options.get("ChangePossibleSideEffects", false)) {}
+
+void BoolBitwiseOperationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "ChangePossibleSideEffects", ChangePossibleSideEffects);
+}
+
void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(binaryOperator(
unless(isExpansionInSystemHeader()),
@@ -90,7 +98,7 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
const bool HasVolatileOperand = MatchedExpr->getLHS()->IgnoreImpCasts()->getType().isVolatileQualified() ||
MatchedExpr->getRHS()->IgnoreImpCasts()->getType().isVolatileQualified();
- const bool HasSideEffects = MatchedExpr->getRHS()->HasSideEffects(*Result.Context); // TODO: `IncludePossibleEffects` option
+ const bool HasSideEffects = MatchedExpr->getRHS()->HasSideEffects(*Result.Context, !ChangePossibleSideEffects);
if (HasVolatileOperand || HasSideEffects)
return;
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
index d3a348116bf24..0cc2b3504dd5f 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
@@ -19,13 +19,16 @@ namespace clang::tidy::performance {
/// http://clang.llvm.org/extra/clang-tidy/checks/performance/bool-bitwise-operation.html
class BoolBitwiseOperationCheck : public ClangTidyCheck {
public:
- BoolBitwiseOperationCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ BoolBitwiseOperationCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Options) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
+
+private:
+ bool ChangePossibleSideEffects;
};
} // namespace clang::tidy::performance
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation-change-possible-side-effect.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation-change-possible-side-effect.cpp
new file mode 100644
index 0000000000000..b22f504d56b4c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation-change-possible-side-effect.cpp
@@ -0,0 +1,83 @@
+// RUN: %check_clang_tidy %s performance-bool-bitwise-operation %t \
+// RUN: -config="{CheckOptions: { \
+// RUN: performance-bool-bitwise-operation.ChangePossibleSideEffects: true }}"
+
+bool function_with_possible_side_effects();
+
+void bad_possible_side_effects() {
+ bool a = true, b = false;
+
+ a | function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a || function_with_possible_side_effects();
+
+ a & function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a && function_with_possible_side_effects();
+
+ a |= function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a || function_with_possible_side_effects();
+
+ a &= function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && function_with_possible_side_effects();
+
+ bool c = true;
+
+ a &= function_with_possible_side_effects() && c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && function_with_possible_side_effects() && c;
+
+ a &= b && function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a && b && function_with_possible_side_effects();
+
+ a |= function_with_possible_side_effects() || c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a || function_with_possible_side_effects() || c;
+
+ a |= b || function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a || b || function_with_possible_side_effects();
+}
+
+void bad_definitely_side_effects() {
+ bool a = true, b = false;
+ int acc = 0;
+
+ a | (acc++, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a & (acc++, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a |= (acc++, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a &= (acc++, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ bool c = true;
+
+ a &= (acc++, b) && c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a &= b && (acc++, c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a |= (acc++, b) || c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a |= b || (acc++, c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+}
+
>From f6aac994f700b0d9bc1b9cd1f9c832e92a1f6ca9 Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Mon, 2 Jun 2025 01:37:51 +0300
Subject: [PATCH 09/12] Add nontraditional test
---
.../bool-bitwise-operation-nontraditional.cpp | 388 ++++++++++++++++++
.../performance/bool-bitwise-operation.cpp | 60 ---
2 files changed, 388 insertions(+), 60 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation-nontraditional.cpp
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation-nontraditional.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation-nontraditional.cpp
new file mode 100644
index 0000000000000..e1463a290453d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation-nontraditional.cpp
@@ -0,0 +1,388 @@
+// RUN: %check_clang_tidy %s performance-bool-bitwise-operation %t
+
+bool& normal() {
+ int a = 100, b = 200;
+
+ a bitor b;
+ a bitand b;
+ a or_eq b;
+ a and_eq b;
+
+ static bool st = false;
+ return st;
+}
+
+bool bad() noexcept __attribute__((pure)) {
+ bool a = true, b = false;
+ a bitor b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or b;
+ a bitand b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and b;
+ a or_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a or b;
+ a and_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a and b;
+
+ return true;
+}
+
+bool global_1 = bad() bitor bad();
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+// CHECK-FIXES: bool global_1 = bad() or bad();
+bool global_2 = bad() bitand bad();
+// CHECK-MESSAGES: :[[@LINE-1]]:23: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+// CHECK-FIXES: bool global_2 = bad() and bad();
+
+using Boolean = bool;
+
+bool bad_typedef() {
+ Boolean a = true, b = false;
+ a bitor b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or b;
+ a bitand b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and b;
+ a or_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a or b;
+ a and_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a and b;
+ return true;
+}
+
+bool function_with_possible_side_effects();
+
+void bad_side_effects() {
+ bool a = true, b = false;
+
+ a bitor function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a bitand function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ function_with_possible_side_effects() bitor a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: function_with_possible_side_effects() or a;
+
+ function_with_possible_side_effects() bitand a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:43: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: function_with_possible_side_effects() and a;
+ a or_eq function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a and_eq function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ // count of evaluation with side effect remains the same, so the fixit will be provided
+ bool c = true;
+
+ a or function_with_possible_side_effects() bitor c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or function_with_possible_side_effects() or c;
+
+ function_with_possible_side_effects() or b bitor c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: function_with_possible_side_effects() or b or c;
+
+ a and function_with_possible_side_effects() bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:49: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and function_with_possible_side_effects() and c;
+
+ function_with_possible_side_effects() and b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:49: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: function_with_possible_side_effects() and b and c;
+
+ // but here the count of evaluation migh be changed - no fix must be provided
+
+ a and_eq function_with_possible_side_effects() and c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a and_eq b and function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a or_eq function_with_possible_side_effects() or c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a or_eq b or function_with_possible_side_effects();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+}
+
+void bad_side_effects_volatile() {
+ bool a = true;
+ volatile bool b = false;
+ a bitor b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a bitand b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ a or_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a and_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+}
+
+void bad_with_priors() {
+ bool a = false, b = true, c = true;
+ a and b bitor c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and (b or c);
+ a and b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and b and c;
+ a or b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or b and c;
+ a or b bitor c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or b or c;
+ b bitor c and a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b or c) and a;
+
+ bool q = (true and false bitor true) and (false bitor true and (false and true bitor false));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:53: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-3]]:84: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: bool q = (true and (false or true)) and ((false or true) and (false and (true or false)));
+}
+
+void bad_with_priors2() {
+ bool a = false, b = true, c = true;
+ a xor b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a xor (b and c);
+
+ // braces added in the first change
+ a bitor b bitand c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or (b and c);
+
+ b bitand c xor a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b and c) xor a;
+
+ // braces added in the first change
+ b bitand c bitor a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b and c) or a;
+
+ // case to check `hasAncestor` works as we expected:
+ bool d = false;
+ d xor (a and b bitand c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: d xor (a and b and c);
+}
+
+void bad_with_priors_already_braced() {
+ bool a = false, b = true, c = true;
+ a and (b bitor c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and (b or c);
+ (b bitor c) and a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b or c) and a;
+
+ bool q = (true and (false bitor true)) and ((false bitor true) and (false and (true bitor false)));
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:56: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-3]]:89: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: bool q = (true and (false or true)) and ((false or true) and (false and (true or false)));
+
+ a xor (b bitand c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a xor (b and c);
+
+ a bitor (b bitand c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a or (b and c);
+
+ (b bitand c) xor a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b and c) xor a;
+
+ (b bitand c) bitor a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES: :[[@LINE-2]]:18: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: (b and c) or a;
+}
+
+void bad_with_priors_compound() {
+ bool a = false, b = true, c = true;
+ a and_eq b or c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a and (b or c);
+ a or_eq b or c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a or b or c;
+ a and_eq b and c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a and b and c;
+ a or_eq b and c;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a or b and c;
+}
+
+void bad_with_priors_compound_already_braced() {
+ bool a = false, b = true, c = true;
+ a and_eq (b or c);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a = a and (b or c);
+}
+
+void bad_no_fixit() {
+ bool b = false;
+ normal() or_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ normal() and_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+}
+
+#define MY_OR bitor
+#define MY_AND bitand
+#define MY_OR_ASSIGN or_eq
+#define MY_AND_ASSIGN and_eq
+#define MY_LOG_AND and
+
+#define CAT(a, b) a ## b
+#define IDENT(a) a
+
+void bad_in_macro() {
+ bool a = true, b = false;
+
+ // change operator - BAD
+ a MY_OR b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a MY_AND b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a MY_OR_ASSIGN b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a MY_AND_ASSIGN b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ IDENT(a and_eq b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ // change operator - GOOD
+ IDENT(a) bitor b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: IDENT(a) or b;
+ a bitand IDENT(b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and IDENT(b);
+ IDENT(a) bitand IDENT(b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: IDENT(a) and IDENT(b);
+
+ // insert `)` - BAD
+ bool c = true, e = false;
+ a and b bitor IDENT(c and) e;
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ // insert `)` - GOOD
+ a and b bitor c IDENT(and e);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: a and (b or c) IDENT(and e);
+
+ // insert `(` - BAD
+ a IDENT(and b) bitor c and e;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ // insert `(` - GOOD
+ IDENT(a and) b bitor c and e;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: IDENT(a and) (b or c) and e;
+
+ bool ab = false;
+ // insert ` = a` - BAD
+ CAT(a, b) and_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+
+ // insert ` = a`- GOOD
+ b and_eq CAT(a, b);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-FIXES: b = b and CAT(a, b);
+}
+
+template<typename T>
+void good_in_unreachable_template(T a, T b) {
+ a bitor b;
+ a bitand b;
+ a or_eq b;
+ a and_eq b;
+}
+
+template<typename T>
+int bad_in_template(T a, T b) {
+ bool c = false;
+ a bitor b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a bitand b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a or_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a and_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ c and_eq a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ return 0;
+}
+
+template<typename T>
+int bad_in_template_lamnda_captured(T a, T b) {
+ [=] mutable {
+ bool c = false;
+ a bitor b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a bitand b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ a or_eq b;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ b and_eq a;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
+ // CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
+ }();
+ return 0;
+}
+
+int dummy = bad_in_template(true, false) + bad_in_template_lamnda_captured(false, true);
+
+
diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
index 3207c9a5f125b..a880cd6cb5d70 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance/bool-bitwise-operation.cpp
@@ -8,11 +8,6 @@ bool& normal() {
a |= b;
a &= b;
- a bitor b;
- a bitand b;
- a or_eq b;
- a and_eq b;
-
static bool st = false;
return st;
}
@@ -145,22 +140,6 @@ void bad_side_effects_volatile() {
// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested code changes
}
-void bad_nontraditional() {
- bool a = true, b = false;
- a bitor b;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a or b;
- a bitand b;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a and b;
- a or_eq b;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a = a or b;
- a and_eq b;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a = a and b;
-}
-
void bad_with_priors() {
bool a = false, b = true, c = true;
a && b | c;
@@ -272,45 +251,6 @@ void bad_with_priors_compound_already_braced() {
// CHECK-FIXES: a = a && (b || c);
}
-void bad_with_priors_nontraditional() {
- bool a = false, b = true, c = true;
- a and b bitor c;
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a and (b or c);
- a and b bitand c;
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a and b and c;
- a or b bitand c;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a or b and c;
- a or b bitor c;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a or b or c;
- b bitor c and a;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: (b or c) and a;
-}
-
-void bad_with_priors2_nontraditional() {
- // TODO: implement this
-}
-
-void bad_with_priors_compound_nontraditional() {
- bool a = false, b = true, c = true;
- a and_eq b or c;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a = a and (b or c);
- a or_eq b or c;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a = a or b or c;
- a and_eq b and c;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a = a and b and c;
- a or_eq b and c;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use logical operator instead of bitwise one for bool [performance-bool-bitwise-operation]
- // CHECK-FIXES: a = a or b and c;
-}
-
void bad_no_fixit() {
bool b = false;
normal() |= b;
>From e2128c6d18eb0234f8ccf2fd49d79b842e798c22 Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Mon, 2 Jun 2025 01:55:31 +0300
Subject: [PATCH 10/12] format
---
.../performance/BoolBitwiseOperationCheck.cpp | 106 +++++++++++-------
1 file changed, 63 insertions(+), 43 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
index 17bb3d024304e..148ba65de129d 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.cpp
@@ -10,40 +10,44 @@
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
#include <array>
-#include <utility>
#include <optional>
+#include <utility>
using namespace clang::ast_matchers;
namespace clang::tidy::performance {
namespace {
-bool hasExplicitParentheses(const Expr* E, const SourceManager& SM, const LangOptions& LangOpts) {
+bool hasExplicitParentheses(const Expr *E, const SourceManager &SM,
+ const LangOptions &LangOpts) {
if (!E)
return false;
const SourceLocation Start = E->getBeginLoc();
const SourceLocation End = E->getEndLoc();
- if (Start.isMacroID() || End.isMacroID() || !Start.isValid() || !End.isValid()) {
+ if (Start.isMacroID() || End.isMacroID() || !Start.isValid() ||
+ !End.isValid()) {
return false;
}
- const std::optional<Token> PrevTok = Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
- const std::optional<Token> NextTok = Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional<Token> PrevTok =
+ Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional<Token> NextTok =
+ Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
- return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
(NextTok && NextTok->is(tok::r_paren));
}
-template<typename AstNode>
-bool isInTemplateFunction(const AstNode* AN, ASTContext& Context) {
+template <typename AstNode>
+bool isInTemplateFunction(const AstNode *AN, ASTContext &Context) {
auto Parents = Context.getParents(*AN);
- for (const auto& Parent : Parents) {
- if (const auto* FD = Parent.template get<FunctionDecl>()) {
- return FD->isTemplateInstantiation() ||
- FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate;
+ for (const auto &Parent : Parents) {
+ if (const auto *FD = Parent.template get<FunctionDecl>()) {
+ return FD->isTemplateInstantiation() ||
+ FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate;
}
- if (const auto* S = Parent.template get<Stmt>()) {
+ if (const auto *S = Parent.template get<Stmt>()) {
return isInTemplateFunction(S, Context);
}
}
@@ -68,37 +72,47 @@ llvm::StringRef translate(llvm::StringRef Value) {
return {};
}
-}
+} // namespace
-BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context),
- ChangePossibleSideEffects(Options.get("ChangePossibleSideEffects", false)) {}
+BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context), ChangePossibleSideEffects(Options.get(
+ "ChangePossibleSideEffects", false)) {}
-void BoolBitwiseOperationCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+void BoolBitwiseOperationCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "ChangePossibleSideEffects", ChangePossibleSideEffects);
}
void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(binaryOperator(
- unless(isExpansionInSystemHeader()),
- hasAnyOperatorName("|", "&", "|=", "&="),
- hasEitherOperand(expr(ignoringImpCasts(hasType(booleanType())))),
- optionally(hasAncestor( // to simple implement transformations like `a&&b|c` -> `a&&(b||c)`
- binaryOperator().bind("p")))
- ).bind("op"), this);
+ Finder->addMatcher(
+ binaryOperator(
+ unless(isExpansionInSystemHeader()),
+ hasAnyOperatorName("|", "&", "|=", "&="),
+ hasEitherOperand(expr(ignoringImpCasts(hasType(booleanType())))),
+ optionally(hasAncestor( // to simple implement transformations like
+ // `a&&b|c` -> `a&&(b||c)`
+ binaryOperator().bind("p"))))
+ .bind("op"),
+ this);
}
void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
const auto *MatchedExpr = Result.Nodes.getNodeAs<BinaryOperator>("op");
- auto Diag = diag(MatchedExpr->getOperatorLoc(), "use logical operator instead of bitwise one for bool");
+ auto Diag = diag(MatchedExpr->getOperatorLoc(),
+ "use logical operator instead of bitwise one for bool");
if (isInTemplateFunction(MatchedExpr, *Result.Context))
return;
- const bool HasVolatileOperand = MatchedExpr->getLHS()->IgnoreImpCasts()->getType().isVolatileQualified() ||
- MatchedExpr->getRHS()->IgnoreImpCasts()->getType().isVolatileQualified();
- const bool HasSideEffects = MatchedExpr->getRHS()->HasSideEffects(*Result.Context, !ChangePossibleSideEffects);
+ const bool HasVolatileOperand = llvm::any_of(
+ std::array{MatchedExpr->getLHS(), MatchedExpr->getRHS()},
+ [](const Expr *E) {
+ return E->IgnoreImpCasts()->getType().isVolatileQualified();
+ });
+ const bool HasSideEffects = MatchedExpr->getRHS()->HasSideEffects(
+ *Result.Context, !ChangePossibleSideEffects);
if (HasVolatileOperand || HasSideEffects)
return;
@@ -126,20 +140,20 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
FixItHint InsertEqual, ReplaceOperator, InsertBrace1, InsertBrace2;
if (MatchedExpr->isCompoundAssignmentOp()) {
- const auto *DelcRefLHS = dyn_cast<DeclRefExpr>(MatchedExpr->getLHS()->IgnoreImpCasts());
+ const auto *DelcRefLHS =
+ dyn_cast<DeclRefExpr>(MatchedExpr->getLHS()->IgnoreImpCasts());
if (!DelcRefLHS)
return;
const SourceLocation LocLHS = DelcRefLHS->getEndLoc();
if (LocLHS.isInvalid() || LocLHS.isMacroID())
return;
const SourceLocation InsertLoc = clang::Lexer::getLocForEndOfToken(
- LocLHS, 0, *Result.SourceManager,
- Result.Context->getLangOpts());
+ LocLHS, 0, *Result.SourceManager, Result.Context->getLangOpts());
if (InsertLoc.isInvalid() || InsertLoc.isMacroID()) {
return;
}
- InsertEqual = FixItHint::CreateInsertion(InsertLoc,
- " = " + DelcRefLHS->getDecl()->getNameAsString());
+ InsertEqual = FixItHint::CreateInsertion(
+ InsertLoc, " = " + DelcRefLHS->getDecl()->getNameAsString());
}
ReplaceOperator = FixItHint::CreateReplacement(TokenRange, FixSpelling);
@@ -147,28 +161,34 @@ void BoolBitwiseOperationCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *Parent = Result.Nodes.getNodeAs<BinaryOperator>("p"); Parent)
ParentOpcode = Parent->getOpcode();
- const auto *RHS = dyn_cast<BinaryOperator>(MatchedExpr->getRHS()->IgnoreParenCasts());
+ const auto *RHS =
+ dyn_cast<BinaryOperator>(MatchedExpr->getRHS()->IgnoreParenCasts());
std::optional<BinaryOperatorKind> RHSOpcode;
if (RHS)
RHSOpcode = RHS->getOpcode();
- const BinaryOperator* SurroundedExpr = nullptr;
- if ((MatchedExpr->getOpcode() == BO_Or && ParentOpcode.has_value() && *ParentOpcode == BO_LAnd) ||
- (MatchedExpr->getOpcode() == BO_And && ParentOpcode.has_value() && llvm::is_contained({BO_Xor, BO_Or}, *ParentOpcode))) {
+ const BinaryOperator *SurroundedExpr = nullptr;
+ if ((MatchedExpr->getOpcode() == BO_Or && ParentOpcode.has_value() &&
+ *ParentOpcode == BO_LAnd) ||
+ (MatchedExpr->getOpcode() == BO_And && ParentOpcode.has_value() &&
+ llvm::is_contained({BO_Xor, BO_Or}, *ParentOpcode))) {
SurroundedExpr = MatchedExpr;
- } else if (MatchedExpr->getOpcode() == BO_AndAssign && RHSOpcode.has_value() && *RHSOpcode == BO_LOr) {
+ } else if (MatchedExpr->getOpcode() == BO_AndAssign &&
+ RHSOpcode.has_value() && *RHSOpcode == BO_LOr) {
SurroundedExpr = RHS;
}
- if (hasExplicitParentheses(SurroundedExpr, *Result.SourceManager,Result.Context->getLangOpts()))
+ if (hasExplicitParentheses(SurroundedExpr, *Result.SourceManager,
+ Result.Context->getLangOpts()))
SurroundedExpr = nullptr;
if (SurroundedExpr) {
const SourceLocation InsertFirstLoc = SurroundedExpr->getBeginLoc();
- const SourceLocation InsertSecondLoc =
- clang::Lexer::getLocForEndOfToken(SurroundedExpr->getEndLoc(), 0, *Result.SourceManager,
+ const SourceLocation InsertSecondLoc = clang::Lexer::getLocForEndOfToken(
+ SurroundedExpr->getEndLoc(), 0, *Result.SourceManager,
Result.Context->getLangOpts());
- if (InsertFirstLoc.isInvalid() || InsertFirstLoc.isMacroID() || InsertSecondLoc.isInvalid() || InsertSecondLoc.isMacroID())
+ if (InsertFirstLoc.isInvalid() || InsertFirstLoc.isMacroID() ||
+ InsertSecondLoc.isInvalid() || InsertSecondLoc.isMacroID())
return;
InsertBrace1 = FixItHint::CreateInsertion(InsertFirstLoc, "(");
InsertBrace2 = FixItHint::CreateInsertion(InsertSecondLoc, ")");
>From dcd3a7eb4332af64bd34df8b6bbefb47a8cb49ad Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Mon, 2 Jun 2025 02:52:05 +0300
Subject: [PATCH 11/12] docs
---
.../performance/BoolBitwiseOperationCheck.h | 4 ++-
clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-
.../performance/bool-bitwise-operation.rst | 36 ++++++++++++++++++-
3 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
index 0cc2b3504dd5f..6a5f0c7db6f1b 100644
--- a/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
+++ b/clang-tools-extra/clang-tidy/performance/BoolBitwiseOperationCheck.h
@@ -13,7 +13,9 @@
namespace clang::tidy::performance {
-/// FIXME: Write a short description.
+/// Finds potentially unefficient uses of bitwise operators such as `|`,
+/// `&` and their compound analogues with `bool` type and suggests replacing
+/// them with logical ones, like `||` and `&&`.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/performance/bool-bitwise-operation.html
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 183038598235b..6499ed554968f 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -139,7 +139,9 @@ New checks
- New :doc:`performance-bool-bitwise-operation
<clang-tidy/checks/performance/bool-bitwise-operation>` check.
- FIXME: Write a short description.
+ Finds potentially unefficient uses of bitwise operators such as ``|``,
+ ``&`` and their compound analogues with ``bool`` type and suggests replacing
+ them with logical ones, like ``||`` and ``&&``.
- New :doc:`portability-avoid-pragma-once
<clang-tidy/checks/portability/avoid-pragma-once>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
index 9b236f95504ad..03c38f5e53447 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
@@ -3,4 +3,38 @@
performance-bool-bitwise-operation
==================================
-FIXME: Describe what patterns does the check detect and why. Give examples.
+Finds potentially inefficient use of bitwise operators such as ``&``,
+``|`` and their compound analogues on boolean values where logical
+operators like ``&&`` and ``||`` would be more appropriate. Bitwise
+operations on booleans can incur unnecessary performance overhead due
+ to implicit integer conversions and missed short-circuit evaluation.
+
+.. code-block:: c++
+
+ bool invalid = false;
+ invalid |= x > limit.x; // warning: use logical operator instead of bitwise one for bool
+ invalid |= y > limit.y; // warning: use logical operator instead of bitwise one for bool
+ invalid |= z > limit.z; // warning: use logical operator instead of bitwise one for bool
+ if (invalid) {
+ // error handling
+ }
+
+These 3 warnings suggest to assign result of logical ``||`` operation instead of using ``|=`` operator:
+
+.. code-block:: c++
+
+ bool invalid = false;
+ invalid = invalid || x > limit.x;
+ invalid = invalid || y > limit.x;
+ invalid = invalid || z > limit.z;
+ if (invalid) {
+ // error handling
+ }
+
+Options
+-------
+
+.. option:: ChangePossibleSideEffects
+
+ Enabling this option promotes more fixit hints even when they might
+ change evaluation order or skip side effects. Default value is `false`.
\ No newline at end of file
>From 19faf60794fe5762147958845109eb50836ca071 Mon Sep 17 00:00:00 2001
From: Denis Mikhailov <denismikhaylov38 at gmail.com>
Date: Mon, 2 Jun 2025 02:56:26 +0300
Subject: [PATCH 12/12] fix the doc
---
.../clang-tidy/checks/performance/bool-bitwise-operation.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst b/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
index 03c38f5e53447..762e57352db66 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance/bool-bitwise-operation.rst
@@ -6,8 +6,8 @@ performance-bool-bitwise-operation
Finds potentially inefficient use of bitwise operators such as ``&``,
``|`` and their compound analogues on boolean values where logical
operators like ``&&`` and ``||`` would be more appropriate. Bitwise
-operations on booleans can incur unnecessary performance overhead due
- to implicit integer conversions and missed short-circuit evaluation.
+operations on booleans can incur unnecessary performance overhead
+due to implicit integer conversions and missed short-circuit evaluation.
.. code-block:: c++
More information about the cfe-commits
mailing list