10 #include "clang/AST/RecursiveASTVisitor.h"
11 #include "clang/Lex/Lexer.h"
12 #include "llvm/Support/SaveAndRestore.h"
21 namespace readability {
25 StringRef
getText(
const ASTContext &Context, SourceRange
Range) {
27 Context.getSourceManager(),
28 Context.getLangOpts());
31 template <
typename T> StringRef
getText(
const ASTContext &Context, T &Node) {
32 return getText(Context, Node.getSourceRange());
38 "redundant boolean literal supplied to boolean operator";
40 "redundant boolean literal in if statement condition";
42 "redundant boolean literal in conditional return statement";
45 E =
E->IgnoreImpCasts();
46 if (isa<BinaryOperator>(
E) || isa<ConditionalOperator>(
E))
49 if (
const auto *Op = dyn_cast<CXXOperatorCallExpr>(
E))
50 return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call &&
51 Op->getOperator() != OO_Subscript;
56 static std::pair<BinaryOperatorKind, BinaryOperatorKind>
Opposites[] = {
57 {BO_LT, BO_GE}, {BO_GT, BO_LE}, {BO_EQ, BO_NE}};
60 const BinaryOperatorKind Opcode = BinOp->getOpcode();
62 if (Opcode == NegatableOp.first)
63 return BinOp->getOpcodeStr(NegatableOp.second);
64 if (Opcode == NegatableOp.second)
65 return BinOp->getOpcodeStr(NegatableOp.first);
71 {OO_EqualEqual,
"=="}, {OO_ExclaimEqual,
"!="}, {OO_Less,
"<"},
72 {OO_GreaterEqual,
">="}, {OO_Greater,
">"}, {OO_LessEqual,
"<="}};
76 if (
Name.first == OpKind)
83 static std::pair<OverloadedOperatorKind, OverloadedOperatorKind>
85 {OO_Less, OO_GreaterEqual},
86 {OO_Greater, OO_LessEqual}};
89 const OverloadedOperatorKind Opcode = OpCall->getOperator();
91 if (Opcode == NegatableOp.first)
93 if (Opcode == NegatableOp.second)
99 static std::string
asBool(StringRef Text,
bool NeedsStaticCast) {
101 return (
"static_cast<bool>(" +
Text +
")").str();
103 return std::string(
Text);
107 if (
const auto *ImpCast = dyn_cast<ImplicitCastExpr>(
E))
108 return ImpCast->getCastKind() == CK_PointerToBoolean ||
109 ImpCast->getCastKind() == CK_MemberPointerToBoolean;
115 if (
const auto *ImpCast = dyn_cast<ImplicitCastExpr>(
E))
116 return ImpCast->getCastKind() == CK_IntegralToBoolean;
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())
136 E =
E->IgnoreImpCasts();
137 return !
E->getType()->isBooleanType();
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) +
")")
148 return ExprText +
" " + (Negated ?
"!=" :
"==") +
" " + Constant;
152 const Expr *
E,
bool Negated) {
153 const char *NullPtr = Context.getLangOpts().CPlusPlus11 ?
"nullptr" :
"NULL";
158 const Expr *
E,
bool Negated) {
163 bool Negated,
const Expr *
E) {
164 E =
E->IgnoreParenBaseCasts();
165 if (
const auto *EC = dyn_cast<ExprWithCleanups>(
E))
166 E = EC->getSubExpr();
170 if (
const auto *UnOp = dyn_cast<UnaryOperator>(
E)) {
171 if (UnOp->getOpcode() == UO_LNot) {
188 StringRef NegatedOperator;
189 const Expr *LHS =
nullptr;
190 const Expr *RHS =
nullptr;
191 if (
const auto *BinOp = dyn_cast<BinaryOperator>(
E)) {
193 LHS = BinOp->getLHS();
194 RHS = BinOp->getRHS();
195 }
else if (
const auto *OpExpr = dyn_cast<CXXOperatorCallExpr>(
E)) {
196 if (OpExpr->getNumArgs() == 2) {
198 LHS = OpExpr->getArg(0);
199 RHS = OpExpr->getArg(1);
202 if (!NegatedOperator.empty() && LHS && RHS)
203 return (
asBool((
getText(Context, *LHS) +
" " + NegatedOperator +
" " +
210 return (
"!(" +
Text +
")").str();
221 if (
const auto *UnOp = dyn_cast<UnaryOperator>(
E)) {
222 if (UnOp->getOpcode() == UO_LNot) {
241 CharSourceRange CharRange) {
242 std::string ReplacementText =
244 Context.getLangOpts())
246 Lexer Lex(CharRange.getBegin(), Context.getLangOpts(), ReplacementText.data(),
247 ReplacementText.data(),
248 ReplacementText.data() + ReplacementText.size());
249 Lex.SetCommentRetentionState(
true);
252 while (!Lex.LexFromRawLexer(Tok)) {
253 if (Tok.is(tok::TokenKind::comment) || Tok.is(tok::TokenKind::hash))
261 using Base = RecursiveASTVisitor<Visitor>;
265 : Check(Check), Context(Context) {}
270 switch (S->getStmtClass()) {
271 case Stmt::ImplicitCastExprClass:
272 case Stmt::MaterializeTemporaryExprClass:
273 case Stmt::CXXBindTemporaryExprClass:
281 if (S && !shouldIgnore(S))
282 StmtStack.push_back(S);
287 if (S && !shouldIgnore(S)) {
288 assert(StmtStack.back() == S);
289 StmtStack.pop_back();
295 Check->reportBinOp(Context, Op);
301 if (
const auto *Bool = dyn_cast<CXXBoolLiteralExpr>(
E)) {
302 if (FilterMacro && Bool->getBeginLoc().isMacroID())
304 return Bool->getValue();
306 if (
const auto *UnaryOp = dyn_cast<UnaryOperator>(
E)) {
307 if (FilterMacro && UnaryOp->getBeginLoc().isMacroID())
309 if (UnaryOp->getOpcode() == UO_LNot)
310 if (Optional<bool> Res = getAsBoolLiteral(
311 UnaryOp->getSubExpr()->IgnoreImplicit(), FilterMacro))
318 const Node *Item =
nullptr;
321 operator bool()
const {
return Item !=
nullptr; }
329 const auto *RS = dyn_cast<ReturnStmt>(S);
330 if (!RS || !RS->getRetValue())
332 if (Optional<bool> Ret =
333 getAsBoolLiteral(RS->getRetValue()->IgnoreImplicit(),
false)) {
334 return {RS->getRetValue(), *Ret};
342 template <
typename Functor>
344 if (
auto *CS = dyn_cast<CompoundStmt>(S)) {
346 return F(CS->body_front());
353 return StmtStack.size() < 2 ? nullptr : StmtStack[StmtStack.size() - 2];
358 if (If->hasInitStorage() || If->hasVarStorage())
365 Expr *Cond = If->getCond()->IgnoreImplicit();
366 if (Optional<bool> Bool = getAsBoolLiteral(Cond,
true)) {
368 Check->replaceWithThenStatement(Context, If, Cond);
370 Check->replaceWithElseStatement(Context, If, Cond);
379 checkSingleStatement(If->getThen(), parseReturnLiteralBool)) {
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);
396 auto VarBoolAssignmentMatcher = [&Var,
398 const auto *BO = dyn_cast<BinaryOperator>(S);
399 if (!BO || BO->getOpcode() != BO_Assign)
401 Optional<bool> RightasBool =
402 getAsBoolLiteral(BO->getRHS()->IgnoreImplicit(),
false);
405 Expr *IgnImp = BO->getLHS()->IgnoreImplicit();
408 Loc = BO->getRHS()->getBeginLoc();
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};
418 checkSingleStatement(If->getThen(), VarBoolAssignmentMatcher)) {
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);
440 if (Optional<bool> Then =
441 getAsBoolLiteral(Cond->getTrueExpr()->IgnoreImplicit(),
false)) {
442 if (Optional<bool> Else =
443 getAsBoolLiteral(Cond->getFalseExpr()->IgnoreImplicit(),
false)) {
445 Check->replaceWithCondition(Context, Cond, *Else);
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) {
459 CurIf = isa<IfStmt>(*First);
460 ExprAndBool TrailingReturnBool = parseReturnLiteralBool(*Second);
461 if (!TrailingReturnBool)
469 auto *If = cast<IfStmt>(*First);
470 if (!If->hasInitStorage() && !If->hasVarStorage()) {
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);
483 }
else if (isa<LabelStmt, CaseStmt, DefaultStmt>(*First)) {
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()) {
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);
510 return isa<UnaryOperator>(
E) &&
511 cast<UnaryOperator>(
E)->getOpcode() == UO_LNot;
514 template <
typename Functor>
516 return Func(BO->getLHS()) || Func(BO->getRHS());
520 const auto *BO = dyn_cast<BinaryOperator>(
E->IgnoreUnlessSpelledInSource());
523 if (!BO->getType()->isBooleanType())
525 switch (BO->getOpcode()) {
535 if (checkEitherSide(BO, isUnaryLNot))
538 if (checkEitherSide(BO, [NestingLevel](
const Expr *
E) {
539 return nestedDemorgan(
E, NestingLevel - 1);
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);
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 (Check->SimplifyDeMorganRelaxed ||
562 checkEitherSide(BinaryOp, isUnaryLNot) ||
563 checkEitherSide(BinaryOp,
564 [](
const Expr *
E) {
return nestedDemorgan(
E, 1); })) {
565 if (Check->reportDeMorgan(Context, Op, BinaryOp, !IsProcessing, parent(),
567 !Check->areDiagsSelfContained()) {
568 llvm::SaveAndRestore<bool> RAII(IsProcessing,
true);
569 return Base::TraverseUnaryOperator(Op);
572 return Base::TraverseUnaryOperator(Op);
576 bool IsProcessing =
false;
578 SmallVector<Stmt *, 32> StmtStack;
582 SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef
Name,
585 ChainedConditionalReturn(Options.get(
"ChainedConditionalReturn", false)),
586 ChainedConditionalAssignment(
587 Options.get(
"ChainedConditionalAssignment", false)),
588 SimplifyDeMorgan(Options.get(
"SimplifyDeMorgan", true)),
589 SimplifyDeMorganRelaxed(Options.get(
"SimplifyDeMorganRelaxed", false)) {
590 if (SimplifyDeMorganRelaxed && !SimplifyDeMorgan)
592 "without 'SimplifyDeMorgan' enabled")
599 E =
E->IgnoreParenImpCasts();
600 if (isa<CXXBoolLiteralExpr>(
E))
602 if (
const auto *BinOp = dyn_cast<BinaryOperator>(
E))
605 if (
const auto *UnaryOp = dyn_cast<UnaryOperator>(
E))
610 void SimplifyBooleanExprCheck::reportBinOp(
const ASTContext &Context,
611 const BinaryOperator *Op) {
612 const auto *LHS = Op->getLHS()->IgnoreParenImpCasts();
613 const auto *RHS = Op->getRHS()->IgnoreParenImpCasts();
615 const CXXBoolLiteralExpr *Bool;
617 if ((Bool = dyn_cast<CXXBoolLiteralExpr>(LHS)) !=
nullptr)
619 else if ((Bool = dyn_cast<CXXBoolLiteralExpr>(RHS)) !=
nullptr)
624 if (Bool->getBeginLoc().isMacroID())
631 bool BoolValue = Bool->getValue();
633 auto ReplaceWithExpression = [
this, &Context, LHS, RHS,
634 Bool](
const Expr *ReplaceWith,
bool Negated) {
635 std::string Replacement =
637 SourceRange
Range(LHS->getBeginLoc(), RHS->getEndLoc());
642 switch (Op->getOpcode()) {
646 ReplaceWithExpression(Other,
false);
649 ReplaceWithExpression(Bool,
false);
654 ReplaceWithExpression(Bool,
false);
657 ReplaceWithExpression(Other,
false);
661 ReplaceWithExpression(Other, !BoolValue);
665 ReplaceWithExpression(Other, BoolValue);
673 Options.
store(Opts,
"ChainedConditionalReturn", ChainedConditionalReturn);
675 ChainedConditionalAssignment);
676 Options.
store(Opts,
"SimplifyDeMorgan", SimplifyDeMorgan);
677 Options.
store(Opts,
"SimplifyDeMorganRelaxed", SimplifyDeMorganRelaxed);
681 Finder->addMatcher(translationUnitDecl(),
this);
688 void SimplifyBooleanExprCheck::issueDiag(
const ASTContext &Context,
691 SourceRange ReplacementRange,
692 StringRef Replacement) {
693 CharSourceRange CharRange =
694 Lexer::makeFileCharRange(CharSourceRange::getTokenRange(ReplacementRange),
699 Diag << FixItHint::CreateReplacement(CharRange, Replacement);
702 void SimplifyBooleanExprCheck::replaceWithThenStatement(
703 const ASTContext &Context,
const IfStmt *IfStatement,
704 const Expr *BoolLiteral) {
706 IfStatement->getSourceRange(),
707 getText(Context, *IfStatement->getThen()));
710 void SimplifyBooleanExprCheck::replaceWithElseStatement(
711 const ASTContext &Context,
const IfStmt *IfStatement,
712 const Expr *BoolLiteral) {
713 const Stmt *ElseStatement = IfStatement->getElse();
715 IfStatement->getSourceRange(),
716 ElseStatement ?
getText(Context, *ElseStatement) :
"");
719 void SimplifyBooleanExprCheck::replaceWithCondition(
720 const ASTContext &Context,
const ConditionalOperator *Ternary,
722 std::string Replacement =
724 issueDiag(Context, Ternary->getTrueExpr()->getBeginLoc(),
725 "redundant boolean literal in ternary expression result",
726 Ternary->getSourceRange(), Replacement);
729 void SimplifyBooleanExprCheck::replaceWithReturnCondition(
730 const ASTContext &Context,
const IfStmt *If,
const Expr *BoolLiteral,
732 StringRef Terminator = isa<CompoundStmt>(If->getElse()) ?
";" :
"";
735 std::string Replacement = (
"return " +
Condition + Terminator).str();
736 SourceLocation Start = BoolLiteral->getBeginLoc();
738 If->getSourceRange(), Replacement);
741 void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition(
742 const ASTContext &Context,
const ReturnStmt *Ret,
bool Negated,
743 const IfStmt *If,
const Expr *ThenReturn) {
744 const std::string Replacement =
746 issueDiag(Context, ThenReturn->getBeginLoc(),
748 SourceRange(If->getBeginLoc(), Ret->getEndLoc()), Replacement);
751 void SimplifyBooleanExprCheck::replaceWithAssignment(
const ASTContext &Context,
752 const IfStmt *IfAssign,
756 SourceRange
Range = IfAssign->getSourceRange();
757 StringRef VariableName =
getText(Context, *Var);
758 StringRef Terminator = isa<CompoundStmt>(IfAssign->getElse()) ?
";" :
"";
761 std::string Replacement =
762 (VariableName +
" = " +
Condition + Terminator).str();
763 issueDiag(Context,
Loc,
"redundant boolean literal in conditional assignment",
769 const BinaryOperator *BO) {
770 assert(BO->isLogicalOp());
771 if (BO->getOperatorLoc().isMacroID())
773 Output.push_back(FixItHint::CreateReplacement(
774 BO->getOperatorLoc(), BO->getOpcode() == BO_LAnd ?
"||" :
"&&"));
779 assert(BinaryOperator::isLogicalOp(BO));
780 return BO == BO_LAnd ? BO_LOr : BO_LAnd;
784 const ASTContext &
Ctx,
const Expr *
E,
785 Optional<BinaryOperatorKind> OuterBO);
791 const ASTContext &
Ctx,
792 const BinaryOperator *BinOp,
793 Optional<BinaryOperatorKind> OuterBO,
794 const ParenExpr *Parens =
nullptr) {
795 switch (BinOp->getOpcode()) {
809 constexpr
bool LogicalOpParentheses =
true;
810 if (((*OuterBO == NewOp) || (!LogicalOpParentheses &&
811 (*OuterBO == BO_LOr && NewOp == BO_LAnd))) &&
813 if (!Parens->getLParen().isMacroID() &&
814 !Parens->getRParen().isMacroID()) {
815 Fixes.push_back(FixItHint::CreateRemoval(Parens->getLParen()));
816 Fixes.push_back(FixItHint::CreateRemoval(Parens->getRParen()));
819 if (*OuterBO == BO_LAnd && NewOp == BO_LOr && !Parens) {
820 Fixes.push_back(FixItHint::CreateInsertion(BinOp->getBeginLoc(),
"("));
821 Fixes.push_back(FixItHint::CreateInsertion(
822 Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0,
823 Ctx.getSourceManager(),
840 if (BinOp->getOperatorLoc().isMacroID())
842 Fixes.push_back(FixItHint::CreateReplacement(
843 BinOp->getOperatorLoc(),
844 BinaryOperator::getOpcodeStr(
845 BinaryOperator::negateComparisonOp(BinOp->getOpcode()))));
851 if (Parens->getBeginLoc().isMacroID())
853 Fixes.push_back(FixItHint::CreateInsertion(Parens->getBeginLoc(),
"!"));
855 if (BinOp->getBeginLoc().isMacroID() || BinOp->getEndLoc().isMacroID())
857 Fixes.append({FixItHint::CreateInsertion(BinOp->getBeginLoc(),
"!("),
858 FixItHint::CreateInsertion(
859 Lexer::getLocForEndOfToken(BinOp->getEndLoc(), 0,
860 Ctx.getSourceManager(),
870 const ASTContext &
Ctx,
const Expr *
E,
871 Optional<BinaryOperatorKind> OuterBO) {
872 if (isa<UnaryOperator>(
E) && cast<UnaryOperator>(
E)->getOpcode() == UO_LNot) {
874 if (cast<UnaryOperator>(
E)->getOperatorLoc().
isMacroID())
877 FixItHint::CreateRemoval(cast<UnaryOperator>(
E)->getOperatorLoc()));
880 if (
const auto *BinOp = dyn_cast<BinaryOperator>(
E)) {
883 if (
const auto *Paren = dyn_cast<ParenExpr>(
E)) {
884 if (
const auto *BinOp = dyn_cast<BinaryOperator>(Paren->getSubExpr())) {
889 if (
E->getBeginLoc().isMacroID())
891 Fixes.push_back(FixItHint::CreateInsertion(
E->getBeginLoc(),
"!"));
896 BinaryOperatorKind NewOuterBinary,
897 const ParenExpr *Parens) {
902 switch (
Parent->getStmtClass()) {
903 case Stmt::BinaryOperatorClass: {
904 const auto *BO = cast<BinaryOperator>(
Parent);
905 if (BO->isAssignmentOp())
909 if (BO->getOpcode() == NewOuterBinary)
913 case Stmt::UnaryOperatorClass:
914 case Stmt::CXXRewrittenBinaryOperatorClass:
921 bool SimplifyBooleanExprCheck::reportDeMorgan(
const ASTContext &Context,
922 const UnaryOperator *
Outer,
923 const BinaryOperator *
Inner,
926 const ParenExpr *Parens) {
929 assert(
Inner->isLogicalOp());
933 "boolean expression can be simplified by DeMorgan's theorem");
934 Diag <<
Outer->getSourceRange();
938 if (
Outer->getOperatorLoc().isMacroID())
940 SmallVector<FixItHint> Fixes;
943 Fixes.push_back(FixItHint::CreateRemoval(
944 SourceRange(
Outer->getOperatorLoc(), Parens->getLParen())));
945 Fixes.push_back(FixItHint::CreateRemoval(Parens->getRParen()));
947 Fixes.push_back(FixItHint::CreateRemoval(
Outer->getOperatorLoc()));