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

Richard legalize at xmission.com
Wed Mar 18 23:26:02 PDT 2015


================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:254
@@ +253,3 @@
+    const CXXBoolLiteralExpr *BoolLiteral, bool UseLHS, bool Negated) {
+  const auto LHS = Result.Nodes.getNodeAs<Expr>(LHSId);
+  const auto RHS = Result.Nodes.getNodeAs<Expr>(RHSId);
----------------
alexfh wrote:
> Please use "const auto *" for pointers.
Fixed

================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:315
@@ +314,3 @@
+  std::string Replacement =
+      (Negated ? (VariableName + " = !(" + Condition + ")" + Terminator)
+               : (VariableName + " = " + Condition + Terminator)).str();
----------------
alexfh wrote:
> 1. Parentheses are not always needed. I'd suggest adding a couple of helper functions to format negation correctly. These may be missing something, but should be mostly correct:
> 
> 
>     bool needsParensAfterUnaryNegation(const Expr &E) {
>       if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E))
>         return true;
>       if (const auto &Op = dyn_cast<CXXOperatorCallExpr>(E))
>         return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call && Op->getOperator() != OO_Subscript;
>       return false;
>     }
> 
>   std::string negateExpression(const MatchFinder::MatchResult &Result, const Expr &E) {
>     StringRef Text = getText(Result, E);
>     return (needsParensAfterUnaryNegation(E) ? "!(" + Text + ")" : "!" + Text).str();
>   }
> 
> This applies to other replaceWith* methods as well.
> 
> 2. How about moving out all non-conditional parts to shorten the expression?
> 
>   (VariableName + " = " + (Negated ? "!(" + Condition + ")" : Condition) + Terminator).str();
Fixed

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:51
@@ +50,3 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: {{.*}} to boolean operator
+// X-CHECK-FIXES: {{^bool a8 = false;$}}
+bool a9 = a1 && true;
----------------
alexfh wrote:
> Why?
Fixed

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:62
@@ +61,3 @@
+
+void if_with_bool_literal_condition()
+{
----------------
alexfh wrote:
> Please make the test code closer to LLVM style, except for the places where special formatting is required for testing whitespace handling. In particular: 2 space indentation, braces on the same line as function headers or control flow statements, function names should be camelCaseStartingWithALowerCaseLetter, comments should use proper capitalization and punctuation.
Fixed

================
Comment at: test/clang-tidy/readability-simplify-bool-expr.cpp:170
@@ +169,3 @@
+        // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: {{.*}} to boolean operator
+        // X-CHECK-FIXES: {{^    if \(true\) {$}}
+        i = 20;
----------------
alexfh wrote:
> Why?
Fixed

http://reviews.llvm.org/D7648

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






More information about the cfe-commits mailing list