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