[PATCH] Enhance clang-tidy readability-simplify-boolean-expr to handle 'if (e) return true; return false; '

[Alexander Kornienko]

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/