123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410411412413414415416417418419420421422423424425426427428429430431432433434435436437438439440441442443444445446447448449450451452453454455456457458459460461462463464465466467468469470471472473474475476477478479480481482483484485486487488489490491492493494495496497498499500501502503504505506507508509510511512513514515516517518519520521522523524525526527528529530531532533534535536537538539540541542543544545546547548549550551552553554555556557558559560561562563564565566567568569570571572573574575576577578579580581582583584585586587588589590591592593594595596597598599600601602603604605606607608609610611612613614615616617618619620621622623624625626627628629630631632633634635636637638639640641642643644645646647648649650651652653654655656657658659660661662663664665666667668669670671672673674675676677678679680681682683684685686687688689690691692693694695696697698699700701702703704705706707708709710711712713714715716717718719720721722723724725726727728729730731732733734735736737738739740741742743744745746747748749750751752753754755756757758759760761762763764765766767768769770771772773774775776777778779780781782783784785786787788789790791792793794795796797798799800801802803804805806807808809810811812813814815816817818819820821822823824825826827828829830831832833834835836837838839840841842843844845846847848849850851852853854855856857858859860861862863864865866867868869870871872873874875876877878879880881882883884885886887888889890891892893894895896897898899900901902903904905906907908909910911912913914915916917918919920921922923924925926927928929930931932933934935936937938939940941942943944945946947948949950951952953954955956957958 |
- //===-- SimplifyBooleanExprCheck.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 "SimplifyBooleanExprCheck.h"
- #include "clang/AST/RecursiveASTVisitor.h"
- #include "clang/Lex/Lexer.h"
- #include "llvm/Support/SaveAndRestore.h"
- #include <optional>
- #include <string>
- #include <utility>
- using namespace clang::ast_matchers;
- namespace clang::tidy::readability {
- namespace {
- StringRef getText(const ASTContext &Context, SourceRange Range) {
- return Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
- Context.getSourceManager(),
- Context.getLangOpts());
- }
- template <typename T> StringRef getText(const ASTContext &Context, T &Node) {
- return getText(Context, Node.getSourceRange());
- }
- } // namespace
- static constexpr char SimplifyOperatorDiagnostic[] =
- "redundant boolean literal supplied to boolean operator";
- static constexpr char SimplifyConditionDiagnostic[] =
- "redundant boolean literal in if statement condition";
- static constexpr char SimplifyConditionalReturnDiagnostic[] =
- "redundant boolean literal in conditional return statement";
- static bool needsParensAfterUnaryNegation(const Expr *E) {
- E = E->IgnoreImpCasts();
- if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
- return true;
- if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E))
- return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
- Op->getOperator() != OO_Subscript;
- return false;
- }
- static std::pair<BinaryOperatorKind, BinaryOperatorKind> Opposites[] = {
- {BO_LT, BO_GE}, {BO_GT, BO_LE}, {BO_EQ, BO_NE}};
- static StringRef negatedOperator(const BinaryOperator *BinOp) {
- const BinaryOperatorKind Opcode = BinOp->getOpcode();
- for (auto NegatableOp : Opposites) {
- if (Opcode == NegatableOp.first)
- return BinOp->getOpcodeStr(NegatableOp.second);
- if (Opcode == NegatableOp.second)
- return BinOp->getOpcodeStr(NegatableOp.first);
- }
- return {};
- }
- static std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = {
- {OO_EqualEqual, "=="}, {OO_ExclaimEqual, "!="}, {OO_Less, "<"},
- {OO_GreaterEqual, ">="}, {OO_Greater, ">"}, {OO_LessEqual, "<="}};
- static StringRef getOperatorName(OverloadedOperatorKind OpKind) {
- for (auto Name : OperatorNames) {
- if (Name.first == OpKind)
- return Name.second;
- }
- return {};
- }
- static std::pair<OverloadedOperatorKind, OverloadedOperatorKind>
- OppositeOverloads[] = {{OO_EqualEqual, OO_ExclaimEqual},
- {OO_Less, OO_GreaterEqual},
- {OO_Greater, OO_LessEqual}};
- static StringRef negatedOperator(const CXXOperatorCallExpr *OpCall) {
- const OverloadedOperatorKind Opcode = OpCall->getOperator();
- for (auto NegatableOp : OppositeOverloads) {
- if (Opcode == NegatableOp.first)
- return getOperatorName(NegatableOp.second);
- if (Opcode == NegatableOp.second)
- return getOperatorName(NegatableOp.first);
- }
- return {};
- }
- static std::string asBool(StringRef Text, bool NeedsStaticCast) {
- if (NeedsStaticCast)
- return ("static_cast<bool>(" + Text + ")").str();
- return std::string(Text);
- }
- static bool needsNullPtrComparison(const Expr *E) {
- if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))
- return ImpCast->getCastKind() == CK_PointerToBoolean ||
- ImpCast->getCastKind() == CK_MemberPointerToBoolean;
- return false;
- }
- static bool needsZeroComparison(const Expr *E) {
- if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))
- return ImpCast->getCastKind() == CK_IntegralToBoolean;
- return false;
- }
- static bool needsStaticCast(const Expr *E) {
- if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {
- if (ImpCast->getCastKind() == CK_UserDefinedConversion &&
- ImpCast->getSubExpr()->getType()->isBooleanType()) {
- if (const auto *MemCall =
- dyn_cast<CXXMemberCallExpr>(ImpCast->getSubExpr())) {
- if (const auto *MemDecl =
- dyn_cast<CXXConversionDecl>(MemCall->getMethodDecl())) {
- if (MemDecl->isExplicit())
- return true;
- }
- }
- }
- }
- E = E->IgnoreImpCasts();
- return !E->getType()->isBooleanType();
- }
- static std::string compareExpressionToConstant(const ASTContext &Context,
- const Expr *E, bool Negated,
- const char *Constant) {
- E = E->IgnoreImpCasts();
- const std::string ExprText =
- (isa<BinaryOperator>(E) ? ("(" + getText(Context, *E) + ")")
- : getText(Context, *E))
- .str();
- return ExprText + " " + (Negated ? "!=" : "==") + " " + Constant;
- }
- static std::string compareExpressionToNullPtr(const ASTContext &Context,
- const Expr *E, bool Negated) {
- const char *NullPtr = Context.getLangOpts().CPlusPlus11 ? "nullptr" : "NULL";
- return compareExpressionToConstant(Context, E, Negated, NullPtr);
- }
- static std::string compareExpressionToZero(const ASTContext &Context,
- const Expr *E, bool Negated) {
- return compareExpressionToConstant(Context, E, Negated, "0");
- }
- static std::string replacementExpression(const ASTContext &Context,
- bool Negated, const Expr *E) {
- E = E->IgnoreParenBaseCasts();
- if (const auto *EC = dyn_cast<ExprWithCleanups>(E))
- E = EC->getSubExpr();
- const bool NeedsStaticCast = needsStaticCast(E);
- if (Negated) {
- if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
- if (UnOp->getOpcode() == UO_LNot) {
- if (needsNullPtrComparison(UnOp->getSubExpr()))
- return compareExpressionToNullPtr(Context, UnOp->getSubExpr(), true);
- if (needsZeroComparison(UnOp->getSubExpr()))
- return compareExpressionToZero(Context, UnOp->getSubExpr(), true);
- return replacementExpression(Context, false, UnOp->getSubExpr());
- }
- }
- if (needsNullPtrComparison(E))
- return compareExpressionToNullPtr(Context, E, false);
- if (needsZeroComparison(E))
- return compareExpressionToZero(Context, E, false);
- StringRef NegatedOperator;
- const Expr *LHS = nullptr;
- const Expr *RHS = nullptr;
- if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
- NegatedOperator = negatedOperator(BinOp);
- LHS = BinOp->getLHS();
- RHS = BinOp->getRHS();
- } else if (const auto *OpExpr = dyn_cast<CXXOperatorCallExpr>(E)) {
- if (OpExpr->getNumArgs() == 2) {
- NegatedOperator = negatedOperator(OpExpr);
- LHS = OpExpr->getArg(0);
- RHS = OpExpr->getArg(1);
- }
- }
- if (!NegatedOperator.empty() && LHS && RHS)
- return (asBool((getText(Context, *LHS) + " " + NegatedOperator + " " +
- getText(Context, *RHS))
- .str(),
- NeedsStaticCast));
- StringRef Text = getText(Context, *E);
- if (!NeedsStaticCast && needsParensAfterUnaryNegation(E))
- return ("!(" + Text + ")").str();
- if (needsNullPtrComparison(E))
- return compareExpressionToNullPtr(Context, E, false);
- if (needsZeroComparison(E))
- return compareExpressionToZero(Context, E, false);
- return ("!" + asBool(Text, NeedsStaticCast));
- }
- if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {
- if (UnOp->getOpcode() == UO_LNot) {
- if (needsNullPtrComparison(UnOp->getSubExpr()))
- return compareExpressionToNullPtr(Context, UnOp->getSubExpr(), false);
- if (needsZeroComparison(UnOp->getSubExpr()))
- return compareExpressionToZero(Context, UnOp->getSubExpr(), false);
- }
- }
- if (needsNullPtrComparison(E))
- return compareExpressionToNullPtr(Context, E, true);
- if (needsZeroComparison(E))
- return compareExpressionToZero(Context, E, true);
- return asBool(getText(Context, *E), NeedsStaticCast);
- }
- static bool containsDiscardedTokens(const ASTContext &Context,
- CharSourceRange CharRange) {
- std::string ReplacementText =
- Lexer::getSourceText(CharRange, Context.getSourceManager(),
- Context.getLangOpts())
- .str();
- Lexer Lex(CharRange.getBegin(), Context.getLangOpts(), ReplacementText.data(),
- ReplacementText.data(),
- ReplacementText.data() + ReplacementText.size());
- Lex.SetCommentRetentionState(true);
- Token Tok;
- while (!Lex.LexFromRawLexer(Tok)) {
- if (Tok.is(tok::TokenKind::comment) || Tok.is(tok::TokenKind::hash))
- return true;
- }
- return false;
- }
- class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {
- using Base = RecursiveASTVisitor<Visitor>;
- public:
- Visitor(SimplifyBooleanExprCheck *Check, ASTContext &Context)
- : Check(Check), Context(Context) {}
- bool traverse() { return TraverseAST(Context); }
- static bool shouldIgnore(Stmt *S) {
- switch (S->getStmtClass()) {
- case Stmt::ImplicitCastExprClass:
- case Stmt::MaterializeTemporaryExprClass:
- case Stmt::CXXBindTemporaryExprClass:
- return true;
- default:
- return false;
- }
- }
- bool dataTraverseStmtPre(Stmt *S) {
- if (S && !shouldIgnore(S))
- StmtStack.push_back(S);
- return true;
- }
- bool dataTraverseStmtPost(Stmt *S) {
- if (S && !shouldIgnore(S)) {
- assert(StmtStack.back() == S);
- StmtStack.pop_back();
- }
- return true;
- }
- bool VisitBinaryOperator(const BinaryOperator *Op) const {
- Check->reportBinOp(Context, Op);
- return true;
- }
- // Extracts a bool if an expression is (true|false|!true|!false);
- static std::optional<bool> getAsBoolLiteral(const Expr *E, bool FilterMacro) {
- if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(E)) {
- if (FilterMacro && Bool->getBeginLoc().isMacroID())
- return std::nullopt;
- return Bool->getValue();
- }
- if (const auto *UnaryOp = dyn_cast<UnaryOperator>(E)) {
- if (FilterMacro && UnaryOp->getBeginLoc().isMacroID())
- return std::nullopt;
- if (UnaryOp->getOpcode() == UO_LNot)
- if (std::optional<bool> Res = getAsBoolLiteral(
- UnaryOp->getSubExpr()->IgnoreImplicit(), FilterMacro))
- return !*Res;
- }
- return std::nullopt;
- }
- template <typename Node> struct NodeAndBool {
- const Node *Item = nullptr;
- bool Bool = false;
- operator bool() const { return Item != nullptr; }
- };
- using ExprAndBool = NodeAndBool<Expr>;
- using DeclAndBool = NodeAndBool<Decl>;
- /// Detect's return (true|false|!true|!false);
- static ExprAndBool parseReturnLiteralBool(const Stmt *S) {
- const auto *RS = dyn_cast<ReturnStmt>(S);
- if (!RS || !RS->getRetValue())
- return {};
- if (std::optional<bool> Ret =
- getAsBoolLiteral(RS->getRetValue()->IgnoreImplicit(), false)) {
- return {RS->getRetValue(), *Ret};
- }
- return {};
- }
- /// If \p S is not a \c CompoundStmt, applies F on \p S, otherwise if there is
- /// only 1 statement in the \c CompoundStmt, applies F on that single
- /// statement.
- template <typename Functor>
- static auto checkSingleStatement(Stmt *S, Functor F) -> decltype(F(S)) {
- if (auto *CS = dyn_cast<CompoundStmt>(S)) {
- if (CS->size() == 1)
- return F(CS->body_front());
- return {};
- }
- return F(S);
- }
- Stmt *parent() const {
- return StmtStack.size() < 2 ? nullptr : StmtStack[StmtStack.size() - 2];
- }
- bool VisitIfStmt(IfStmt *If) {
- // Skip any if's that have a condition var or an init statement, or are
- // "if consteval" statements.
- if (If->hasInitStorage() || If->hasVarStorage() || If->isConsteval())
- return true;
- /*
- * if (true) ThenStmt(); -> ThenStmt();
- * if (false) ThenStmt(); -> <Empty>;
- * if (false) ThenStmt(); else ElseStmt() -> ElseStmt();
- */
- Expr *Cond = If->getCond()->IgnoreImplicit();
- if (std::optional<bool> Bool = getAsBoolLiteral(Cond, true)) {
- if (*Bool)
- Check->replaceWithThenStatement(Context, If, Cond);
- else
- Check->replaceWithElseStatement(Context, If, Cond);
- }
- if (If->getElse()) {
- /*
- * if (Cond) return true; else return false; -> return Cond;
- * if (Cond) return false; else return true; -> return !Cond;
- */
- if (ExprAndBool ThenReturnBool =
- checkSingleStatement(If->getThen(), parseReturnLiteralBool)) {
- ExprAndBool ElseReturnBool =
- checkSingleStatement(If->getElse(), parseReturnLiteralBool);
- if (ElseReturnBool && ThenReturnBool.Bool != ElseReturnBool.Bool) {
- if (Check->ChainedConditionalReturn ||
- !isa_and_nonnull<IfStmt>(parent())) {
- Check->replaceWithReturnCondition(Context, If, ThenReturnBool.Item,
- ElseReturnBool.Bool);
- }
- }
- } else {
- /*
- * if (Cond) A = true; else A = false; -> A = Cond;
- * if (Cond) A = false; else A = true; -> A = !Cond;
- */
- Expr *Var = nullptr;
- SourceLocation Loc;
- auto VarBoolAssignmentMatcher = [&Var,
- &Loc](const Stmt *S) -> DeclAndBool {
- const auto *BO = dyn_cast<BinaryOperator>(S);
- if (!BO || BO->getOpcode() != BO_Assign)
- return {};
- std::optional<bool> RightasBool =
- getAsBoolLiteral(BO->getRHS()->IgnoreImplicit(), false);
- if (!RightasBool)
- return {};
- Expr *IgnImp = BO->getLHS()->IgnoreImplicit();
- if (!Var) {
- // We only need to track these for the Then branch.
- Loc = BO->getRHS()->getBeginLoc();
- Var = IgnImp;
- }
- if (auto *DRE = dyn_cast<DeclRefExpr>(IgnImp))
- return {DRE->getDecl(), *RightasBool};
- if (auto *ME = dyn_cast<MemberExpr>(IgnImp))
- return {ME->getMemberDecl(), *RightasBool};
- return {};
- };
- if (DeclAndBool ThenAssignment =
- checkSingleStatement(If->getThen(), VarBoolAssignmentMatcher)) {
- DeclAndBool ElseAssignment =
- checkSingleStatement(If->getElse(), VarBoolAssignmentMatcher);
- if (ElseAssignment.Item == ThenAssignment.Item &&
- ElseAssignment.Bool != ThenAssignment.Bool) {
- if (Check->ChainedConditionalAssignment ||
- !isa_and_nonnull<IfStmt>(parent())) {
- Check->replaceWithAssignment(Context, If, Var, Loc,
- ElseAssignment.Bool);
- }
- }
- }
- }
- }
- return true;
- }
- bool VisitConditionalOperator(ConditionalOperator *Cond) {
- /*
- * Condition ? true : false; -> Condition
- * Condition ? false : true; -> !Condition;
- */
- if (std::optional<bool> Then =
- getAsBoolLiteral(Cond->getTrueExpr()->IgnoreImplicit(), false)) {
- if (std::optional<bool> Else =
- getAsBoolLiteral(Cond->getFalseExpr()->IgnoreImplicit(), false)) {
- if (*Then != *Else)
- Check->replaceWithCondition(Context, Cond, *Else);
- }
- }
- return true;
- }
- bool VisitCompoundStmt(CompoundStmt *CS) {
- if (CS->size() < 2)
- return true;
- bool CurIf = false, PrevIf = false;
- for (auto First = CS->body_begin(), Second = std::next(First),
- End = CS->body_end();
- Second != End; ++Second, ++First) {
- PrevIf = CurIf;
- CurIf = isa<IfStmt>(*First);
- ExprAndBool TrailingReturnBool = parseReturnLiteralBool(*Second);
- if (!TrailingReturnBool)
- continue;
- if (CurIf) {
- /*
- * if (Cond) return true; return false; -> return Cond;
- * if (Cond) return false; return true; -> return !Cond;
- */
- auto *If = cast<IfStmt>(*First);
- if (!If->hasInitStorage() && !If->hasVarStorage() &&
- !If->isConsteval()) {
- ExprAndBool ThenReturnBool =
- checkSingleStatement(If->getThen(), parseReturnLiteralBool);
- if (ThenReturnBool &&
- ThenReturnBool.Bool != TrailingReturnBool.Bool) {
- if ((Check->ChainedConditionalReturn || !PrevIf) &&
- If->getElse() == nullptr) {
- Check->replaceCompoundReturnWithCondition(
- Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,
- If, ThenReturnBool.Item);
- }
- }
- }
- } else if (isa<LabelStmt, CaseStmt, DefaultStmt>(*First)) {
- /*
- * (case X|label_X|default): if (Cond) return BoolLiteral;
- * return !BoolLiteral
- */
- Stmt *SubStmt =
- isa<LabelStmt>(*First) ? cast<LabelStmt>(*First)->getSubStmt()
- : isa<CaseStmt>(*First) ? cast<CaseStmt>(*First)->getSubStmt()
- : cast<DefaultStmt>(*First)->getSubStmt();
- auto *SubIf = dyn_cast<IfStmt>(SubStmt);
- if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
- !SubIf->hasVarStorage() && !SubIf->isConsteval()) {
- ExprAndBool ThenReturnBool =
- checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
- if (ThenReturnBool &&
- ThenReturnBool.Bool != TrailingReturnBool.Bool) {
- Check->replaceCompoundReturnWithCondition(
- Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,
- SubIf, ThenReturnBool.Item);
- }
- }
- }
- }
- return true;
- }
- static bool isUnaryLNot(const Expr *E) {
- return isa<UnaryOperator>(E) &&
- cast<UnaryOperator>(E)->getOpcode() == UO_LNot;
- }
- 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) {
- const auto *BO = dyn_cast<BinaryOperator>(E->IgnoreUnlessSpelledInSource());
- if (!BO)
- return false;
- if (!BO->getType()->isBooleanType())
- return false;
- switch (BO->getOpcode()) {
- case BO_LT:
- case BO_GT:
- case BO_LE:
- case BO_GE:
- case BO_EQ:
- case BO_NE:
- 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;
- default:
- return false;
- }
- }
- 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())
- return Base::TraverseUnaryOperator(Op);
- if (Check->SimplifyDeMorganRelaxed ||
- checkEitherSide(BinaryOp, isUnaryLNot) ||
- checkEitherSide(BinaryOp,
- [](const Expr *E) { return nestedDemorgan(E, 1); })) {
- if (Check->reportDeMorgan(Context, Op, BinaryOp, !IsProcessing, parent(),
- Parens) &&
- !Check->areDiagsSelfContained()) {
- llvm::SaveAndRestore RAII(IsProcessing, true);
- return Base::TraverseUnaryOperator(Op);
- }
- }
- return Base::TraverseUnaryOperator(Op);
- }
- private:
- bool IsProcessing = false;
- SimplifyBooleanExprCheck *Check;
- SmallVector<Stmt *, 32> StmtStack;
- ASTContext &Context;
- };
- SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
- ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context),
- ChainedConditionalReturn(Options.get("ChainedConditionalReturn", false)),
- ChainedConditionalAssignment(
- Options.get("ChainedConditionalAssignment", false)),
- SimplifyDeMorgan(Options.get("SimplifyDeMorgan", true)),
- SimplifyDeMorganRelaxed(Options.get("SimplifyDeMorganRelaxed", false)) {
- if (SimplifyDeMorganRelaxed && !SimplifyDeMorgan)
- configurationDiag("%0: 'SimplifyDeMorganRelaxed' cannot be enabled "
- "without 'SimplifyDeMorgan' enabled")
- << Name;
- }
- static bool containsBoolLiteral(const Expr *E) {
- if (!E)
- return false;
- E = E->IgnoreParenImpCasts();
- if (isa<CXXBoolLiteralExpr>(E))
- return true;
- if (const auto *BinOp = dyn_cast<BinaryOperator>(E))
- return containsBoolLiteral(BinOp->getLHS()) ||
- containsBoolLiteral(BinOp->getRHS());
- if (const auto *UnaryOp = dyn_cast<UnaryOperator>(E))
- return containsBoolLiteral(UnaryOp->getSubExpr());
- return false;
- }
- void SimplifyBooleanExprCheck::reportBinOp(const ASTContext &Context,
- const BinaryOperator *Op) {
- const auto *LHS = Op->getLHS()->IgnoreParenImpCasts();
- const auto *RHS = Op->getRHS()->IgnoreParenImpCasts();
- const CXXBoolLiteralExpr *Bool;
- const Expr *Other;
- if ((Bool = dyn_cast<CXXBoolLiteralExpr>(LHS)) != nullptr)
- Other = RHS;
- else if ((Bool = dyn_cast<CXXBoolLiteralExpr>(RHS)) != nullptr)
- Other = LHS;
- else
- return;
- if (Bool->getBeginLoc().isMacroID())
- return;
- // FIXME: why do we need this?
- if (!isa<CXXBoolLiteralExpr>(Other) && containsBoolLiteral(Other))
- return;
- bool BoolValue = Bool->getValue();
- auto ReplaceWithExpression = [this, &Context, LHS, RHS,
- Bool](const Expr *ReplaceWith, bool Negated) {
- std::string Replacement =
- replacementExpression(Context, Negated, ReplaceWith);
- SourceRange Range(LHS->getBeginLoc(), RHS->getEndLoc());
- issueDiag(Context, Bool->getBeginLoc(), SimplifyOperatorDiagnostic, Range,
- Replacement);
- };
- switch (Op->getOpcode()) {
- case BO_LAnd:
- if (BoolValue)
- // expr && true -> expr
- ReplaceWithExpression(Other, /*Negated=*/false);
- else
- // expr && false -> false
- ReplaceWithExpression(Bool, /*Negated=*/false);
- break;
- case BO_LOr:
- if (BoolValue)
- // expr || true -> true
- ReplaceWithExpression(Bool, /*Negated=*/false);
- else
- // expr || false -> expr
- ReplaceWithExpression(Other, /*Negated=*/false);
- break;
- case BO_EQ:
- // expr == true -> expr, expr == false -> !expr
- ReplaceWithExpression(Other, /*Negated=*/!BoolValue);
- break;
- case BO_NE:
- // expr != true -> !expr, expr != false -> expr
- ReplaceWithExpression(Other, /*Negated=*/BoolValue);
- break;
- default:
- break;
- }
- }
- void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
- Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);
- Options.store(Opts, "ChainedConditionalAssignment",
- ChainedConditionalAssignment);
- Options.store(Opts, "SimplifyDeMorgan", SimplifyDeMorgan);
- Options.store(Opts, "SimplifyDeMorganRelaxed", SimplifyDeMorganRelaxed);
- }
- void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(translationUnitDecl(), this);
- }
- void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {
- Visitor(this, *Result.Context).traverse();
- }
- void SimplifyBooleanExprCheck::issueDiag(const ASTContext &Context,
- SourceLocation Loc,
- StringRef Description,
- SourceRange ReplacementRange,
- StringRef Replacement) {
- CharSourceRange CharRange =
- Lexer::makeFileCharRange(CharSourceRange::getTokenRange(ReplacementRange),
- Context.getSourceManager(), getLangOpts());
- DiagnosticBuilder Diag = diag(Loc, Description);
- if (!containsDiscardedTokens(Context, CharRange))
- Diag << FixItHint::CreateReplacement(CharRange, Replacement);
- }
- void SimplifyBooleanExprCheck::replaceWithThenStatement(
- const ASTContext &Context, const IfStmt *IfStatement,
- const Expr *BoolLiteral) {
- issueDiag(Context, BoolLiteral->getBeginLoc(), SimplifyConditionDiagnostic,
- IfStatement->getSourceRange(),
- getText(Context, *IfStatement->getThen()));
- }
- void SimplifyBooleanExprCheck::replaceWithElseStatement(
- const ASTContext &Context, const IfStmt *IfStatement,
- const Expr *BoolLiteral) {
- const Stmt *ElseStatement = IfStatement->getElse();
- issueDiag(Context, BoolLiteral->getBeginLoc(), SimplifyConditionDiagnostic,
- IfStatement->getSourceRange(),
- ElseStatement ? getText(Context, *ElseStatement) : "");
- }
- void SimplifyBooleanExprCheck::replaceWithCondition(
- const ASTContext &Context, const ConditionalOperator *Ternary,
- bool Negated) {
- std::string Replacement =
- replacementExpression(Context, Negated, Ternary->getCond());
- issueDiag(Context, Ternary->getTrueExpr()->getBeginLoc(),
- "redundant boolean literal in ternary expression result",
- Ternary->getSourceRange(), Replacement);
- }
- void SimplifyBooleanExprCheck::replaceWithReturnCondition(
- const ASTContext &Context, const IfStmt *If, const Expr *BoolLiteral,
- bool Negated) {
- StringRef Terminator = isa<CompoundStmt>(If->getElse()) ? ";" : "";
- std::string Condition =
- replacementExpression(Context, Negated, If->getCond());
- std::string Replacement = ("return " + Condition + Terminator).str();
- SourceLocation Start = BoolLiteral->getBeginLoc();
- issueDiag(Context, Start, SimplifyConditionalReturnDiagnostic,
- If->getSourceRange(), Replacement);
- }
- void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
- const ASTContext &Context, const ReturnStmt *Ret, bool Negated,
- const IfStmt *If, const Expr *ThenReturn) {
- const std::string Replacement =
- "return " + replacementExpression(Context, Negated, If->getCond());
- issueDiag(Context, ThenReturn->getBeginLoc(),
- SimplifyConditionalReturnDiagnostic,
- SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement);
- }
- void SimplifyBooleanExprCheck::replaceWithAssignment(const ASTContext &Context,
- const IfStmt *IfAssign,
- const Expr *Var,
- SourceLocation Loc,
- bool Negated) {
- SourceRange Range = IfAssign->getSourceRange();
- StringRef VariableName = getText(Context, *Var);
- StringRef Terminator = isa<CompoundStmt>(IfAssign->getElse()) ? ";" : "";
- std::string Condition =
- replacementExpression(Context, Negated, IfAssign->getCond());
- std::string Replacement =
- (VariableName + " = " + Condition + Terminator).str();
- issueDiag(Context, Loc, "redundant boolean literal in conditional assignment",
- Range, Replacement);
- }
- /// Swaps a \c BinaryOperator opcode from `&&` to `||` or vice-versa.
- static bool flipDemorganOperator(llvm::SmallVectorImpl<FixItHint> &Output,
- const BinaryOperator *BO) {
- assert(BO->isLogicalOp());
- if (BO->getOperatorLoc().isMacroID())
- return true;
- Output.push_back(FixItHint::CreateReplacement(
- BO->getOperatorLoc(), BO->getOpcode() == BO_LAnd ? "||" : "&&"));
- return false;
- }
- static BinaryOperatorKind getDemorganFlippedOperator(BinaryOperatorKind BO) {
- assert(BinaryOperator::isLogicalOp(BO));
- return BO == BO_LAnd ? BO_LOr : BO_LAnd;
- }
- static bool flipDemorganSide(SmallVectorImpl<FixItHint> &Fixes,
- const ASTContext &Ctx, const Expr *E,
- std::optional<BinaryOperatorKind> OuterBO);
- /// Inverts \p BinOp, Removing \p Parens if they exist and are safe to remove.
- /// returns \c true if there is any issue building the Fixes, \c false
- /// otherwise.
- static bool
- flipDemorganBinaryOperator(SmallVectorImpl<FixItHint> &Fixes,
- const ASTContext &Ctx, const BinaryOperator *BinOp,
- std::optional<BinaryOperatorKind> OuterBO,
- const ParenExpr *Parens = nullptr) {
- switch (BinOp->getOpcode()) {
- case BO_LAnd:
- case BO_LOr: {
- // if we have 'a && b' or 'a || b', use demorgan to flip it to '!a || !b'
- // or '!a && !b'.
- if (flipDemorganOperator(Fixes, BinOp))
- return true;
- auto NewOp = getDemorganFlippedOperator(BinOp->getOpcode());
- if (OuterBO) {
- // The inner parens are technically needed in a fix for
- // `!(!A1 && !(A2 || A3)) -> (A1 || (A2 && A3))`,
- // however this would trip the LogicalOpParentheses warning.
- // FIXME: Make this user configurable or detect if that warning is
- // enabled.
- constexpr bool LogicalOpParentheses = true;
- if (((*OuterBO == NewOp) || (!LogicalOpParentheses &&
- (*OuterBO == BO_LOr && NewOp == BO_LAnd))) &&
- Parens) {
- if (!Parens->getLParen().isMacroID() &&
- !Parens->getRParen().isMacroID()) {
- Fixes.push_back(FixItHint::CreateRemoval(Parens->getLParen()));
- Fixes.push_back(FixItHint::CreateRemoval(Parens->getRParen()));
- }
- }
- if (*OuterBO == BO_LAnd && NewOp == BO_LOr && !Parens) {
- Fixes.push_back(FixItHint::CreateInsertion(BinOp->getBeginLoc(), "("));
- Fixes.push_back(FixItHint::CreateInsertion(
- Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0,
- Ctx.getSourceManager(),
- Ctx.getLangOpts()),
- ")"));
- }
- }
- if (flipDemorganSide(Fixes, Ctx, BinOp->getLHS(), NewOp) ||
- flipDemorganSide(Fixes, Ctx, BinOp->getRHS(), NewOp))
- return true;
- return false;
- };
- case BO_LT:
- case BO_GT:
- case BO_LE:
- case BO_GE:
- case BO_EQ:
- case BO_NE:
- // For comparison operators, just negate the comparison.
- if (BinOp->getOperatorLoc().isMacroID())
- return true;
- Fixes.push_back(FixItHint::CreateReplacement(
- BinOp->getOperatorLoc(),
- BinaryOperator::getOpcodeStr(
- BinaryOperator::negateComparisonOp(BinOp->getOpcode()))));
- return false;
- default:
- // for any other binary operator, just use logical not and wrap in
- // parens.
- if (Parens) {
- if (Parens->getBeginLoc().isMacroID())
- return true;
- Fixes.push_back(FixItHint::CreateInsertion(Parens->getBeginLoc(), "!"));
- } else {
- if (BinOp->getBeginLoc().isMacroID() || BinOp->getEndLoc().isMacroID())
- return true;
- Fixes.append({FixItHint::CreateInsertion(BinOp->getBeginLoc(), "!("),
- FixItHint::CreateInsertion(
- Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0,
- Ctx.getSourceManager(),
- Ctx.getLangOpts()),
- ")")});
- }
- break;
- }
- return false;
- }
- static bool flipDemorganSide(SmallVectorImpl<FixItHint> &Fixes,
- const ASTContext &Ctx, const Expr *E,
- std::optional<BinaryOperatorKind> OuterBO) {
- if (isa<UnaryOperator>(E) && cast<UnaryOperator>(E)->getOpcode() == UO_LNot) {
- // if we have a not operator, '!a', just remove the '!'.
- if (cast<UnaryOperator>(E)->getOperatorLoc().isMacroID())
- return true;
- Fixes.push_back(
- FixItHint::CreateRemoval(cast<UnaryOperator>(E)->getOperatorLoc()));
- return false;
- }
- if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
- return flipDemorganBinaryOperator(Fixes, Ctx, BinOp, OuterBO);
- }
- if (const auto *Paren = dyn_cast<ParenExpr>(E)) {
- if (const auto *BinOp = dyn_cast<BinaryOperator>(Paren->getSubExpr())) {
- return flipDemorganBinaryOperator(Fixes, Ctx, BinOp, OuterBO, Paren);
- }
- }
- // Fallback case just insert a logical not operator.
- if (E->getBeginLoc().isMacroID())
- return true;
- Fixes.push_back(FixItHint::CreateInsertion(E->getBeginLoc(), "!"));
- return false;
- }
- static bool shouldRemoveParens(const Stmt *Parent,
- BinaryOperatorKind NewOuterBinary,
- const ParenExpr *Parens) {
- if (!Parens)
- return false;
- if (!Parent)
- return true;
- switch (Parent->getStmtClass()) {
- case Stmt::BinaryOperatorClass: {
- const auto *BO = cast<BinaryOperator>(Parent);
- if (BO->isAssignmentOp())
- return true;
- if (BO->isCommaOp())
- return true;
- if (BO->getOpcode() == NewOuterBinary)
- return true;
- return false;
- }
- case Stmt::UnaryOperatorClass:
- case Stmt::CXXRewrittenBinaryOperatorClass:
- return false;
- default:
- return true;
- }
- }
- bool SimplifyBooleanExprCheck::reportDeMorgan(const ASTContext &Context,
- const UnaryOperator *Outer,
- const BinaryOperator *Inner,
- bool TryOfferFix,
- const Stmt *Parent,
- const ParenExpr *Parens) {
- assert(Outer);
- assert(Inner);
- assert(Inner->isLogicalOp());
- auto Diag =
- diag(Outer->getBeginLoc(),
- "boolean expression can be simplified by DeMorgan's theorem");
- Diag << Outer->getSourceRange();
- // If we have already fixed this with a previous fix, don't attempt any fixes
- if (!TryOfferFix)
- return false;
- if (Outer->getOperatorLoc().isMacroID())
- return false;
- SmallVector<FixItHint> Fixes;
- auto NewOpcode = getDemorganFlippedOperator(Inner->getOpcode());
- if (shouldRemoveParens(Parent, NewOpcode, Parens)) {
- Fixes.push_back(FixItHint::CreateRemoval(
- SourceRange(Outer->getOperatorLoc(), Parens->getLParen())));
- Fixes.push_back(FixItHint::CreateRemoval(Parens->getRParen()));
- } else {
- Fixes.push_back(FixItHint::CreateRemoval(Outer->getOperatorLoc()));
- }
- if (flipDemorganOperator(Fixes, Inner))
- return false;
- if (flipDemorganSide(Fixes, Context, Inner->getLHS(), NewOpcode) ||
- flipDemorganSide(Fixes, Context, Inner->getRHS(), NewOpcode))
- return false;
- Diag << Fixes;
- return true;
- }
- } // namespace clang::tidy::readability
|