[PATCH] Add readability-simplify-boolean-expr check to clang-tidy

Alexander Kornienko alexfh at google.com
Wed Feb 25 07:17:39 PST 2015


This is going to be a really nice check if it's done right. Thanks for the contribution!

There's another pattern (similar to the examples with ternary expressions in the patch description) that annoys me:

  if (<something>)
    return true;
  else
    return false;

Could you add a support for it?

And a few style issues.


================
Comment at: clang-tidy/readability/SimplifyBooleanExpr.cpp:16
@@ +15,3 @@
+
+using namespace clang;
+using namespace clang::ast_matchers;
----------------
I'd remove this and start namespace clang right after the next line.

================
Comment at: clang-tidy/readability/SimplifyBooleanExpr.cpp:34
@@ +33,3 @@
+
+char const *const LeftBooleanLiteralId = "bool-op-expr-yields-bool";
+char const *const RightExpressionId = "bool-op-expr-yields-expr";
----------------
I'd go with
  const char Something[] = "...";
as it's more readable due to fewer `const`s.

================
Comment at: clang-tidy/readability/SimplifyBooleanExpr.cpp:282
@@ +281,3 @@
+    diag(Ternary->getTrueExpr()->getLocStart(),
+            "Redundant boolean literal in ternary expression result.")
+        << FixItHint::CreateReplacement(
----------------
Please use the style of Clang warnings: first letter is lower-case, no trailing period.

================
Comment at: clang-tidy/readability/SimplifyBooleanExpr.h:26
@@ +25,3 @@
+class SimplifyBooleanExpr : public ClangTidyCheck {
+public:
+    SimplifyBooleanExpr(StringRef Name, ClangTidyContext *Context)
----------------
Please clang-format the code with the LLVM style.

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:11
@@ +10,3 @@
+bool ab = true == a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: Redundant boolean constant supplied to boolean operator. [readability-simplify-boolean-expr]
+// CHECK-FIXES: {{^}}bool ab = a1;{{$}}
----------------
Please leave each distinct error message completely only once, and shorten all other copies of it. 

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:62
@@ +61,3 @@
+
+void if_with_bool_literal_condition()
+{
----------------
Please clang-format the test with LLVM style and infinite column limit (to avoid breaking the CHECK lines).

http://reviews.llvm.org/D7648

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list