Index: clang-tidy/readability/SimplifyBooleanExprCheck.h =================================================================== --- clang-tidy/readability/SimplifyBooleanExprCheck.h +++ clang-tidy/readability/SimplifyBooleanExprCheck.h @@ -34,9 +34,46 @@ /// `if (e) return false; else return true;` becomes `return !e;` /// `if (e) b = true; else b = false;` becomes `b = e;` /// `if (e) b = false; else b = true;` becomes `b = !e;` +/// `if (e) return true; return false;` becomes `return e;` +/// `if (e) return false; return true;` becomes `return !e;` /// -/// Parenthesis from the resulting expression `e` are removed whenever -/// possible. +/// The resulting expression `e` is modified as follows: +/// 1. Unnecessary parentheses around the expression are removed. +/// 2. Negated applications of `!` are eliminated. +/// 3. Negated applications of comparison operators are changed to use the +/// opposite condition. +/// 4. Implicit conversions of pointer to `bool` are replaced with explicit +/// comparisons to `nullptr`. +/// 5. Implicit casts to `bool` are replaced with explicit casts to `bool`. +/// 6. Object expressions with `explicit operator bool` conversion operators +/// are replaced with explicit casts to `bool`. +/// +/// Examples: +/// 1. The ternary assignment `bool b = (i < 0) ? true : false;` has redundant +/// parentheses and becomes `bool b = i < 0;`. +/// +/// 2. The conditional return `if (!b) return false; return true;` has an +/// implied double negation and becomes `return b;`. +/// +/// 3. The conditional return `if (i < 0) return false; return true;` becomes +/// `return i >= 0;`. +/// The conditional return `if (i != 0) return false; return true;` becomes +/// `return i == 0;`. +/// +/// 4. The conditional return `if (p) return true; return false;` has an +/// implicit conversion of a pointer to `bool` and becomes +/// `return p != nullptr;`. +/// The ternary assignment `bool b = (i & 1) ? true : false;` has an implicit +/// conversion of `i & 1` to `bool` and becomes +/// `bool b = static_cast(i & 1);`. +/// +/// 5. The conditional return `if (i & 1) return true; else return false;` has +/// an implicit conversion of an integer quantity `i & 1` to `bool` and becomes +/// `return static_cast(i & 1);` +/// +/// 6. Given `struct X { explicit operator bool(); };`, and an instance `x` of +/// `struct X`, the conditional return `if (x) return true; return false;` +/// becomes `return static_cast(x);` /// /// When a conditional boolean return or assignment appears at the end of a /// chain of `if`, `else if` statements, the conditional statement is left @@ -77,6 +114,9 @@ void matchIfAssignsBool(ast_matchers::MatchFinder *Finder, bool Value, StringRef Id); + void matchCompoundIfReturnsBool(ast_matchers::MatchFinder *Finder, bool Value, + StringRef Id); + void replaceWithExpression(const ast_matchers::MatchFinder::MatchResult &Result, const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS, @@ -103,6 +143,10 @@ replaceWithAssignment(const ast_matchers::MatchFinder::MatchResult &Result, const IfStmt *If, bool Negated = false); + void replaceCompoundReturnWithCondition( + const ast_matchers::MatchFinder::MatchResult &Result, + const CompoundStmt *Compound, bool Negated = false); + const bool ChainedConditionalReturn; const bool ChainedConditionalAssignment; }; Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp =================================================================== --- clang-tidy/readability/SimplifyBooleanExprCheck.cpp +++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp @@ -12,6 +12,7 @@ #include #include +#include using namespace clang::ast_matchers; @@ -48,6 +49,11 @@ const char IfAssignBoolId[] = "if-assign"; const char IfAssignNotBoolId[] = "if-assign-not"; const char IfAssignObjId[] = "if-assign-obj"; +const char CompoundId[] = "compound"; +const char CompoundIfReturnsBoolId[] = "compound-if"; +const char CompoundReturnId[] = "compound-return"; +const char CompoundBoolId[] = "compound-bool"; +const char CompoundNotBoolId[] = "compound-bool-not"; const char IfStmtId[] = "if"; const char LHSId[] = "lhs-expr"; @@ -57,6 +63,8 @@ "redundant boolean literal supplied to boolean operator"; const char SimplifyConditionDiagnostic[] = "redundant boolean literal in if statement condition"; +const char SimplifyConditionalReturnDiagnostic[] = + "redundant boolean literal in conditional return statement"; const CXXBoolLiteralExpr *getBoolLiteral(const MatchFinder::MatchResult &Result, StringRef Id) { @@ -67,14 +75,15 @@ : Literal; } -internal::Matcher ReturnsBool(bool Value, StringRef Id = "") { - auto SimpleReturnsBool = returnStmt( - has(boolLiteral(equals(Value)).bind(Id.empty() ? "ignored" : Id))); +internal::Matcher returnsBool(bool Value, StringRef Id = "ignored") { + auto SimpleReturnsBool = + returnStmt(has(boolLiteral(equals(Value)).bind(Id))).bind("returns-bool"); return anyOf(SimpleReturnsBool, compoundStmt(statementCountIs(1), has(SimpleReturnsBool))); } bool needsParensAfterUnaryNegation(const Expr *E) { + E = E->IgnoreImpCasts(); if (isa(E) || isa(E)) return true; if (const auto *Op = dyn_cast(E)) @@ -84,8 +93,7 @@ } std::pair Opposites[] = { - std::make_pair(BO_LT, BO_GE), std::make_pair(BO_GT, BO_LE), - std::make_pair(BO_EQ, BO_NE)}; + {BO_LT, BO_GE}, {BO_GT, BO_LE}, {BO_EQ, BO_NE}}; StringRef negatedOperator(const BinaryOperator *BinOp) { const BinaryOperatorKind Opcode = BinOp->getOpcode(); @@ -98,24 +106,145 @@ return StringRef(); } +std::pair OperatorNames[] = { + {OO_EqualEqual, "=="}, + {OO_ExclaimEqual, "!="}, + {OO_Less, "<"}, + {OO_GreaterEqual, ">="}, + {OO_Greater, ">"}, + {OO_LessEqual, "<="}}; + +StringRef getOperatorName(OverloadedOperatorKind OpKind) { + for (auto Name : OperatorNames) + if (Name.first == OpKind) + return Name.second; + + return StringRef(); +} + +std::pair OppositeOverloads[] = + {{OO_EqualEqual, OO_ExclaimEqual}, + {OO_Less, OO_GreaterEqual}, + {OO_Greater, OO_LessEqual}}; + +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 StringRef(); +} + +std::string asBool(StringRef text, bool NeedsStaticCast) { + if (NeedsStaticCast) + return ("static_cast(" + text + ")").str(); + + return text; +} + +bool needsNullPtrComparison(const Expr *E) { + if (const auto *ImpCast = dyn_cast(E)) + return ImpCast->getCastKind() == CK_PointerToBoolean; + + return false; +} + +bool needsStaticCast(const Expr *E) { + if (const auto *ImpCast = dyn_cast(E)) + if (ImpCast->getCastKind() == CK_UserDefinedConversion && + ImpCast->getSubExpr()->getType()->isBooleanType()) + if (const auto *MemCall = + dyn_cast(ImpCast->getSubExpr())) + if (const auto *MemDecl = + dyn_cast(MemCall->getMethodDecl())) + if (MemDecl->isExplicit()) + return true; + + E = E->IgnoreImpCasts(); + return !E->getType()->isBooleanType(); +} + std::string replacementExpression(const MatchFinder::MatchResult &Result, bool Negated, const Expr *E) { - while (const auto *Parenthesized = dyn_cast(E)) { - E = Parenthesized->getSubExpr(); - } + E = E->ignoreParenBaseCasts(); + const bool NeedsStaticCast = needsStaticCast(E); if (Negated) { + if (const auto *UnOp = dyn_cast(E)) + if (UnOp->getOpcode() == UO_LNot) { + if (needsNullPtrComparison(UnOp->getSubExpr())) + return (getText(Result, *UnOp->getSubExpr()) + " != nullptr").str(); + + return replacementExpression(Result, false, UnOp->getSubExpr()); + } + + if (needsNullPtrComparison(E)) + return (getText(Result, *E) + " == nullptr").str(); + + StringRef NegatedOperator; + const Expr *LHS = nullptr; + const Expr *RHS = nullptr; if (const auto *BinOp = dyn_cast(E)) { - StringRef NegatedOperator = negatedOperator(BinOp); - if (!NegatedOperator.empty()) { - return (getText(Result, *BinOp->getLHS()) + " " + NegatedOperator + - " " + getText(Result, *BinOp->getRHS())).str(); + NegatedOperator = negatedOperator(BinOp); + LHS = BinOp->getLHS(); + RHS = BinOp->getRHS(); + } else if (const auto *OpExpr = dyn_cast(E)) { + if (OpExpr->getNumArgs() == 2) { + NegatedOperator = negatedOperator(OpExpr); + LHS = OpExpr->getArg(0); + RHS = OpExpr->getArg(1); } } + if (!NegatedOperator.empty() && LHS && RHS) + return (asBool((getText(Result, *LHS) + " " + NegatedOperator + " " + + getText(Result, *RHS)).str(), + NeedsStaticCast)); + + StringRef Text = getText(Result, *E); + if (!NeedsStaticCast && needsParensAfterUnaryNegation(E)) + return ("!(" + Text + ")").str(); + + if (needsNullPtrComparison(E)) + return (getText(Result, *E) + " == nullptr").str(); + + return ("!" + asBool(Text, NeedsStaticCast)); } - StringRef Text = getText(Result, *E); - return (Negated ? (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")" - : "!" + Text) - : Text).str(); + + if (const auto *UnOp = dyn_cast(E)) + if (UnOp->getOpcode() == UO_LNot) + if (needsNullPtrComparison(UnOp->getSubExpr())) + return (getText(Result, *UnOp->getSubExpr()) + " == nullptr").str(); + + if (needsNullPtrComparison(E)) + return (getText(Result, *E) + " != nullptr").str(); + + return asBool(getText(Result, *E), NeedsStaticCast); +} + +const CXXBoolLiteralExpr *stmtReturnsBool(const ReturnStmt *Ret, bool Negated) { + if (const auto *Bool = dyn_cast(Ret->getRetValue())) + if (Bool->getValue() == !Negated) + return Bool; + + return nullptr; +} + +const CXXBoolLiteralExpr *stmtReturnsBool(const IfStmt *IfRet, bool Negated) { + if (IfRet->getElse() != nullptr) + return nullptr; + + if (const auto *Ret = dyn_cast(IfRet->getThen())) + return stmtReturnsBool(Ret, Negated); + + if (const auto *Compound = dyn_cast(IfRet->getThen())) + if (Compound->size() == 1) + if (const auto *CompoundRet = + dyn_cast(Compound->body_back())) + return stmtReturnsBool(CompoundRet, Negated); + + return nullptr; } } // namespace @@ -204,18 +333,17 @@ void SimplifyBooleanExprCheck::matchIfReturnsBool(MatchFinder *Finder, bool Value, StringRef Id) { - if (ChainedConditionalReturn) { + if (ChainedConditionalReturn) Finder->addMatcher(ifStmt(isExpansionInMainFile(), - hasThen(ReturnsBool(Value, ThenLiteralId)), - hasElse(ReturnsBool(!Value))).bind(Id), + hasThen(returnsBool(Value, ThenLiteralId)), + hasElse(returnsBool(!Value))).bind(Id), this); - } else { + else Finder->addMatcher(ifStmt(isExpansionInMainFile(), unless(hasParent(ifStmt())), - hasThen(ReturnsBool(Value, ThenLiteralId)), - hasElse(ReturnsBool(!Value))).bind(Id), + hasThen(returnsBool(Value, ThenLiteralId)), + hasElse(returnsBool(!Value))).bind(Id), this); - } } void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder, @@ -233,16 +361,27 @@ hasRHS(boolLiteral(equals(!Value)))); auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1), hasAnySubstatement(SimpleElse))); - if (ChainedConditionalAssignment) { + if (ChainedConditionalAssignment) Finder->addMatcher( ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id), this); - } else { + else Finder->addMatcher(ifStmt(isExpansionInMainFile(), unless(hasParent(ifStmt())), hasThen(Then), hasElse(Else)).bind(Id), this); - } +} + +void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder, + bool Value, + StringRef Id) { + Finder->addMatcher( + compoundStmt( + allOf(hasAnySubstatement(ifStmt(hasThen(returnsBool(Value)), + unless(hasElse(stmt())))), + hasAnySubstatement(returnStmt(has(boolLiteral(equals(!Value)))) + .bind(CompoundReturnId)))).bind(Id), + this); } void SimplifyBooleanExprCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { @@ -283,46 +422,54 @@ matchIfAssignsBool(Finder, true, IfAssignBoolId); matchIfAssignsBool(Finder, false, IfAssignNotBoolId); + + matchCompoundIfReturnsBool(Finder, true, CompoundBoolId); + matchCompoundIfReturnsBool(Finder, false, CompoundNotBoolId); } void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) { if (const CXXBoolLiteralExpr *LeftRemoved = - getBoolLiteral(Result, RightExpressionId)) { + getBoolLiteral(Result, RightExpressionId)) replaceWithExpression(Result, LeftRemoved, false); - } else if (const CXXBoolLiteralExpr *RightRemoved = - getBoolLiteral(Result, LeftExpressionId)) { + else if (const CXXBoolLiteralExpr *RightRemoved = + getBoolLiteral(Result, LeftExpressionId)) replaceWithExpression(Result, RightRemoved, true); - } else if (const CXXBoolLiteralExpr *NegatedLeftRemoved = - getBoolLiteral(Result, NegatedRightExpressionId)) { + else if (const CXXBoolLiteralExpr *NegatedLeftRemoved = + getBoolLiteral(Result, NegatedRightExpressionId)) replaceWithExpression(Result, NegatedLeftRemoved, false, true); - } else if (const CXXBoolLiteralExpr *NegatedRightRemoved = - getBoolLiteral(Result, NegatedLeftExpressionId)) { + else if (const CXXBoolLiteralExpr *NegatedRightRemoved = + getBoolLiteral(Result, NegatedLeftExpressionId)) replaceWithExpression(Result, NegatedRightRemoved, true, true); - } else if (const CXXBoolLiteralExpr *TrueConditionRemoved = - getBoolLiteral(Result, ConditionThenStmtId)) { + else if (const CXXBoolLiteralExpr *TrueConditionRemoved = + getBoolLiteral(Result, ConditionThenStmtId)) replaceWithThenStatement(Result, TrueConditionRemoved); - } else if (const CXXBoolLiteralExpr *FalseConditionRemoved = - getBoolLiteral(Result, ConditionElseStmtId)) { + else if (const CXXBoolLiteralExpr *FalseConditionRemoved = + getBoolLiteral(Result, ConditionElseStmtId)) replaceWithElseStatement(Result, FalseConditionRemoved); - } else if (const auto *Ternary = - Result.Nodes.getNodeAs(TernaryId)) { + else if (const auto *Ternary = + Result.Nodes.getNodeAs(TernaryId)) replaceWithCondition(Result, Ternary); - } else if (const auto *TernaryNegated = + else if (const auto *TernaryNegated = Result.Nodes.getNodeAs( - TernaryNegatedId)) { + TernaryNegatedId)) replaceWithCondition(Result, TernaryNegated, true); - } else if (const auto *If = Result.Nodes.getNodeAs(IfReturnsBoolId)) { + else if (const auto *If = Result.Nodes.getNodeAs(IfReturnsBoolId)) replaceWithReturnCondition(Result, If); - } else if (const auto *IfNot = - Result.Nodes.getNodeAs(IfReturnsNotBoolId)) { + else if (const auto *IfNot = + Result.Nodes.getNodeAs(IfReturnsNotBoolId)) replaceWithReturnCondition(Result, IfNot, true); - } else if (const auto *IfAssign = - Result.Nodes.getNodeAs(IfAssignBoolId)) { + else if (const auto *IfAssign = + Result.Nodes.getNodeAs(IfAssignBoolId)) replaceWithAssignment(Result, IfAssign); - } else if (const auto *IfAssignNot = - Result.Nodes.getNodeAs(IfAssignNotBoolId)) { + else if (const auto *IfAssignNot = + Result.Nodes.getNodeAs(IfAssignNotBoolId)) replaceWithAssignment(Result, IfAssignNot, true); - } + else if (const auto *Compound = + Result.Nodes.getNodeAs(CompoundBoolId)) + replaceCompoundReturnWithCondition(Result, Compound); + else if (const auto *Compound = + Result.Nodes.getNodeAs(CompoundNotBoolId)) + replaceCompoundReturnWithCondition(Result, Compound, true); } void SimplifyBooleanExprCheck::replaceWithExpression( @@ -375,10 +522,50 @@ std::string Replacement = ("return " + Condition + Terminator).str(); SourceLocation Start = Result.Nodes.getNodeAs(ThenLiteralId)->getLocStart(); - diag(Start, "redundant boolean literal in conditional return statement") + diag(Start, SimplifyConditionalReturnDiagnostic) << FixItHint::CreateReplacement(If->getSourceRange(), Replacement); } +void SimplifyBooleanExprCheck::replaceCompoundReturnWithCondition( + const MatchFinder::MatchResult &Result, const CompoundStmt *Compound, + bool Negated) { + const auto *Ret = Result.Nodes.getNodeAs(CompoundReturnId); + + // The body shouldn't be empty because the matcher ensures that it must + // contain at least two statements: + // 1) A `return` statement returning a boolean literal `false` or `true` + // 2) An `if` statement with no `else` clause that consists fo a single + // `return` statement returning the opposite boolean literal `true` or + // `false`. + assert(Compound->size() >= 2); + const IfStmt *BeforeIf = nullptr; + CompoundStmt::const_body_iterator Current = Compound->body_begin(); + CompoundStmt::const_body_iterator After = Compound->body_begin(); + for (++After; After != Compound->body_end() && *Current != Ret; + ++Current, ++After) + if (const auto *If = dyn_cast(*Current)) { + if (const CXXBoolLiteralExpr *Lit = stmtReturnsBool(If, Negated)) { + if (*After == Ret) { + if (!ChainedConditionalReturn && BeforeIf) + continue; + + const Expr *Condition = If->getCond(); + std::string Replacement = + "return " + replacementExpression(Result, Negated, Condition); + diag(Lit->getLocStart(), SimplifyConditionalReturnDiagnostic) + << FixItHint::CreateReplacement( + SourceRange(If->getLocStart(), Ret->getLocEnd()), + Replacement); + return; + } + + BeforeIf = If; + } + } else { + BeforeIf = nullptr; + } +} + void SimplifyBooleanExprCheck::replaceWithAssignment( const MatchFinder::MatchResult &Result, const IfStmt *IfAssign, bool Negated) { Index: test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp =================================================================== --- test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp +++ test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp @@ -31,3 +31,65 @@ // CHECK-FIXES-NEXT: {{^}} return false; // CHECK-FIXES-NEXT: {{^}} else return i > 20; } + +bool chained_simple_if_return(int i) { + if (i < 5) + return true; + if (i > 10) + return true; + return false; +} +// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}}bool chained_simple_if_return(int i) {{{$}} +// CHECK-FIXES: {{^}} if (i < 5){{$}} +// CHECK-FIXES: {{^ return true;$}} +// CHECK-FIXES: {{^ return i > 10;$}} +// CHECK-FIXES: {{^}$}} + +bool chained_simple_if_return_negated(int i) { + if (i < 5) + return false; + if (i > 10) + return false; + return true; +} +// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}}bool chained_simple_if_return_negated(int i) {{{$}} +// CHECK-FIXES: {{^}} if (i < 5){{$}} +// CHECK-FIXES: {{^ return false;$}} +// CHECK-FIXES: {{^ return i <= 10;$}} +// CHECK-FIXES: {{^}$}} + +bool complex_chained_if_return_return(int i) { + if (i < 5) { + return true; + } + if (i > 10) { + return true; + } + return false; +} +// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}}bool complex_chained_if_return_return(int i) {{{$}} +// CHECK-FIXES: {{^}} if (i < 5) {{{$}} +// CHECK-FIXES: {{^}} return true;{{$}} +// CHECK-FIXES: {{^}} }{{$}} +// CHECK-FIXES: {{^ return i > 10;$}} +// CHECK-FIXES: {{^}$}} + +bool complex_chained_if_return_return_negated(int i) { + if (i < 5) { + return false; + } + if (i > 10) { + return false; + } + return true; +} +// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}}bool complex_chained_if_return_return_negated(int i) {{{$}} +// CHECK-FIXES: {{^}} if (i < 5) {{{$}} +// CHECK-FIXES: {{^}} return false;{{$}} +// CHECK-FIXES: {{^}} }{{$}} +// CHECK-FIXES: {{^ return i <= 10;$}} +// CHECK-FIXES: {{^}$}} Index: test/clang-tidy/readability-simplify-bool-expr.cpp =================================================================== --- test/clang-tidy/readability-simplify-bool-expr.cpp +++ test/clang-tidy/readability-simplify-bool-expr.cpp @@ -321,6 +321,13 @@ // CHECK-FIXES: {{^}} return i != 0;{{$}} // CHECK-FIXES-NEXT: {{^}$}} +bool negative_condition_conditional_return_statement(int i) { + if (!(i == 0)) return false; else return true; +} +// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: {{.*}} in conditional return statement +// CHECK-FIXES: {{^}} return i == 0;{{$}} +// CHECK-FIXES-NEXT: {{^}$}} + bool conditional_compound_return_statements(int i) { if (i == 1) { return true; @@ -593,3 +600,275 @@ else b = false; } + +// unchanged: chained return statements, but ChainedConditionalReturn not set +bool chained_simple_if_return_negated(int i) { + if (i < 5) + return false; + if (i > 10) + return false; + return true; +} + +// unchanged: chained return statements, but ChainedConditionalReturn not set +bool complex_chained_if_return_return(int i) { + if (i < 5) { + return true; + } + if (i > 10) { + return true; + } + return false; +} + +// unchanged: chained return statements, but ChainedConditionalReturn not set +bool complex_chained_if_return_return_negated(int i) { + if (i < 5) { + return false; + } + if (i > 10) { + return false; + } + return true; +} + +// unchanged: chained return statements, but ChainedConditionalReturn not set +bool chained_simple_if_return(int i) { + if (i < 5) + return true; + if (i > 10) + return true; + return false; +} + +bool simple_if_return_return(int i) { + if (i > 10) + return true; + return false; +} +// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}}bool simple_if_return_return(int i) {{{$}} +// CHECK-FIXES: {{^ return i > 10;$}} +// CHECK-FIXES: {{^}$}} + +bool simple_if_return_return_negated(int i) { + if (i > 10) + return false; + return true; +} +// CHECK-MESSAGES: :[[@LINE-3]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}}bool simple_if_return_return_negated(int i) {{{$}} +// CHECK-FIXES: {{^ return i <= 10;$}} +// CHECK-FIXES: {{^}$}} + +bool complex_if_return_return(int i) { + if (i > 10) { + return true; + } + return false; +} +// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}}bool complex_if_return_return(int i) {{{$}} +// CHECK-FIXES: {{^ return i > 10;$}} +// CHECK-FIXES: {{^}$}} + +bool complex_if_return_return_negated(int i) { + if (i > 10) { + return false; + } + return true; +} +// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}}bool complex_if_return_return_negated(int i) {{{$}} +// CHECK-FIXES: {{^ return i <= 10;$}} +// CHECK-FIXES: {{^}$}} + +bool if_implicit_bool_expr(int i) { + if (i & 1) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return static_cast(i & 1);{{$}} + +bool negated_if_implicit_bool_expr(int i) { + if (i - 1) { + return false; + } else { + return true; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return !static_cast(i - 1);{{$}} + +bool implicit_int(int i) { + if (i) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return static_cast(i);{{$}} + +bool explicit_bool(bool b) { + if (b) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return b;{{$}} + +class Implicit { +public: + operator bool() { + return true; + } +}; + +bool object_bool_implicit_conversion(Implicit O) { + if (O) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return O;{{$}} + +bool negated_explicit_bool(bool b) { + if (!b) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return !b;{{$}} + +bool bitwise_complement_conversion(int i) { + if (~i) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return static_cast(~i);{{$}} + +bool logical_or(bool a, bool b) { + if (a || b) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return a || b;{{$}} + +bool logical_and(bool a, bool b) { + if (a && b) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return a && b;{{$}} + +class Comparable +{ +public: + bool operator==(Comparable const &rhs) { return true; } + bool operator!=(Comparable const &rhs) { return false; } +}; + +bool comparable_objects() { + Comparable c; + Comparable d; + if (c == d) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return c == d;{{$}} + +bool negated_comparable_objects() { + Comparable c; + Comparable d; + if (c == d) { + return false; + } else { + return true; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: {{^}} return c != d;{{$}} + +struct X { + explicit operator bool(); +}; + +void explicit_conversion_assignment(X x) { + bool y; + if (x) { + y = true; + } else { + y = false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: {{.*}} in conditional assignment +// CHECK-FIXES: {{^ bool y;$}} +// CHECK-FIXES: {{^}} y = static_cast(x);{{$}} + +void ternary_integer_condition(int i) { + bool b = i ? true : false; +} +// CHECK-MESSAGES: :[[@LINE-2]]:16: warning: {{.*}} in ternary expression result +// CHECK-FIXES: bool b = static_cast(i);{{$}} + +bool non_null_pointer_condition(int *p1) { + if (p1) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: return p1 != nullptr;{{$}} + +bool null_pointer_condition(int *p2) { + if (!p2) { + return true; + } else { + return false; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: return p2 == nullptr;{{$}} + +bool negated_non_null_pointer_condition(int *p3) { + if (p3) { + return false; + } else { + return true; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: return p3 == nullptr;{{$}} + +bool negated_null_pointer_condition(int *p4) { + if (!p4) { + return false; + } else { + return true; + } +} +// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return +// CHECK-FIXES: return p4 != nullptr;{{$}}