[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,197 @@
+//===--- 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"
+#include
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+static const NamedDecl *
+getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
+ if (BO->isCompoundAssignmentOp()) {
+const auto *DeclRefLHS =
+dyn_cast(BO->getLHS()->IgnoreImpCasts());
+return DeclRefLHS ? DeclRefLHS->getDecl() : nullptr;
+ }
+ return nullptr;
+}
+
+static 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.isInvalid() ||
+ End.isInvalid())
+return false;
+
+ const std::optional PrevTok =
+ Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional NextTok =
+ Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ (NextTok && NextTok->is(tok::r_paren));
+}
+
+constexpr std::array, 8U>
+OperatorsTransformation{{{"|", "||"},
+ {"|=", "||"},
+ {"&", "&&"},
+ {"&=", "&&"},
+ {"bitand", "and"},
+ {"and_eq", "and"},
+ {"bitor", "or"},
+ {"or_eq", "or"}}};
+
+static llvm::StringRef translate(llvm::StringRef Value) {
+ for (const auto &[Bitwise, Logical] : OperatorsTransformation) {
+if (Value == Bitwise)
+ return Logical;
+ }
+
+ return {};
+}
+
+BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ StrictMode(Options.get("StrictMode", false)),
+ IgnoreMacros(Options.get("IgnoreMacros", false)) {}
+
+void BoolBitwiseOperationCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+ Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
+void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(
+ unless(isExpansionInSystemHeader()),
+ hasAnyOperatorName("|", "&", "|=", "&="),
+ hasEitherOperand(expr(hasType(booleanType(,
+ optionally(hasParent( // 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("op");
+
+ auto DiagEmitter = [MatchedExpr, this] {
+const NamedDecl *ND = getLHSNamedDeclIfCompoundAssign(MatchedExpr);
+return diag(MatchedExpr->getOperatorLoc(),
+"use logical operator '%0' for boolean %select{variable "
+"%2|values}1 instead of bitwise operator '%3'")
+ << translate(MatchedExpr->getOpcodeStr()) << (ND == nullptr) << ND
+ << MatchedExpr->getOpcodeStr();
+ };
+
+ const bool HasVolatileOperand = llvm::any_of(
+ std::array{MatchedExpr->getLHS(), MatchedExpr->getRHS()},
+ [](const Expr *E) {
+return E->IgnoreImpCasts()->getType().isVolatileQualified();
+ });
+ if (HasVolatileOperand)
+return (void)DiagEmitter();
+
+ const bool HasSideEffects = MatchedExpr->getRHS()->HasSideEffects(
+ *Result.Context, /*IncludePossibleEffects=*/!StrictMode);
+ if (HasSideEffects)
+return (void)DiagEmitter();
+
+ SourceLocation Loc = MatchedExpr->getOperatorLoc();
+
+ if (Loc.isInvalid() || Loc.isMacroID())
+return void(IgnoreMacros || DiagEmitter());
+
+ Loc = Result.SourceManager->getSpellingLoc(Loc);
+ if (Loc.isInvalid() || Loc.isMacroID())
+return void(IgnoreMacros || DiagEmitter());
+
+ const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
+ if (TokenRange.isInvalid())
+return void(IgnoreMacros || DiagEmitter());
+
+ const StringRef FixSpelling = translate(Lexer::getSourceText(
+ TokenRange, *Result.SourceManager, Result.Context->getLangOpts()));
denzor200 wrote:
Stayed as it is
https://github.com/llvm/llvm-project/pull
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,59 @@
+// RUN: %check_clang_tidy %s misc-bool-bitwise-operation %t \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-bool-bitwise-operation.IgnoreMacros: true }}"
+
+#define MY_OR |
+#define MY_AND &
+#define MY_OR_ASSIGN |=
+#define MY_AND_ASSIGN &=
+#define MY_LOG_AND &&
denzor200 wrote:
The test you've asked for will become not actual in future versions(See note
https://github.com/denzor200/llvm-project/blob/b9be35d4d4bec7ede2ebe141eeb8a9d4fe1c550c/clang-tools-extra/test/clang-tidy/checkers/misc/bool-bitwise-operation.cpp#L352
)
I will just remove these defines
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,62 @@
+.. title:: clang-tidy - misc-bool-bitwise-operation
+
+misc-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.
+
+.. code-block:: c++
+
+ bool invalid = false;
+ invalid |= x > limit.x; // warning: use logical operator '||' for boolean
variable 'invalid' instead of bitwise operator '|='
+ invalid |= y > limit.y; // warning: use logical operator '||' for boolean
variable 'invalid' instead of bitwise operator '|='
+ invalid |= z > limit.z; // warning: use logical operator '||' for boolean
variable 'invalid' instead of bitwise operator '|='
+ 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
+ }
+
+Limitations
+---
+
+* Templates aren't matched.
vbvictor wrote:
```suggestion
Bitwise operators inside templates aren't matched.
```
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,62 @@
+.. title:: clang-tidy - misc-bool-bitwise-operation
+
+misc-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.
+
+.. code-block:: c++
+
+ bool invalid = false;
+ invalid |= x > limit.x; // warning: use logical operator '||' for boolean
variable 'invalid' instead of bitwise operator '|='
+ invalid |= y > limit.y; // warning: use logical operator '||' for boolean
variable 'invalid' instead of bitwise operator '|='
+ invalid |= z > limit.z; // warning: use logical operator '||' for boolean
variable 'invalid' instead of bitwise operator '|='
+ 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
+ }
+
+Limitations
+---
EugeneZelenko wrote:
```suggestion
Limitations
---
```
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
denzor200 wrote: > we could add Limitations section in check docs to avoid potential confusion > and false-negative reports in the future. Done https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
denzor200 wrote: > I believe all PiotrZSL's comments are correctly resolved, and we could merge > it next week if there would be no more reviews. One nit from me: since we > don't match template functions, we could add `Limitations` section in check > docs to avoid potential confusion and false-negative reports in the future. Oke, I will extend the doc later https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
vbvictor wrote: I believe all PiotrZSL's comments are correctly resolved, and we could merge it next week if there would be no more reviews. One nit from me: since we don't match template functions, we could add `Limitations` section in check docs to avoid potential confusion and false-negative issues in the future. https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
denzor200 wrote: ping https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,83 @@
+// RUN: %check_clang_tidy %s misc-bool-bitwise-operation %t \
+// RUN: -config="{CheckOptions: { \
+// RUN: misc-bool-bitwise-operation.StrictMode: 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 '||' for
boolean values instead of bitwise operator '|' [misc-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 '&&' for
boolean values instead of bitwise operator '&' [misc-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 '||' for
boolean variable 'a' instead of bitwise operator '|='
[misc-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 '&&' for
boolean variable 'a' instead of bitwise operator '&='
[misc-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 '&&' for
boolean variable 'a' instead of bitwise operator '&='
[misc-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 '&&' for
boolean variable 'a' instead of bitwise operator '&='
[misc-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 '||' for
boolean variable 'a' instead of bitwise operator '|='
[misc-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 '||' for
boolean variable 'a' instead of bitwise operator '|='
[misc-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 '||' for
boolean values instead of bitwise operator '|' [misc-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 '&&' for
boolean values instead of bitwise operator '&' [misc-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 '||' for
boolean variable 'a' instead of bitwise operator '|='
[misc-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 '&&' for
boolean variable 'a' instead of bitwise operator '&='
[misc-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 '&&' for
boolean variable 'a' instead of bitwise operator '&='
[misc-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 '&&' for
boolean variable 'a' instead of bitwise operator '&='
[misc-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 '||' for
boolean variable 'a' instead of bitwise operator '|='
[misc-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 '||' for
boolean variable 'a' instead of bitwise operator '|='
[misc-bool-bitwise-operation]
+// CHECK-MESSAGES-NOT: :[[@LINE-2]]:{{.*}}: note: FIX-IT applied suggested
c
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,219 @@
+//===--- 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"
+#include "llvm/ADT/ScopeExit.h"
+#include
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+static const NamedDecl *
+getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
+ if (BO->isCompoundAssignmentOp()) {
+const auto *DelcRefLHS =
+dyn_cast(BO->getLHS()->IgnoreImpCasts());
+return DelcRefLHS ? DelcRefLHS->getDecl() : nullptr;
+ }
+ return nullptr;
+}
+
+static 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.isInvalid() ||
+ End.isInvalid())
+return false;
+
+ const std::optional PrevTok =
+ Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional NextTok =
+ Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ (NextTok && NextTok->is(tok::r_paren));
+}
+
+constexpr std::array, 8U>
+OperatorsTransformation{{{"|", "||"},
+ {"|=", "||"},
+ {"&", "&&"},
+ {"&=", "&&"},
+ {"bitand", "and"},
+ {"and_eq", "and"},
+ {"bitor", "or"},
+ {"or_eq", "or"}}};
+
+static std::string translate(llvm::StringRef Value) {
+ for (const auto &[Bitwise, Logical] : OperatorsTransformation) {
+if (Value == Bitwise)
+ return Logical.str();
+ }
+
+ return {};
+}
+
+BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ StrictMode(Options.get("StrictMode", false)),
+ IgnoreMacros(Options.get("IgnoreMacros", false)) {}
+
+void BoolBitwiseOperationCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+ Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
+void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(
+ unless(isExpansionInSystemHeader()),
+ hasAnyOperatorName("|", "&", "|=", "&="),
+ hasEitherOperand(expr(hasType(booleanType(,
+ optionally(hasParent( // 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("op");
+
+ auto DiagEmitterProc = [MatchedExpr, this] {
+const NamedDecl *ND = getLHSNamedDeclIfCompoundAssign(MatchedExpr);
+return diag(MatchedExpr->getOperatorLoc(),
+"use logical operator '%0' for boolean %select{variable "
+"%2|values}1 instead of bitwise operator '%3'")
+ << translate(MatchedExpr->getOpcodeStr()) << (ND == nullptr) << ND
+ << MatchedExpr->getOpcodeStr();
+ };
+ auto DiagEmitter = llvm::make_scope_exit(DiagEmitterProc);
denzor200 wrote:
It will have a problem when an exception will be trown from the lambda, so I
will change it to use lambda directly
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,198 @@
+//===--- 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"
+#include
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+static const NamedDecl *
+getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
+ if (BO->isCompoundAssignmentOp()) {
+const auto *DelcRefLHS =
vbvictor wrote:
`Delc` -> `Decl` everywhere in this file
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,198 @@
+//===--- 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"
+#include
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+static const NamedDecl *
+getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
+ if (BO->isCompoundAssignmentOp()) {
+const auto *DelcRefLHS =
+dyn_cast(BO->getLHS()->IgnoreImpCasts());
+return DelcRefLHS ? DelcRefLHS->getDecl() : nullptr;
+ }
+ return nullptr;
+}
+
+static 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.isInvalid() ||
+ End.isInvalid())
+return false;
+
+ const std::optional PrevTok =
+ Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional NextTok =
+ Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ (NextTok && NextTok->is(tok::r_paren));
+}
+
+constexpr std::array, 8U>
+OperatorsTransformation{{{"|", "||"},
+ {"|=", "||"},
+ {"&", "&&"},
+ {"&=", "&&"},
+ {"bitand", "and"},
+ {"and_eq", "and"},
+ {"bitor", "or"},
+ {"or_eq", "or"}}};
+
+static std::string translate(llvm::StringRef Value) {
+ for (const auto &[Bitwise, Logical] : OperatorsTransformation) {
+if (Value == Bitwise)
+ return Logical.str();
+ }
+
+ return {};
+}
+
+BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ StrictMode(Options.get("StrictMode", false)),
+ IgnoreMacros(Options.get("IgnoreMacros", false)) {}
+
+void BoolBitwiseOperationCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+ Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
+void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(
+ unless(isExpansionInSystemHeader()),
+ hasAnyOperatorName("|", "&", "|=", "&="),
vbvictor wrote:
you could write
```cpp
IgnoreMacros
? unless(isInMacro())
: static_cast>(
anything())
```
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
https://github.com/vbvictor approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,198 @@
+//===--- 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"
+#include
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+static const NamedDecl *
+getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
+ if (BO->isCompoundAssignmentOp()) {
+const auto *DelcRefLHS =
+dyn_cast(BO->getLHS()->IgnoreImpCasts());
+return DelcRefLHS ? DelcRefLHS->getDecl() : nullptr;
+ }
+ return nullptr;
+}
+
+static 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.isInvalid() ||
+ End.isInvalid())
+return false;
+
+ const std::optional PrevTok =
+ Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional NextTok =
+ Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ (NextTok && NextTok->is(tok::r_paren));
+}
+
+constexpr std::array, 8U>
+OperatorsTransformation{{{"|", "||"},
+ {"|=", "||"},
+ {"&", "&&"},
+ {"&=", "&&"},
+ {"bitand", "and"},
+ {"and_eq", "and"},
+ {"bitor", "or"},
+ {"or_eq", "or"}}};
+
+static std::string translate(llvm::StringRef Value) {
+ for (const auto &[Bitwise, Logical] : OperatorsTransformation) {
+if (Value == Bitwise)
+ return Logical.str();
+ }
+
+ return {};
+}
+
+BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ StrictMode(Options.get("StrictMode", false)),
+ IgnoreMacros(Options.get("IgnoreMacros", false)) {}
+
+void BoolBitwiseOperationCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+ Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
+void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(
+ unless(isExpansionInSystemHeader()),
+ hasAnyOperatorName("|", "&", "|=", "&="),
+ hasEitherOperand(expr(hasType(booleanType(,
+ optionally(hasParent( // 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("op");
+
+ auto DiagEmitter = [MatchedExpr, this] {
+const NamedDecl *ND = getLHSNamedDeclIfCompoundAssign(MatchedExpr);
+return diag(MatchedExpr->getOperatorLoc(),
+"use logical operator '%0' for boolean %select{variable "
+"%2|values}1 instead of bitwise operator '%3'")
+ << translate(MatchedExpr->getOpcodeStr()) << (ND == nullptr) << ND
+ << MatchedExpr->getOpcodeStr();
+ };
+
+ const bool HasVolatileOperand = llvm::any_of(
+ std::array{MatchedExpr->getLHS(), MatchedExpr->getRHS()},
+ [](const Expr *E) {
+return E->IgnoreImpCasts()->getType().isVolatileQualified();
+ });
+ if (HasVolatileOperand)
+return (void)DiagEmitter();
+
+ const bool HasSideEffects = MatchedExpr->getRHS()->HasSideEffects(
+ *Result.Context, /*IncludePossibleEffects=*/!StrictMode);
+ if (HasSideEffects)
+return (void)DiagEmitter();
+
+ SourceLocation Loc = MatchedExpr->getOperatorLoc();
+
+ if (Loc.isInvalid() || Loc.isMacroID())
+return void(IgnoreMacros || DiagEmitter());
+
+ Loc = Result.SourceManager->getSpellingLoc(Loc);
+ if (Loc.isInvalid() || Loc.isMacroID())
+return void(IgnoreMacros || DiagEmitter());
+
+ const CharSourceRange TokenRange = CharSourceRange::getTokenRange(Loc);
+ if (TokenRange.isInvalid())
+return void(IgnoreMacros || DiagEmitter());
+
+ StringRef Spelling = Lexer::getSourceText(TokenRange, *Result.SourceManager,
+Result.Context->getLangOpts());
+ const std::string FixSpelling = translate(Spelling);
v
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,198 @@
+//===--- 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"
+#include
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+static const NamedDecl *
+getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
+ if (BO->isCompoundAssignmentOp()) {
+const auto *DelcRefLHS =
+dyn_cast(BO->getLHS()->IgnoreImpCasts());
+return DelcRefLHS ? DelcRefLHS->getDecl() : nullptr;
+ }
+ return nullptr;
+}
+
+static 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.isInvalid() ||
+ End.isInvalid())
+return false;
+
+ const std::optional PrevTok =
+ Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional NextTok =
+ Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ (NextTok && NextTok->is(tok::r_paren));
+}
+
+constexpr std::array, 8U>
+OperatorsTransformation{{{"|", "||"},
+ {"|=", "||"},
+ {"&", "&&"},
+ {"&=", "&&"},
+ {"bitand", "and"},
+ {"and_eq", "and"},
+ {"bitor", "or"},
+ {"or_eq", "or"}}};
+
+static std::string translate(llvm::StringRef Value) {
+ for (const auto &[Bitwise, Logical] : OperatorsTransformation) {
+if (Value == Bitwise)
+ return Logical.str();
vbvictor wrote:
```suggestion
return Logical;
```
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,198 @@
+//===--- 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"
+#include
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+static const NamedDecl *
+getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
+ if (BO->isCompoundAssignmentOp()) {
+const auto *DelcRefLHS =
+dyn_cast(BO->getLHS()->IgnoreImpCasts());
+return DelcRefLHS ? DelcRefLHS->getDecl() : nullptr;
+ }
+ return nullptr;
+}
+
+static 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.isInvalid() ||
+ End.isInvalid())
+return false;
+
+ const std::optional PrevTok =
+ Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional NextTok =
+ Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ (NextTok && NextTok->is(tok::r_paren));
+}
+
+constexpr std::array, 8U>
+OperatorsTransformation{{{"|", "||"},
+ {"|=", "||"},
+ {"&", "&&"},
+ {"&=", "&&"},
+ {"bitand", "and"},
+ {"and_eq", "and"},
+ {"bitor", "or"},
+ {"or_eq", "or"}}};
+
+static std::string translate(llvm::StringRef Value) {
vbvictor wrote:
I think we don't need to convert to `std::string`
```suggestion
static llvm::StringRef translate(llvm::StringRef Value) {
```
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
https://github.com/vbvictor edited https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
https://github.com/vbvictor deleted https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
denzor200 wrote: @HerrCai0907 @PiotrZSL could you please approve again? https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
https://github.com/HerrCai0907 edited https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -0,0 +1,219 @@
+//===--- 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"
+#include "llvm/ADT/ScopeExit.h"
+#include
+#include
+#include
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::misc {
+
+static const NamedDecl *
+getLHSNamedDeclIfCompoundAssign(const BinaryOperator *BO) {
+ if (BO->isCompoundAssignmentOp()) {
+const auto *DelcRefLHS =
+dyn_cast(BO->getLHS()->IgnoreImpCasts());
+return DelcRefLHS ? DelcRefLHS->getDecl() : nullptr;
+ }
+ return nullptr;
+}
+
+static 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.isInvalid() ||
+ End.isInvalid())
+return false;
+
+ const std::optional PrevTok =
+ Lexer::findPreviousToken(Start, SM, LangOpts, /*IncludeComments=*/false);
+ const std::optional NextTok =
+ Lexer::findNextToken(End, SM, LangOpts, /*IncludeComments=*/false);
+
+ return (PrevTok && PrevTok->is(tok::l_paren)) &&
+ (NextTok && NextTok->is(tok::r_paren));
+}
+
+constexpr std::array, 8U>
+OperatorsTransformation{{{"|", "||"},
+ {"|=", "||"},
+ {"&", "&&"},
+ {"&=", "&&"},
+ {"bitand", "and"},
+ {"and_eq", "and"},
+ {"bitor", "or"},
+ {"or_eq", "or"}}};
+
+static std::string translate(llvm::StringRef Value) {
+ for (const auto &[Bitwise, Logical] : OperatorsTransformation) {
+if (Value == Bitwise)
+ return Logical.str();
+ }
+
+ return {};
+}
+
+BoolBitwiseOperationCheck::BoolBitwiseOperationCheck(StringRef Name,
+ ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+ StrictMode(Options.get("StrictMode", false)),
+ IgnoreMacros(Options.get("IgnoreMacros", false)) {}
+
+void BoolBitwiseOperationCheck::storeOptions(
+ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+ Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
+void BoolBitwiseOperationCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ binaryOperator(
+ unless(isExpansionInSystemHeader()),
+ hasAnyOperatorName("|", "&", "|=", "&="),
+ hasEitherOperand(expr(hasType(booleanType(,
+ optionally(hasParent( // 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("op");
+
+ auto DiagEmitterProc = [MatchedExpr, this] {
+const NamedDecl *ND = getLHSNamedDeclIfCompoundAssign(MatchedExpr);
+return diag(MatchedExpr->getOperatorLoc(),
+"use logical operator '%0' for boolean %select{variable "
+"%2|values}1 instead of bitwise operator '%3'")
+ << translate(MatchedExpr->getOpcodeStr()) << (ND == nullptr) << ND
+ << MatchedExpr->getOpcodeStr();
+ };
+ auto DiagEmitter = llvm::make_scope_exit(DiagEmitterProc);
HerrCai0907 wrote:
I am not sure whether it is a good code pattern here to use raii for diag.
maybe using lambda directly is more straight forward.
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
https://github.com/HerrCai0907 approved this pull request. lgtm https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -136,6 +136,13 @@ New checks Finds unintended character output from ``unsigned char`` and ``signed char`` to an ``ostream``. +- New :doc:`misc-bool-bitwise-operation EugeneZelenko wrote: Please keep alphabetical order (by check name) in this section. https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
@@ -36,6 +37,8 @@ class PerformanceModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck("performance-avoid-endl");
+CheckFactories.registerCheck(
+"performance-bool-bitwise-operation");
denzor200 wrote:
Moved the check into misc
https://github.com/llvm/llvm-project/pull/142324
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang-tidy] Add misc-bool-bitwise-operation check (PR #142324)
https://github.com/denzor200 edited https://github.com/llvm/llvm-project/pull/142324 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
