[clang-tools-extra] 3676b09 - [clang-tidy] `readability-simplify-boolean-expr` avoid to warn expression expand from macro when ``IgnoreMacro`` option is enabled. (#91757)
via cfe-commits
cfe-commits at lists.llvm.org
Fri May 10 21:15:28 PDT 2024
Author: Congcong Cai
Date: 2024-05-11T12:15:24+08:00
New Revision: 3676b0945c800e3105f648d178b331953246716a
URL: https://github.com/llvm/llvm-project/commit/3676b0945c800e3105f648d178b331953246716a
DIFF: https://github.com/llvm/llvm-project/commit/3676b0945c800e3105f648d178b331953246716a.diff
LOG: [clang-tidy] `readability-simplify-boolean-expr` avoid to warn expression expand from macro when ``IgnoreMacro`` option is enabled. (#91757)
Fixes: #91487
Added:
Modified:
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
index edb67614bd558..fd4730d9c8b9c 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "SimplifyBooleanExprCheck.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Lex/Lexer.h"
#include "llvm/Support/SaveAndRestore.h"
@@ -280,9 +281,8 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
if (!S) {
return true;
}
- if (Check->IgnoreMacros && S->getBeginLoc().isMacroID()) {
+ if (Check->canBeBypassed(S))
return false;
- }
if (!shouldIgnore(S))
StmtStack.push_back(S);
return true;
@@ -513,17 +513,23 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
return true;
}
- static bool isUnaryLNot(const Expr *E) {
- return isa<UnaryOperator>(E) &&
+ bool isExpectedUnaryLNot(const Expr *E) {
+ return !Check->canBeBypassed(E) && isa<UnaryOperator>(E) &&
cast<UnaryOperator>(E)->getOpcode() == UO_LNot;
}
+ bool isExpectedBinaryOp(const Expr *E) {
+ const auto *BinaryOp = dyn_cast<BinaryOperator>(E);
+ return !Check->canBeBypassed(E) && BinaryOp && BinaryOp->isLogicalOp() &&
+ BinaryOp->getType()->isBooleanType();
+ }
+
template <typename Functor>
static bool checkEitherSide(const BinaryOperator *BO, Functor Func) {
return Func(BO->getLHS()) || Func(BO->getRHS());
}
- static bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {
+ bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {
const auto *BO = dyn_cast<BinaryOperator>(E->IgnoreUnlessSpelledInSource());
if (!BO)
return false;
@@ -539,15 +545,13 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
return true;
case BO_LAnd:
case BO_LOr:
- if (checkEitherSide(BO, isUnaryLNot))
- return true;
- if (NestingLevel) {
- if (checkEitherSide(BO, [NestingLevel](const Expr *E) {
- return nestedDemorgan(E, NestingLevel - 1);
- }))
- return true;
- }
- return false;
+ return checkEitherSide(
+ BO,
+ [this](const Expr *E) { return isExpectedUnaryLNot(E); }) ||
+ (NestingLevel &&
+ checkEitherSide(BO, [this, NestingLevel](const Expr *E) {
+ return nestedDemorgan(E, NestingLevel - 1);
+ }));
default:
return false;
}
@@ -556,19 +560,19 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
bool TraverseUnaryOperator(UnaryOperator *Op) {
if (!Check->SimplifyDeMorgan || Op->getOpcode() != UO_LNot)
return Base::TraverseUnaryOperator(Op);
- Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
- auto *Parens = dyn_cast<ParenExpr>(SubImp);
- auto *BinaryOp =
- Parens
- ? dyn_cast<BinaryOperator>(Parens->getSubExpr()->IgnoreImplicit())
- : dyn_cast<BinaryOperator>(SubImp);
- if (!BinaryOp || !BinaryOp->isLogicalOp() ||
- !BinaryOp->getType()->isBooleanType())
+ const Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();
+ const auto *Parens = dyn_cast<ParenExpr>(SubImp);
+ const Expr *SubExpr =
+ Parens ? Parens->getSubExpr()->IgnoreImplicit() : SubImp;
+ if (!isExpectedBinaryOp(SubExpr))
return Base::TraverseUnaryOperator(Op);
+ const auto *BinaryOp = cast<BinaryOperator>(SubExpr);
if (Check->SimplifyDeMorganRelaxed ||
- checkEitherSide(BinaryOp, isUnaryLNot) ||
- checkEitherSide(BinaryOp,
- [](const Expr *E) { return nestedDemorgan(E, 1); })) {
+ checkEitherSide(
+ BinaryOp,
+ [this](const Expr *E) { return isExpectedUnaryLNot(E); }) ||
+ checkEitherSide(
+ BinaryOp, [this](const Expr *E) { return nestedDemorgan(E, 1); })) {
if (Check->reportDeMorgan(Context, Op, BinaryOp, !IsProcessing, parent(),
Parens) &&
!Check->areDiagsSelfContained()) {
@@ -694,6 +698,10 @@ void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
Visitor(this, *Result.Context).traverse();
}
+bool SimplifyBooleanExprCheck::canBeBypassed(const Stmt *S) const {
+ return IgnoreMacros && S->getBeginLoc().isMacroID();
+}
+
void SimplifyBooleanExprCheck::issueDiag(const ASTContext &Context,
SourceLocation Loc,
StringRef Description,
diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
index ccc6f3d879fc0..63c3caa01e01a 100644
--- a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
@@ -64,6 +64,8 @@ class SimplifyBooleanExprCheck : public ClangTidyCheck {
StringRef Description, SourceRange ReplacementRange,
StringRef Replacement);
+ bool canBeBypassed(const Stmt *S) const;
+
const bool IgnoreMacros;
const bool ChainedConditionalReturn;
const bool ChainedConditionalAssignment;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3f0d25ec8c752..6a2b8d3b6ded6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -357,6 +357,10 @@ Changes in existing checks
support calls to overloaded operators as base expression and provide fixes to
expressions with side-effects.
+- Improved :doc:`readability-simplify-boolean-expr
+ <clang-tidy/checks/readability/simplify-boolean-expr>` check to avoid to emit
+ warning for macro when IgnoreMacro option is enabled.
+
- Improved :doc:`readability-static-definition-in-anonymous-namespace
<clang-tidy/checks/readability/static-definition-in-anonymous-namespace>`
check by resolving fix-it overlaps in template code by disregarding implicit
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
index 7d0cfe7e27dc2..d1df79e23a1e6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/simplify-boolean-expr-macros.cpp
@@ -6,6 +6,7 @@
// RUN: --
#define NEGATE(expr) !(expr)
+#define NOT_AND_NOT(a, b) (!a && !b)
bool without_macro(bool a, bool b) {
return !(!a && b);
@@ -13,8 +14,17 @@ bool without_macro(bool a, bool b) {
// CHECK-FIXES: return a || !b;
}
-bool macro(bool a, bool b) {
- return NEGATE(!a && b);
- // CHECK-MESSAGES-MACROS: :[[@LINE-1]]:12: warning: boolean expression can be simplified by DeMorgan's theorem
- // CHECK-FIXES: return NEGATE(!a && b);
+void macro(bool a, bool b) {
+ NEGATE(!a && b);
+ // CHECK-MESSAGES-MACROS: :[[@LINE-1]]:5: warning: boolean expression can be simplified by DeMorgan's theorem
+ // CHECK-FIXES: NEGATE(!a && b);
+ !NOT_AND_NOT(a, b);
+ // CHECK-MESSAGES-MACROS: :[[@LINE-1]]:5: warning: boolean expression can be simplified by DeMorgan's theorem
+ // CHECK-FIXES: !NOT_AND_NOT(a, b);
+ !(NEGATE(a) && b);
+ // CHECK-MESSAGES-MACROS: :[[@LINE-1]]:5: warning: boolean expression can be simplified by DeMorgan's theorem
+ // CHECK-FIXES: !(NEGATE(a) && b);
+ !(a && NEGATE(b));
+ // CHECK-MESSAGES-MACROS: :[[@LINE-1]]:5: warning: boolean expression can be simplified by DeMorgan's theorem
+ // CHECK-FIXES: !(a && NEGATE(b));
}
More information about the cfe-commits
mailing list