[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