[PATCH] Enhance clang-tidy readability-simplify-boolean-expr to handle 'if (e) return true; return false; '
Richard
legalize at xmission.com
Sat May 30 20:31:54 PDT 2015
================
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/
More information about the cfe-commits
mailing list