================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:134
@@ +133,3 @@
+                     const CXXBoolLiteralExpr *&Lit) {
+  if (auto *Bool = dyn_cast<CXXBoolLiteralExpr>(Ret->getRetValue())) {
+    Lit = Bool;
----------------
alexfh wrote:
> Please use `const auto *` to make the constness explicit.
Fixed.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:141
@@ +140,3 @@
+
+bool stmtReturnsBool(IfStmt *IfRet, bool Negated,
+                     const CXXBoolLiteralExpr *&Lit) {
----------------
alexfh wrote:
> 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.
Any reason to not return just the pointer and compare against `nulltpr`?

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:147
@@ +146,3 @@
+
+  if (auto *Ret = dyn_cast<ReturnStmt>(IfRet->getThen())) {
+    return stmtReturnsBool(Ret, Negated, Lit);
----------------
alexfh wrote:
> Please use `const auto *` to make the constness explicit. Also two times 
> below.
Fixed.

================
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) {
----------------
alexfh wrote:
> nit: No `else` after `return` please.
Fixed.

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:451
@@ +450,3 @@
+       ++BeforeIt, ++AfterIt) {
+    if (auto *If = dyn_cast<IfStmt>(*BeforeIt)) {
+      const CXXBoolLiteralExpr *Lit = nullptr;
----------------
alexfh wrote:
> Please use `const auto *` to make the constness explicit.
Fixed.

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:704
@@ +703,3 @@
+// CHECK-FIXES: {{^}}    return false;{{$}}
+// CHECK-FIXES: {{^}}  }{{$}}
+// CHECK-FIXES: {{^  return i <= 10;$}}
----------------
alexfh wrote:
> How about making it a single regexp?
> 
>   {{^  }$}}
> 
> Same applies for other similar lines.
Fixed.

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

Reply via email to