[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