I'd make an exception for chained conditional returns: if (X) return true; if (Y) return true; if (Z) return true; return false;
It's arguable whether replacing the last piece with `return Z;` is an improvement, so I'd better be conservative here. ================ Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:134 @@ +133,3 @@ + const CXXBoolLiteralExpr *&Lit) { + if (auto *Bool = dyn_cast<CXXBoolLiteralExpr>(Ret->getRetValue())) { + Lit = Bool; ---------------- Please use `const auto *` to make the constness explicit. ================ Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:141 @@ +140,3 @@ + +bool stmtReturnsBool(IfStmt *IfRet, bool Negated, + const CXXBoolLiteralExpr *&Lit) { ---------------- Instead of using the return value and additionally return a literal by reference, I'd suggest making the function return `llvm::Optional<const CXXBoolLiteralExpr*>`. Same above. ================ Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:147 @@ +146,3 @@ + + if (auto *Ret = dyn_cast<ReturnStmt>(IfRet->getThen())) { + return stmtReturnsBool(Ret, Negated, Lit); ---------------- Please use `const auto *` to make the constness explicit. Also two times below. ================ Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:149 @@ +148,3 @@ + return stmtReturnsBool(Ret, Negated, Lit); + } else if (auto *Compound = dyn_cast<CompoundStmt>(IfRet->getThen())) { + if (Compound->size() == 1) { ---------------- nit: No `else` after `return` please. ================ Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:451 @@ +450,3 @@ + ++BeforeIt, ++AfterIt) { + if (auto *If = dyn_cast<IfStmt>(*BeforeIt)) { + const CXXBoolLiteralExpr *Lit = nullptr; ---------------- Please use `const auto *` to make the constness explicit. ================ Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:704 @@ +703,3 @@ +// CHECK-FIXES: {{^}} return false;{{$}} +// CHECK-FIXES: {{^}} }{{$}} +// CHECK-FIXES: {{^ return i <= 10;$}} ---------------- How about making it a single regexp? {{^ }$}} Same applies for other similar lines. http://reviews.llvm.org/D9810 EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/ _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits