Author: alexfh Date: Wed Jul 1 07:39:40 2015 New Revision: 241155 URL: http://llvm.org/viewvc/llvm-project?rev=241155&view=rev Log: [clang-tidy] Enhance clang-tidy readability-simplify-boolean-expr...
Enhance clang-tidy readability-simplify-boolean-expr to handle 'if (e) return true; return false;' and improve replacement expressions. This changeset extends the simplify boolean expression check in clang-tidy to simplify if (e) return true; return false; to return e; (note the lack of an else clause on the if statement.) By default, chained conditional assignment is left unchanged, unless a configuration parameter is set to non-zero to override this behavior. It also improves the handling of replacement expressions to apply static_cast<bool>(expr) when expr is not of type bool. http://reviews.llvm.org/D9810 Patch by Richard Thomson! Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp?rev=241155&r1=241154&r2=241155&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.cpp Wed Jul 1 07:39:40 2015 @@ -12,6 +12,7 @@ #include <cassert> #include <string> +#include <utility> using namespace clang::ast_matchers; @@ -48,6 +49,9 @@ const char IfAssignLocId[] = "if-assign- const char IfAssignBoolId[] = "if-assign"; const char IfAssignNotBoolId[] = "if-assign-not"; const char IfAssignObjId[] = "if-assign-obj"; +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 +61,8 @@ const char SimplifyOperatorDiagnostic[] "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,25 +73,26 @@ const CXXBoolLiteralExpr *getBoolLiteral : Literal; } -internal::Matcher<Stmt> ReturnsBool(bool Value, StringRef Id = "") { - auto SimpleReturnsBool = returnStmt( - has(boolLiteral(equals(Value)).bind(Id.empty() ? "ignored" : Id))); +internal::Matcher<Stmt> 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<BinaryOperator>(E) || isa<ConditionalOperator>(E)) return true; - if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E)) + if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E)) { return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call && Op->getOperator() != OO_Subscript; + } return false; } std::pair<BinaryOperatorKind, BinaryOperatorKind> 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 +105,153 @@ StringRef negatedOperator(const BinaryOp return StringRef(); } +std::pair<OverloadedOperatorKind, StringRef> 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<OverloadedOperatorKind, OverloadedOperatorKind> 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<bool>(" + text + ")").str(); + + return text; +} + +bool needsNullPtrComparison(const Expr *E) { + if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) + return ImpCast->getCastKind() == CK_PointerToBoolean; + + return false; +} + +bool needsStaticCast(const Expr *E) { + if (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) { + if (ImpCast->getCastKind() == CK_UserDefinedConversion && + ImpCast->getSubExpr()->getType()->isBooleanType()) { + if (const auto *MemCall = + dyn_cast<CXXMemberCallExpr>(ImpCast->getSubExpr())) { + if (const auto *MemDecl = + dyn_cast<CXXConversionDecl>(MemCall->getMethodDecl())) { + if (MemDecl->isExplicit()) + return true; + } + } + } + } + + E = E->IgnoreImpCasts(); + return !E->getType()->isBooleanType(); +} + std::string replacementExpression(const MatchFinder::MatchResult &Result, bool Negated, const Expr *E) { - while (const auto *Parenthesized = dyn_cast<ParenExpr>(E)) { - E = Parenthesized->getSubExpr(); - } + E = E->ignoreParenBaseCasts(); + const bool NeedsStaticCast = needsStaticCast(E); if (Negated) { + if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) { + if (UnOp->getOpcode() == UO_LNot) { + if (needsNullPtrComparison(UnOp->getSubExpr())) + return (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<BinaryOperator>(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<CXXOperatorCallExpr>(E)) { + if (OpExpr->getNumArgs() == 2) { + NegatedOperator = negatedOperator(OpExpr); + LHS = OpExpr->getArg(0); + RHS = OpExpr->getArg(1); } } + if (!NegatedOperator.empty() && LHS && RHS) { + return (asBool((getText(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)); + } + + if (const auto *UnOp = dyn_cast<UnaryOperator>(E)) { + if (UnOp->getOpcode() == UO_LNot) { + if (needsNullPtrComparison(UnOp->getSubExpr())) + return (getText(Result, *UnOp->getSubExpr()) + " == nullptr").str(); + } } - StringRef Text = getText(Result, *E); - return (Negated ? (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")" - : "!" + Text) - : Text).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<CXXBoolLiteralExpr>(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<ReturnStmt>(IfRet->getThen())) + return stmtReturnsBool(Ret, Negated); + + if (const auto *Compound = dyn_cast<CompoundStmt>(IfRet->getThen())) { + if (Compound->size() == 1) { + if (const auto *CompoundRet = dyn_cast<ReturnStmt>(Compound->body_back())) + return stmtReturnsBool(CompoundRet, Negated); + } + } + + return nullptr; } } // namespace @@ -185,10 +321,11 @@ void SimplifyBooleanExprCheck::matchExpr void SimplifyBooleanExprCheck::matchBoolCondition(MatchFinder *Finder, bool Value, StringRef BooleanId) { - Finder->addMatcher(ifStmt(isExpansionInMainFile(), - hasCondition(boolLiteral(equals(Value)) - .bind(BooleanId))).bind(IfStmtId), - this); + Finder->addMatcher( + ifStmt(isExpansionInMainFile(), + hasCondition(boolLiteral(equals(Value)).bind(BooleanId))) + .bind(IfStmtId), + this); } void SimplifyBooleanExprCheck::matchTernaryResult(MatchFinder *Finder, @@ -206,14 +343,16 @@ void SimplifyBooleanExprCheck::matchIfRe bool Value, StringRef Id) { 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 { 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); } } @@ -240,11 +379,25 @@ void SimplifyBooleanExprCheck::matchIfAs } else { Finder->addMatcher(ifStmt(isExpansionInMainFile(), unless(hasParent(ifStmt())), hasThen(Then), - hasElse(Else)).bind(Id), + 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) { Options.store(Opts, "ChainedConditionalReturn", ChainedConditionalReturn); Options.store(Opts, "ChainedConditionalAssignment", @@ -283,6 +436,9 @@ void SimplifyBooleanExprCheck::registerM matchIfAssignsBool(Finder, true, IfAssignBoolId); matchIfAssignsBool(Finder, false, IfAssignNotBoolId); + + matchCompoundIfReturnsBool(Finder, true, CompoundBoolId); + matchCompoundIfReturnsBool(Finder, false, CompoundNotBoolId); } void SimplifyBooleanExprCheck::check(const MatchFinder::MatchResult &Result) { @@ -322,6 +478,12 @@ void SimplifyBooleanExprCheck::check(con } else if (const auto *IfAssignNot = Result.Nodes.getNodeAs<IfStmt>(IfAssignNotBoolId)) { replaceWithAssignment(Result, IfAssignNot, true); + } else if (const auto *Compound = + Result.Nodes.getNodeAs<CompoundStmt>(CompoundBoolId)) { + replaceCompoundReturnWithCondition(Result, Compound); + } else if (const auto *Compound = + Result.Nodes.getNodeAs<CompoundStmt>(CompoundNotBoolId)) { + replaceCompoundReturnWithCondition(Result, Compound, true); } } @@ -375,10 +537,51 @@ void SimplifyBooleanExprCheck::replaceWi std::string Replacement = ("return " + Condition + Terminator).str(); SourceLocation Start = Result.Nodes.getNodeAs<CXXBoolLiteralExpr>(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<ReturnStmt>(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<IfStmt>(*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) { Modified: clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h?rev=241155&r1=241154&r2=241155&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/readability/SimplifyBooleanExprCheck.h Wed Jul 1 07:39:40 2015 @@ -34,9 +34,46 @@ namespace readability { /// `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<bool>(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<bool>(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<bool>(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 @@ private: 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 @@ private: 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; }; Modified: clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp?rev=241155&r1=241154&r2=241155&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr-chained-conditional-return.cpp Wed Jul 1 07:39:40 2015 @@ -31,3 +31,65 @@ bool chained_conditional_return(int i) { // 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: {{^}$}} Modified: clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp?rev=241155&r1=241154&r2=241155&view=diff ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/readability-simplify-bool-expr.cpp Wed Jul 1 07:39:40 2015 @@ -321,6 +321,13 @@ bool negated_conditional_return_statemen // 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; @@ -447,26 +454,26 @@ void simple_conditional_assignment_state // CHECK-FIXES: {{^ }}c = i <= 20;{{$}} // CHECK-FIXES: bool c2 = false; - // unchanged; different variables + // Unchanged: different variables. bool b2; if (i > 12) b = true; else b2 = false; - // unchanged; no else statement + // Unchanged: no else statement. bool b3; if (i > 15) b3 = true; - // unchanged; not boolean assignment + // Unchanged: not boolean assignment. int j; if (i > 17) j = 10; else j = 20; - // unchanged; different variables assigned + // Unchanged: different variables assigned. int k = 0; bool b4 = false; if (i > 10) @@ -500,13 +507,13 @@ void complex_conditional_assignment_stat // CHECK-FIXES: {{^ }}e = i <= 40;{{$}} // CHECK-FIXES: e = false; - // unchanged; no else statement + // Unchanged: no else statement. bool b3; if (i > 15) { b3 = true; } - // unchanged; not a boolean assignment + // Unchanged: not a boolean assignment. int j; if (i > 17) { j = 10; @@ -514,7 +521,7 @@ void complex_conditional_assignment_stat j = 20; } - // unchanged; multiple statements + // Unchanged: multiple statements. bool f; if (j > 10) { j = 10; @@ -524,7 +531,7 @@ void complex_conditional_assignment_stat f = false; } - // unchanged; multiple statements + // Unchanged: multiple statements. bool g; if (j > 10) f = true; @@ -533,7 +540,7 @@ void complex_conditional_assignment_stat f = false; } - // unchanged; multiple statements + // Unchanged: multiple statements. bool h; if (j > 10) { j = 10; @@ -542,7 +549,7 @@ void complex_conditional_assignment_stat f = false; } -// unchanged: chained return statements, but ChainedConditionalReturn not set +// Unchanged: chained return statements, but ChainedConditionalReturn not set. bool chained_conditional_compound_return(int i) { if (i < 0) { return true; @@ -555,7 +562,7 @@ bool chained_conditional_compound_return } } -// unchanged: chained return statements, but ChainedConditionalReturn not set +// Unchanged: chained return statements, but ChainedConditionalReturn not set. bool chained_conditional_return(int i) { if (i < 0) return true; @@ -567,7 +574,7 @@ bool chained_conditional_return(int i) { return false; } -// unchanged: chained assignments, but ChainedConditionalAssignment not set +// Unchanged: chained assignments, but ChainedConditionalAssignment not set. void chained_conditional_compound_assignment(int i) { bool b; if (i < 0) { @@ -581,7 +588,7 @@ void chained_conditional_compound_assign } } -// unchanged: chained return statements, but ChainedConditionalReturn not set +// Unchanged: chained return statements, but ChainedConditionalReturn not set. void chained_conditional_assignment(int i) { bool b; if (i < 0) @@ -593,3 +600,275 @@ void chained_conditional_assignment(int 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<bool>(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<bool>(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<bool>(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<bool>(~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<bool>(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<bool>(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;{{$}} _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits