diff --git a/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp b/clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp index edb67614bd5585..fd4730d9c8b9c8 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 { 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 { return true; } - static bool isUnaryLNot(const Expr *E) { - return isa(E) && + bool isExpectedUnaryLNot(const Expr *E) { + return !Check->canBeBypassed(E) && isa(E) && cast(E)->getOpcode() == UO_LNot; } + bool isExpectedBinaryOp(const Expr *E) { + const auto *BinaryOp = dyn_cast(E); + return !Check->canBeBypassed(E) && BinaryOp && BinaryOp->isLogicalOp() && + BinaryOp->getType()->isBooleanType(); + } + template 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(E->IgnoreUnlessSpelledInSource()); if (!BO) return false; @@ -539,15 +545,13 @@ class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor { 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 { bool TraverseUnaryOperator(UnaryOperator *Op) { if (!Check->SimplifyDeMorgan || Op->getOpcode() != UO_LNot) return Base::TraverseUnaryOperator(Op); - Expr *SubImp = Op->getSubExpr()->IgnoreImplicit(); - auto *Parens = dyn_cast(SubImp); - auto *BinaryOp = - Parens - ? dyn_cast(Parens->getSubExpr()->IgnoreImplicit()) - : dyn_cast(SubImp); - if (!BinaryOp || !BinaryOp->isLogicalOp() || - !BinaryOp->getType()->isBooleanType()) + const Expr *SubImp = Op->getSubExpr()->IgnoreImplicit(); + const auto *Parens = dyn_cast(SubImp); + const Expr *SubExpr = + Parens ? Parens->getSubExpr()->IgnoreImplicit() : SubImp; + if (!isExpectedBinaryOp(SubExpr)) return Base::TraverseUnaryOperator(Op); + const auto *BinaryOp = cast(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 ccc6f3d879fc02..63c3caa01e01a7 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 3f0d25ec8c7524..6a2b8d3b6ded61 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 + ` check to avoid to emit + warning for macro when IgnoreMacro option is enabled. + - Improved :doc:`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 7d0cfe7e27dc22..d1df79e23a1e6f 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)); }