1001 lines · cpp
1//===----------------------------------------------------------------------===//2//3// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.4// See https://llvm.org/LICENSE.txt for license information.5// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception6//7//===----------------------------------------------------------------------===//8 9#include "SimplifyBooleanExprCheck.h"10#include "clang/AST/Expr.h"11#include "clang/AST/RecursiveASTVisitor.h"12#include "clang/Basic/DiagnosticIDs.h"13#include "clang/Lex/Lexer.h"14#include "llvm/Support/SaveAndRestore.h"15 16#include <optional>17#include <string>18#include <utility>19 20using namespace clang::ast_matchers;21 22namespace clang::tidy::readability {23 24static StringRef getText(const ASTContext &Context, SourceRange Range) {25 return Lexer::getSourceText(CharSourceRange::getTokenRange(Range),26 Context.getSourceManager(),27 Context.getLangOpts());28}29 30template <typename T>31static StringRef getText(const ASTContext &Context, T &Node) {32 return getText(Context, Node.getSourceRange());33}34 35static constexpr char SimplifyOperatorDiagnostic[] =36 "redundant boolean literal supplied to boolean operator";37static constexpr char SimplifyConditionDiagnostic[] =38 "redundant boolean literal in if statement condition";39static constexpr char SimplifyConditionalReturnDiagnostic[] =40 "redundant boolean literal in conditional return statement";41 42static bool needsParensAfterUnaryNegation(const Expr *E) {43 E = E->IgnoreImpCasts();44 if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))45 return true;46 47 if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E))48 return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&49 Op->getOperator() != OO_Subscript;50 51 return false;52}53 54static std::pair<BinaryOperatorKind, BinaryOperatorKind> Opposites[] = {55 {BO_LT, BO_GE}, {BO_GT, BO_LE}, {BO_EQ, BO_NE}};56 57static StringRef negatedOperator(const BinaryOperator *BinOp) {58 const BinaryOperatorKind Opcode = BinOp->getOpcode();59 for (auto NegatableOp : Opposites) {60 if (Opcode == NegatableOp.first)61 return BinaryOperator::getOpcodeStr(NegatableOp.second);62 if (Opcode == NegatableOp.second)63 return BinaryOperator::getOpcodeStr(NegatableOp.first);64 }65 return {};66}67 68static std::pair<OverloadedOperatorKind, StringRef> OperatorNames[] = {69 {OO_EqualEqual, "=="}, {OO_ExclaimEqual, "!="}, {OO_Less, "<"},70 {OO_GreaterEqual, ">="}, {OO_Greater, ">"}, {OO_LessEqual, "<="}};71 72static StringRef getOperatorName(OverloadedOperatorKind OpKind) {73 for (auto Name : OperatorNames) {74 if (Name.first == OpKind)75 return Name.second;76 }77 78 return {};79}80 81static std::pair<OverloadedOperatorKind, OverloadedOperatorKind>82 OppositeOverloads[] = {{OO_EqualEqual, OO_ExclaimEqual},83 {OO_Less, OO_GreaterEqual},84 {OO_Greater, OO_LessEqual}};85 86static StringRef negatedOperator(const CXXOperatorCallExpr *OpCall) {87 const OverloadedOperatorKind Opcode = OpCall->getOperator();88 for (auto NegatableOp : OppositeOverloads) {89 if (Opcode == NegatableOp.first)90 return getOperatorName(NegatableOp.second);91 if (Opcode == NegatableOp.second)92 return getOperatorName(NegatableOp.first);93 }94 return {};95}96 97static std::string asBool(StringRef Text, bool NeedsStaticCast) {98 if (NeedsStaticCast)99 return ("static_cast<bool>(" + Text + ")").str();100 101 return std::string(Text);102}103 104static bool needsNullPtrComparison(const Expr *E) {105 if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))106 return ImpCast->getCastKind() == CK_PointerToBoolean ||107 ImpCast->getCastKind() == CK_MemberPointerToBoolean;108 109 return false;110}111 112static bool needsZeroComparison(const Expr *E) {113 if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E))114 return ImpCast->getCastKind() == CK_IntegralToBoolean;115 116 return false;117}118 119static bool needsStaticCast(const Expr *E) {120 if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {121 if (ImpCast->getCastKind() == CK_UserDefinedConversion &&122 ImpCast->getSubExpr()->getType()->isBooleanType()) {123 if (const auto *MemCall =124 dyn_cast<CXXMemberCallExpr>(ImpCast->getSubExpr())) {125 if (const auto *MemDecl =126 dyn_cast<CXXConversionDecl>(MemCall->getMethodDecl())) {127 if (MemDecl->isExplicit())128 return true;129 }130 }131 }132 }133 134 E = E->IgnoreImpCasts();135 return !E->getType()->isBooleanType();136}137 138static std::string compareExpressionToConstant(const ASTContext &Context,139 const Expr *E, bool Negated,140 const char *Constant) {141 E = E->IgnoreImpCasts();142 const std::string ExprText =143 (isa<BinaryOperator>(E) ? ("(" + getText(Context, *E) + ")")144 : getText(Context, *E))145 .str();146 return ExprText + " " + (Negated ? "!=" : "==") + " " + Constant;147}148 149static std::string compareExpressionToNullPtr(const ASTContext &Context,150 const Expr *E, bool Negated) {151 const char *NullPtr = Context.getLangOpts().CPlusPlus11 ? "nullptr" : "NULL";152 return compareExpressionToConstant(Context, E, Negated, NullPtr);153}154 155static std::string compareExpressionToZero(const ASTContext &Context,156 const Expr *E, bool Negated) {157 return compareExpressionToConstant(Context, E, Negated, "0");158}159 160static std::string replacementExpression(const ASTContext &Context,161 bool Negated, const Expr *E) {162 E = E->IgnoreParenBaseCasts();163 if (const auto *EC = dyn_cast<ExprWithCleanups>(E))164 E = EC->getSubExpr();165 166 const bool NeedsStaticCast = needsStaticCast(E);167 if (Negated) {168 if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {169 if (UnOp->getOpcode() == UO_LNot) {170 if (needsNullPtrComparison(UnOp->getSubExpr()))171 return compareExpressionToNullPtr(Context, UnOp->getSubExpr(), true);172 173 if (needsZeroComparison(UnOp->getSubExpr()))174 return compareExpressionToZero(Context, UnOp->getSubExpr(), true);175 176 return replacementExpression(Context, false, UnOp->getSubExpr());177 }178 }179 180 if (needsNullPtrComparison(E))181 return compareExpressionToNullPtr(Context, E, false);182 183 if (needsZeroComparison(E))184 return compareExpressionToZero(Context, E, false);185 186 StringRef NegatedOperator;187 const Expr *LHS = nullptr;188 const Expr *RHS = nullptr;189 if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {190 NegatedOperator = negatedOperator(BinOp);191 LHS = BinOp->getLHS();192 RHS = BinOp->getRHS();193 } else if (const auto *OpExpr = dyn_cast<CXXOperatorCallExpr>(E)) {194 if (OpExpr->getNumArgs() == 2) {195 NegatedOperator = negatedOperator(OpExpr);196 LHS = OpExpr->getArg(0);197 RHS = OpExpr->getArg(1);198 }199 }200 if (!NegatedOperator.empty() && LHS && RHS)201 return (asBool((getText(Context, *LHS) + " " + NegatedOperator + " " +202 getText(Context, *RHS))203 .str(),204 NeedsStaticCast));205 206 const StringRef Text = getText(Context, *E);207 if (!NeedsStaticCast && needsParensAfterUnaryNegation(E))208 return ("!(" + Text + ")").str();209 210 if (needsNullPtrComparison(E))211 return compareExpressionToNullPtr(Context, E, false);212 213 if (needsZeroComparison(E))214 return compareExpressionToZero(Context, E, false);215 216 return ("!" + asBool(Text, NeedsStaticCast));217 }218 219 if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) {220 if (UnOp->getOpcode() == UO_LNot) {221 if (needsNullPtrComparison(UnOp->getSubExpr()))222 return compareExpressionToNullPtr(Context, UnOp->getSubExpr(), false);223 224 if (needsZeroComparison(UnOp->getSubExpr()))225 return compareExpressionToZero(Context, UnOp->getSubExpr(), false);226 }227 }228 229 if (needsNullPtrComparison(E))230 return compareExpressionToNullPtr(Context, E, true);231 232 if (needsZeroComparison(E))233 return compareExpressionToZero(Context, E, true);234 235 return asBool(getText(Context, *E), NeedsStaticCast);236}237 238static bool containsDiscardedTokens(const ASTContext &Context,239 CharSourceRange CharRange) {240 std::string ReplacementText =241 Lexer::getSourceText(CharRange, Context.getSourceManager(),242 Context.getLangOpts())243 .str();244 Lexer Lex(CharRange.getBegin(), Context.getLangOpts(), ReplacementText.data(),245 ReplacementText.data(),246 ReplacementText.data() + ReplacementText.size());247 Lex.SetCommentRetentionState(true);248 249 Token Tok;250 while (!Lex.LexFromRawLexer(Tok)) {251 if (Tok.is(tok::TokenKind::comment) || Tok.is(tok::TokenKind::hash))252 return true;253 }254 255 return false;256}257 258class SimplifyBooleanExprCheck::Visitor : public RecursiveASTVisitor<Visitor> {259 using Base = RecursiveASTVisitor<Visitor>;260 261public:262 Visitor(SimplifyBooleanExprCheck *Check, ASTContext &Context)263 : Check(Check), Context(Context) {}264 265 bool traverse() { return TraverseAST(Context); }266 267 static bool shouldIgnore(Stmt *S) {268 switch (S->getStmtClass()) {269 case Stmt::ImplicitCastExprClass:270 case Stmt::MaterializeTemporaryExprClass:271 case Stmt::CXXBindTemporaryExprClass:272 return true;273 default:274 return false;275 }276 }277 278 bool dataTraverseStmtPre(Stmt *S) {279 if (!S) {280 return true;281 }282 if (Check->canBeBypassed(S))283 return false;284 if (!shouldIgnore(S))285 StmtStack.push_back(S);286 return true;287 }288 289 bool dataTraverseStmtPost(Stmt *S) {290 if (S && !shouldIgnore(S)) {291 assert(StmtStack.back() == S);292 StmtStack.pop_back();293 }294 return true;295 }296 297 bool VisitBinaryOperator(const BinaryOperator *Op) const {298 Check->reportBinOp(Context, Op);299 return true;300 }301 302 // Extracts a bool if an expression is (true|false|!true|!false);303 static std::optional<bool> getAsBoolLiteral(const Expr *E, bool FilterMacro) {304 if (const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(E)) {305 if (FilterMacro && Bool->getBeginLoc().isMacroID())306 return std::nullopt;307 return Bool->getValue();308 }309 if (const auto *UnaryOp = dyn_cast<UnaryOperator>(E)) {310 if (FilterMacro && UnaryOp->getBeginLoc().isMacroID())311 return std::nullopt;312 if (UnaryOp->getOpcode() == UO_LNot)313 if (std::optional<bool> Res = getAsBoolLiteral(314 UnaryOp->getSubExpr()->IgnoreImplicit(), FilterMacro))315 return !*Res;316 }317 return std::nullopt;318 }319 320 template <typename Node> struct NodeAndBool {321 const Node *Item = nullptr;322 bool Bool = false;323 324 operator bool() const { return Item != nullptr; }325 };326 327 using ExprAndBool = NodeAndBool<Expr>;328 using DeclAndBool = NodeAndBool<Decl>;329 330 /// Detect's return (true|false|!true|!false);331 static ExprAndBool parseReturnLiteralBool(const Stmt *S) {332 const auto *RS = dyn_cast<ReturnStmt>(S);333 if (!RS || !RS->getRetValue())334 return {};335 if (std::optional<bool> Ret =336 getAsBoolLiteral(RS->getRetValue()->IgnoreImplicit(), false)) {337 return {RS->getRetValue(), *Ret};338 }339 return {};340 }341 342 /// If \p S is not a \c CompoundStmt, applies F on \p S, otherwise if there is343 /// only 1 statement in the \c CompoundStmt, applies F on that single344 /// statement.345 template <typename Functor>346 static auto checkSingleStatement(Stmt *S, Functor F) -> decltype(F(S)) {347 if (auto *CS = dyn_cast<CompoundStmt>(S)) {348 if (CS->size() == 1)349 return F(CS->body_front());350 return {};351 }352 return F(S);353 }354 355 Stmt *parent() const {356 return StmtStack.size() < 2 ? nullptr : StmtStack[StmtStack.size() - 2];357 }358 359 bool VisitIfStmt(IfStmt *If) {360 // Skip any if's that have a condition var or an init statement, or are361 // "if consteval" statements.362 if (If->hasInitStorage() || If->hasVarStorage() || If->isConsteval())363 return true;364 /*365 * if (true) ThenStmt(); -> ThenStmt();366 * if (false) ThenStmt(); -> <Empty>;367 * if (false) ThenStmt(); else ElseStmt() -> ElseStmt();368 */369 const Expr *Cond = If->getCond()->IgnoreImplicit();370 if (std::optional<bool> Bool = getAsBoolLiteral(Cond, true)) {371 if (*Bool)372 Check->replaceWithThenStatement(Context, If, Cond);373 else374 Check->replaceWithElseStatement(Context, If, Cond);375 }376 377 if (If->getElse()) {378 /*379 * if (Cond) return true; else return false; -> return Cond;380 * if (Cond) return false; else return true; -> return !Cond;381 */382 if (const ExprAndBool ThenReturnBool =383 checkSingleStatement(If->getThen(), parseReturnLiteralBool)) {384 const ExprAndBool ElseReturnBool =385 checkSingleStatement(If->getElse(), parseReturnLiteralBool);386 if (ElseReturnBool && ThenReturnBool.Bool != ElseReturnBool.Bool) {387 if (Check->ChainedConditionalReturn ||388 !isa_and_nonnull<IfStmt>(parent())) {389 Check->replaceWithReturnCondition(Context, If, ThenReturnBool.Item,390 ElseReturnBool.Bool);391 }392 }393 } else {394 /*395 * if (Cond) A = true; else A = false; -> A = Cond;396 * if (Cond) A = false; else A = true; -> A = !Cond;397 */398 Expr *Var = nullptr;399 SourceLocation Loc;400 auto VarBoolAssignmentMatcher = [&Var,401 &Loc](const Stmt *S) -> DeclAndBool {402 const auto *BO = dyn_cast<BinaryOperator>(S);403 if (!BO || BO->getOpcode() != BO_Assign)404 return {};405 std::optional<bool> RightasBool =406 getAsBoolLiteral(BO->getRHS()->IgnoreImplicit(), false);407 if (!RightasBool)408 return {};409 Expr *IgnImp = BO->getLHS()->IgnoreImplicit();410 if (!Var) {411 // We only need to track these for the Then branch.412 Loc = BO->getRHS()->getBeginLoc();413 Var = IgnImp;414 }415 if (auto *DRE = dyn_cast<DeclRefExpr>(IgnImp))416 return {DRE->getDecl(), *RightasBool};417 if (auto *ME = dyn_cast<MemberExpr>(IgnImp))418 return {ME->getMemberDecl(), *RightasBool};419 return {};420 };421 if (const DeclAndBool ThenAssignment =422 checkSingleStatement(If->getThen(), VarBoolAssignmentMatcher)) {423 const DeclAndBool ElseAssignment =424 checkSingleStatement(If->getElse(), VarBoolAssignmentMatcher);425 if (ElseAssignment.Item == ThenAssignment.Item &&426 ElseAssignment.Bool != ThenAssignment.Bool) {427 if (Check->ChainedConditionalAssignment ||428 !isa_and_nonnull<IfStmt>(parent())) {429 Check->replaceWithAssignment(Context, If, Var, Loc,430 ElseAssignment.Bool);431 }432 }433 }434 }435 }436 return true;437 }438 439 bool VisitConditionalOperator(ConditionalOperator *Cond) {440 /*441 * Condition ? true : false; -> Condition442 * Condition ? false : true; -> !Condition;443 */444 if (std::optional<bool> Then =445 getAsBoolLiteral(Cond->getTrueExpr()->IgnoreImplicit(), false)) {446 if (std::optional<bool> Else =447 getAsBoolLiteral(Cond->getFalseExpr()->IgnoreImplicit(), false)) {448 if (*Then != *Else)449 Check->replaceWithCondition(Context, Cond, *Else);450 }451 }452 return true;453 }454 455 bool VisitCompoundStmt(CompoundStmt *CS) {456 if (CS->size() < 2)457 return true;458 bool CurIf = false, PrevIf = false;459 for (auto First = CS->body_begin(), Second = std::next(First),460 End = CS->body_end();461 Second != End; ++Second, ++First) {462 PrevIf = CurIf;463 CurIf = isa<IfStmt>(*First);464 const ExprAndBool TrailingReturnBool = parseReturnLiteralBool(*Second);465 if (!TrailingReturnBool)466 continue;467 468 if (CurIf) {469 /*470 * if (Cond) return true; return false; -> return Cond;471 * if (Cond) return false; return true; -> return !Cond;472 */473 auto *If = cast<IfStmt>(*First);474 if (!If->hasInitStorage() && !If->hasVarStorage() &&475 !If->isConsteval()) {476 const ExprAndBool ThenReturnBool =477 checkSingleStatement(If->getThen(), parseReturnLiteralBool);478 if (ThenReturnBool &&479 ThenReturnBool.Bool != TrailingReturnBool.Bool) {480 if ((Check->ChainedConditionalReturn || !PrevIf) &&481 If->getElse() == nullptr) {482 Check->replaceCompoundReturnWithCondition(483 Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,484 If, ThenReturnBool.Item);485 }486 }487 }488 } else if (isa<LabelStmt, CaseStmt, DefaultStmt>(*First)) {489 /*490 * (case X|label_X|default): if (Cond) return BoolLiteral;491 * return !BoolLiteral492 */493 Stmt *SubStmt =494 isa<LabelStmt>(*First) ? cast<LabelStmt>(*First)->getSubStmt()495 : isa<CaseStmt>(*First) ? cast<CaseStmt>(*First)->getSubStmt()496 : cast<DefaultStmt>(*First)->getSubStmt();497 auto *SubIf = dyn_cast<IfStmt>(SubStmt);498 if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&499 !SubIf->hasVarStorage() && !SubIf->isConsteval()) {500 const ExprAndBool ThenReturnBool =501 checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);502 if (ThenReturnBool &&503 ThenReturnBool.Bool != TrailingReturnBool.Bool) {504 Check->replaceCompoundReturnWithCondition(505 Context, cast<ReturnStmt>(*Second), TrailingReturnBool.Bool,506 SubIf, ThenReturnBool.Item);507 }508 }509 }510 }511 return true;512 }513 514 bool isExpectedUnaryLNot(const Expr *E) {515 return !Check->canBeBypassed(E) && isa<UnaryOperator>(E) &&516 cast<UnaryOperator>(E)->getOpcode() == UO_LNot;517 }518 519 bool isExpectedBinaryOp(const Expr *E) {520 const auto *BinaryOp = dyn_cast<BinaryOperator>(E);521 return !Check->canBeBypassed(E) && BinaryOp && BinaryOp->isLogicalOp() &&522 BinaryOp->getType()->isBooleanType();523 }524 525 template <typename Functor>526 static bool checkEitherSide(const BinaryOperator *BO, Functor Func) {527 return Func(BO->getLHS()) || Func(BO->getRHS());528 }529 530 bool nestedDemorgan(const Expr *E, unsigned NestingLevel) {531 const auto *BO = dyn_cast<BinaryOperator>(E->IgnoreUnlessSpelledInSource());532 if (!BO)533 return false;534 if (!BO->getType()->isBooleanType())535 return false;536 switch (BO->getOpcode()) {537 case BO_LT:538 case BO_GT:539 case BO_LE:540 case BO_GE:541 case BO_EQ:542 case BO_NE:543 return true;544 case BO_LAnd:545 case BO_LOr:546 return checkEitherSide(547 BO,548 [this](const Expr *E) { return isExpectedUnaryLNot(E); }) ||549 (NestingLevel &&550 checkEitherSide(BO, [this, NestingLevel](const Expr *E) {551 return nestedDemorgan(E, NestingLevel - 1);552 }));553 default:554 return false;555 }556 }557 558 bool TraverseUnaryOperator(UnaryOperator *Op) {559 if (!Check->SimplifyDeMorgan || Op->getOpcode() != UO_LNot)560 return Base::TraverseUnaryOperator(Op);561 const Expr *SubImp = Op->getSubExpr()->IgnoreImplicit();562 const auto *Parens = dyn_cast<ParenExpr>(SubImp);563 const Expr *SubExpr =564 Parens ? Parens->getSubExpr()->IgnoreImplicit() : SubImp;565 if (!isExpectedBinaryOp(SubExpr))566 return Base::TraverseUnaryOperator(Op);567 const auto *BinaryOp = cast<BinaryOperator>(SubExpr);568 if (Check->SimplifyDeMorganRelaxed ||569 checkEitherSide(570 BinaryOp,571 [this](const Expr *E) { return isExpectedUnaryLNot(E); }) ||572 checkEitherSide(573 BinaryOp, [this](const Expr *E) { return nestedDemorgan(E, 1); })) {574 if (Check->reportDeMorgan(Context, Op, BinaryOp, !IsProcessing, parent(),575 Parens) &&576 !Check->areDiagsSelfContained()) {577 const llvm::SaveAndRestore RAII(IsProcessing, true);578 return Base::TraverseUnaryOperator(Op);579 }580 }581 return Base::TraverseUnaryOperator(Op);582 }583 584private:585 bool IsProcessing = false;586 SimplifyBooleanExprCheck *Check;587 SmallVector<Stmt *, 32> StmtStack;588 ASTContext &Context;589};590 591SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,592 ClangTidyContext *Context)593 : ClangTidyCheck(Name, Context),594 IgnoreMacros(Options.get("IgnoreMacros", false)),595 ChainedConditionalReturn(Options.get("ChainedConditionalReturn", false)),596 ChainedConditionalAssignment(597 Options.get("ChainedConditionalAssignment", false)),598 SimplifyDeMorgan(Options.get("SimplifyDeMorgan", true)),599 SimplifyDeMorganRelaxed(Options.get("SimplifyDeMorganRelaxed", false)) {600 if (SimplifyDeMorganRelaxed && !SimplifyDeMorgan)601 configurationDiag("%0: 'SimplifyDeMorganRelaxed' cannot be enabled "602 "without 'SimplifyDeMorgan' enabled")603 << Name;604}605 606static bool containsBoolLiteral(const Expr *E) {607 if (!E)608 return false;609 E = E->IgnoreParenImpCasts();610 if (isa<CXXBoolLiteralExpr>(E))611 return true;612 if (const auto *BinOp = dyn_cast<BinaryOperator>(E))613 return containsBoolLiteral(BinOp->getLHS()) ||614 containsBoolLiteral(BinOp->getRHS());615 if (const auto *UnaryOp = dyn_cast<UnaryOperator>(E))616 return containsBoolLiteral(UnaryOp->getSubExpr());617 return false;618}619 620void SimplifyBooleanExprCheck::reportBinOp(const ASTContext &Context,621 const BinaryOperator *Op) {622 const auto *LHS = Op->getLHS()->IgnoreParenImpCasts();623 const auto *RHS = Op->getRHS()->IgnoreParenImpCasts();624 625 const CXXBoolLiteralExpr *Bool = nullptr;626 const Expr *Other = nullptr;627 if ((Bool = dyn_cast<CXXBoolLiteralExpr>(LHS)) != nullptr)628 Other = RHS;629 else if ((Bool = dyn_cast<CXXBoolLiteralExpr>(RHS)) != nullptr)630 Other = LHS;631 else632 return;633 634 if (Bool->getBeginLoc().isMacroID())635 return;636 637 // FIXME: why do we need this?638 if (!isa<CXXBoolLiteralExpr>(Other) && containsBoolLiteral(Other))639 return;640 641 const bool BoolValue = Bool->getValue();642 643 auto ReplaceWithExpression = [this, &Context, LHS, RHS,644 Bool](const Expr *ReplaceWith, bool Negated) {645 const std::string Replacement =646 replacementExpression(Context, Negated, ReplaceWith);647 const SourceRange Range(LHS->getBeginLoc(), RHS->getEndLoc());648 issueDiag(Context, Bool->getBeginLoc(), SimplifyOperatorDiagnostic, Range,649 Replacement);650 };651 652 switch (Op->getOpcode()) {653 case BO_LAnd:654 if (BoolValue)655 // expr && true -> expr656 ReplaceWithExpression(Other, /*Negated=*/false);657 else658 // expr && false -> false659 ReplaceWithExpression(Bool, /*Negated=*/false);660 break;661 case BO_LOr:662 if (BoolValue)663 // expr || true -> true664 ReplaceWithExpression(Bool, /*Negated=*/false);665 else666 // expr || false -> expr667 ReplaceWithExpression(Other, /*Negated=*/false);668 break;669 case BO_EQ:670 // expr == true -> expr, expr == false -> !expr671 ReplaceWithExpression(Other, /*Negated=*/!BoolValue);672 break;673 case BO_NE:674 // expr != true -> !expr, expr != false -> expr675 ReplaceWithExpression(Other, /*Negated=*/BoolValue);676 break;677 default:678 break;679 }680}681 682void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {683 Options.store(Opts, "IgnoreMacros", IgnoreMacros);684 Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn);685 Options.store(Opts, "ChainedConditionalAssignment",686 ChainedConditionalAssignment);687 Options.store(Opts, "SimplifyDeMorgan", SimplifyDeMorgan);688 Options.store(Opts, "SimplifyDeMorganRelaxed", SimplifyDeMorganRelaxed);689}690 691void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {692 Finder->addMatcher(translationUnitDecl(), this);693}694 695void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) {696 Visitor(this, *Result.Context).traverse();697}698 699bool SimplifyBooleanExprCheck::canBeBypassed(const Stmt *S) const {700 return IgnoreMacros && S->getBeginLoc().isMacroID();701}702 703/// @brief return true when replacement created.704bool SimplifyBooleanExprCheck::issueDiag(const ASTContext &Context,705 SourceLocation Loc,706 StringRef Description,707 SourceRange ReplacementRange,708 StringRef Replacement) {709 const CharSourceRange CharRange =710 Lexer::makeFileCharRange(CharSourceRange::getTokenRange(ReplacementRange),711 Context.getSourceManager(), getLangOpts());712 713 const DiagnosticBuilder Diag = diag(Loc, Description);714 const bool HasReplacement = !containsDiscardedTokens(Context, CharRange);715 if (HasReplacement)716 Diag << FixItHint::CreateReplacement(CharRange, Replacement);717 return HasReplacement;718}719 720void SimplifyBooleanExprCheck::replaceWithThenStatement(721 const ASTContext &Context, const IfStmt *IfStatement,722 const Expr *BoolLiteral) {723 issueDiag(Context, BoolLiteral->getBeginLoc(), SimplifyConditionDiagnostic,724 IfStatement->getSourceRange(),725 getText(Context, *IfStatement->getThen()));726}727 728void SimplifyBooleanExprCheck::replaceWithElseStatement(729 const ASTContext &Context, const IfStmt *IfStatement,730 const Expr *BoolLiteral) {731 const Stmt *ElseStatement = IfStatement->getElse();732 issueDiag(Context, BoolLiteral->getBeginLoc(), SimplifyConditionDiagnostic,733 IfStatement->getSourceRange(),734 ElseStatement ? getText(Context, *ElseStatement) : "");735}736 737void SimplifyBooleanExprCheck::replaceWithCondition(738 const ASTContext &Context, const ConditionalOperator *Ternary,739 bool Negated) {740 const std::string Replacement =741 replacementExpression(Context, Negated, Ternary->getCond());742 issueDiag(Context, Ternary->getTrueExpr()->getBeginLoc(),743 "redundant boolean literal in ternary expression result",744 Ternary->getSourceRange(), Replacement);745}746 747void SimplifyBooleanExprCheck::replaceWithReturnCondition(748 const ASTContext &Context, const IfStmt *If, const Expr *BoolLiteral,749 bool Negated) {750 const StringRef Terminator = isa<CompoundStmt>(If->getElse()) ? ";" : "";751 const std::string Condition =752 replacementExpression(Context, Negated, If->getCond());753 const std::string Replacement = ("return " + Condition + Terminator).str();754 const SourceLocation Start = BoolLiteral->getBeginLoc();755 756 const bool HasReplacement =757 issueDiag(Context, Start, SimplifyConditionalReturnDiagnostic,758 If->getSourceRange(), Replacement);759 760 if (!HasReplacement) {761 const SourceRange ConditionRange = If->getCond()->getSourceRange();762 if (ConditionRange.isValid())763 diag(ConditionRange.getBegin(), "conditions that can be simplified",764 DiagnosticIDs::Note)765 << ConditionRange;766 }767}768 769void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(770 const ASTContext &Context, const ReturnStmt *Ret, bool Negated,771 const IfStmt *If, const Expr *ThenReturn) {772 const std::string Replacement =773 "return " + replacementExpression(Context, Negated, If->getCond());774 775 const bool HasReplacement = issueDiag(776 Context, ThenReturn->getBeginLoc(), SimplifyConditionalReturnDiagnostic,777 SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement);778 779 if (!HasReplacement) {780 const SourceRange ConditionRange = If->getCond()->getSourceRange();781 if (ConditionRange.isValid())782 diag(ConditionRange.getBegin(), "conditions that can be simplified",783 DiagnosticIDs::Note)784 << ConditionRange;785 const SourceRange ReturnRange = Ret->getSourceRange();786 if (ReturnRange.isValid())787 diag(ReturnRange.getBegin(), "return statement that can be simplified",788 DiagnosticIDs::Note)789 << ReturnRange;790 }791}792 793void SimplifyBooleanExprCheck::replaceWithAssignment(const ASTContext &Context,794 const IfStmt *IfAssign,795 const Expr *Var,796 SourceLocation Loc,797 bool Negated) {798 const SourceRange Range = IfAssign->getSourceRange();799 const StringRef VariableName = getText(Context, *Var);800 const StringRef Terminator =801 isa<CompoundStmt>(IfAssign->getElse()) ? ";" : "";802 const std::string Condition =803 replacementExpression(Context, Negated, IfAssign->getCond());804 const std::string Replacement =805 (VariableName + " = " + Condition + Terminator).str();806 issueDiag(Context, Loc, "redundant boolean literal in conditional assignment",807 Range, Replacement);808}809 810/// Swaps a \c BinaryOperator opcode from `&&` to `||` or vice-versa.811static bool flipDemorganOperator(llvm::SmallVectorImpl<FixItHint> &Output,812 const BinaryOperator *BO) {813 assert(BO->isLogicalOp());814 if (BO->getOperatorLoc().isMacroID())815 return true;816 Output.push_back(FixItHint::CreateReplacement(817 BO->getOperatorLoc(), BO->getOpcode() == BO_LAnd ? "||" : "&&"));818 return false;819}820 821static BinaryOperatorKind getDemorganFlippedOperator(BinaryOperatorKind BO) {822 assert(BinaryOperator::isLogicalOp(BO));823 return BO == BO_LAnd ? BO_LOr : BO_LAnd;824}825 826static bool flipDemorganSide(SmallVectorImpl<FixItHint> &Fixes,827 const ASTContext &Ctx, const Expr *E,828 std::optional<BinaryOperatorKind> OuterBO);829 830/// Inverts \p BinOp, Removing \p Parens if they exist and are safe to remove.831/// returns \c true if there is any issue building the Fixes, \c false832/// otherwise.833static bool834flipDemorganBinaryOperator(SmallVectorImpl<FixItHint> &Fixes,835 const ASTContext &Ctx, const BinaryOperator *BinOp,836 std::optional<BinaryOperatorKind> OuterBO,837 const ParenExpr *Parens = nullptr) {838 switch (BinOp->getOpcode()) {839 case BO_LAnd:840 case BO_LOr: {841 // if we have 'a && b' or 'a || b', use demorgan to flip it to '!a || !b'842 // or '!a && !b'.843 if (flipDemorganOperator(Fixes, BinOp))844 return true;845 auto NewOp = getDemorganFlippedOperator(BinOp->getOpcode());846 if (OuterBO) {847 // The inner parens are technically needed in a fix for848 // `!(!A1 && !(A2 || A3)) -> (A1 || (A2 && A3))`,849 // however this would trip the LogicalOpParentheses warning.850 // FIXME: Make this user configurable or detect if that warning is851 // enabled.852 constexpr bool LogicalOpParentheses = true;853 if (((*OuterBO == NewOp) || (!LogicalOpParentheses &&854 (*OuterBO == BO_LOr && NewOp == BO_LAnd))) &&855 Parens) {856 if (!Parens->getLParen().isMacroID() &&857 !Parens->getRParen().isMacroID()) {858 Fixes.push_back(FixItHint::CreateRemoval(Parens->getLParen()));859 Fixes.push_back(FixItHint::CreateRemoval(Parens->getRParen()));860 }861 }862 if (*OuterBO == BO_LAnd && NewOp == BO_LOr && !Parens) {863 Fixes.push_back(FixItHint::CreateInsertion(BinOp->getBeginLoc(), "("));864 Fixes.push_back(FixItHint::CreateInsertion(865 Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0,866 Ctx.getSourceManager(),867 Ctx.getLangOpts()),868 ")"));869 }870 }871 if (flipDemorganSide(Fixes, Ctx, BinOp->getLHS(), NewOp) ||872 flipDemorganSide(Fixes, Ctx, BinOp->getRHS(), NewOp))873 return true;874 return false;875 };876 case BO_LT:877 case BO_GT:878 case BO_LE:879 case BO_GE:880 case BO_EQ:881 case BO_NE:882 // For comparison operators, just negate the comparison.883 if (BinOp->getOperatorLoc().isMacroID())884 return true;885 Fixes.push_back(FixItHint::CreateReplacement(886 BinOp->getOperatorLoc(),887 BinaryOperator::getOpcodeStr(888 BinaryOperator::negateComparisonOp(BinOp->getOpcode()))));889 return false;890 default:891 // for any other binary operator, just use logical not and wrap in892 // parens.893 if (Parens) {894 if (Parens->getBeginLoc().isMacroID())895 return true;896 Fixes.push_back(FixItHint::CreateInsertion(Parens->getBeginLoc(), "!"));897 } else {898 if (BinOp->getBeginLoc().isMacroID() || BinOp->getEndLoc().isMacroID())899 return true;900 Fixes.append({FixItHint::CreateInsertion(BinOp->getBeginLoc(), "!("),901 FixItHint::CreateInsertion(902 Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0,903 Ctx.getSourceManager(),904 Ctx.getLangOpts()),905 ")")});906 }907 break;908 }909 return false;910}911 912static bool flipDemorganSide(SmallVectorImpl<FixItHint> &Fixes,913 const ASTContext &Ctx, const Expr *E,914 std::optional<BinaryOperatorKind> OuterBO) {915 if (isa<UnaryOperator>(E) && cast<UnaryOperator>(E)->getOpcode() == UO_LNot) {916 // if we have a not operator, '!a', just remove the '!'.917 if (cast<UnaryOperator>(E)->getOperatorLoc().isMacroID())918 return true;919 Fixes.push_back(920 FixItHint::CreateRemoval(cast<UnaryOperator>(E)->getOperatorLoc()));921 return false;922 }923 if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {924 return flipDemorganBinaryOperator(Fixes, Ctx, BinOp, OuterBO);925 }926 if (const auto *Paren = dyn_cast<ParenExpr>(E)) {927 if (const auto *BinOp = dyn_cast<BinaryOperator>(Paren->getSubExpr())) {928 return flipDemorganBinaryOperator(Fixes, Ctx, BinOp, OuterBO, Paren);929 }930 }931 // Fallback case just insert a logical not operator.932 if (E->getBeginLoc().isMacroID())933 return true;934 Fixes.push_back(FixItHint::CreateInsertion(E->getBeginLoc(), "!"));935 return false;936}937 938static bool shouldRemoveParens(const Stmt *Parent,939 BinaryOperatorKind NewOuterBinary,940 const ParenExpr *Parens) {941 if (!Parens)942 return false;943 if (!Parent)944 return true;945 switch (Parent->getStmtClass()) {946 case Stmt::BinaryOperatorClass: {947 const auto *BO = cast<BinaryOperator>(Parent);948 if (BO->isAssignmentOp())949 return true;950 if (BO->isCommaOp())951 return true;952 if (BO->getOpcode() == NewOuterBinary)953 return true;954 return false;955 }956 case Stmt::UnaryOperatorClass:957 case Stmt::CXXRewrittenBinaryOperatorClass:958 return false;959 default:960 return true;961 }962}963 964bool SimplifyBooleanExprCheck::reportDeMorgan(const ASTContext &Context,965 const UnaryOperator *Outer,966 const BinaryOperator *Inner,967 bool TryOfferFix,968 const Stmt *Parent,969 const ParenExpr *Parens) {970 assert(Outer);971 assert(Inner);972 assert(Inner->isLogicalOp());973 974 auto Diag =975 diag(Outer->getBeginLoc(),976 "boolean expression can be simplified by DeMorgan's theorem");977 Diag << Outer->getSourceRange();978 // If we have already fixed this with a previous fix, don't attempt any fixes979 if (!TryOfferFix)980 return false;981 if (Outer->getOperatorLoc().isMacroID())982 return false;983 SmallVector<FixItHint> Fixes;984 auto NewOpcode = getDemorganFlippedOperator(Inner->getOpcode());985 if (shouldRemoveParens(Parent, NewOpcode, Parens)) {986 Fixes.push_back(FixItHint::CreateRemoval(987 SourceRange(Outer->getOperatorLoc(), Parens->getLParen())));988 Fixes.push_back(FixItHint::CreateRemoval(Parens->getRParen()));989 } else {990 Fixes.push_back(FixItHint::CreateRemoval(Outer->getOperatorLoc()));991 }992 if (flipDemorganOperator(Fixes, Inner))993 return false;994 if (flipDemorganSide(Fixes, Context, Inner->getLHS(), NewOpcode) ||995 flipDemorganSide(Fixes, Context, Inner->getRHS(), NewOpcode))996 return false;997 Diag << Fixes;998 return true;999}1000} // namespace clang::tidy::readability1001