LegalizeAdulthood updated this revision to Diff 180274.
LegalizeAdulthood added a comment.
Fix typo in id
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56323/new/
https://reviews.llvm.org/D56323
Files:
clang-tidy/readability/SimplifyBooleanExprCheck.cpp
test/clang-tidy/readability-simplify-bool-expr-members.cpp
Index: test/clang-tidy/readability-simplify-bool-expr-members.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/readability-simplify-bool-expr-members.cpp
@@ -0,0 +1,360 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+struct X {
+ explicit operator bool();
+};
+
+class A {
+public:
+ int m;
+};
+
+struct S {
+ S() : m_ar(s_a) {}
+
+ void operator_equals();
+ void operator_or();
+ void operator_and();
+ void ternary_operator();
+ void operator_not_equal();
+ void simple_conditional_assignment_statements();
+ void complex_conditional_assignment_statements();
+ void chained_conditional_assignment();
+ bool non_null_pointer_condition();
+ bool null_pointer_condition();
+ bool negated_non_null_pointer_condition();
+ bool negated_null_pointer_condition();
+ bool integer_not_zero();
+ bool member_pointer_nullptr();
+ bool integer_member_implicit_cast();
+ bool expr_with_cleanups();
+
+ bool m_b1 = false;
+ bool m_b2 = false;
+ bool m_b3 = false;
+ bool m_b4 = false;
+ int *m_p = nullptr;
+ int A::*m_m = nullptr;
+ int m_i = 0;
+ A *m_a = nullptr;
+ static A s_a;
+ A &m_ar;
+};
+
+void S::operator_equals() {
+ int i = 0;
+ m_b1 = (i > 2);
+ if (m_b1 == true) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(m_b1\) {$}}
+ i = 5;
+ } else {
+ i = 6;
+ }
+ m_b2 = (i > 4);
+ if (m_b2 == false) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(!m_b2\) {$}}
+ i = 7;
+ } else {
+ i = 9;
+ }
+ m_b3 = (i > 6);
+ if (true == m_b3) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(m_b3\) {$}}
+ i = 10;
+ } else {
+ i = 11;
+ }
+ m_b4 = (i > 8);
+ if (false == m_b4) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(!m_b4\) {$}}
+ i = 12;
+ } else {
+ i = 13;
+ }
+}
+
+void S::operator_or() {
+ int i = 0;
+ m_b1 = (i > 10);
+ if (m_b1 || false) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(m_b1\) {$}}
+ i = 14;
+ } else {
+ i = 15;
+ }
+ m_b2 = (i > 10);
+ if (m_b2 || true) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(true\) {$}}
+ i = 16;
+ } else {
+ i = 17;
+ }
+ m_b3 = (i > 10);
+ if (false || m_b3) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(m_b3\) {$}}
+ i = 18;
+ } else {
+ i = 19;
+ }
+ m_b4 = (i > 10);
+ if (true || m_b4) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(true\) {$}}
+ i = 20;
+ } else {
+ i = 21;
+ }
+}
+
+void S::operator_and() {
+ int i = 0;
+ m_b1 = (i > 20);
+ if (m_b1 && false) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(false\) {$}}
+ i = 22;
+ } else {
+ i = 23;
+ }
+ m_b2 = (i > 20);
+ if (m_b2 && true) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(m_b2\) {$}}
+ i = 24;
+ } else {
+ i = 25;
+ }
+ m_b3 = (i > 20);
+ if (false && m_b3) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(false\) {$}}
+ i = 26;
+ } else {
+ i = 27;
+ }
+ m_b4 = (i > 20);
+ if (true && m_b4) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(m_b4\) {$}}
+ i = 28;
+ } else {
+ i = 29;
+ }
+}
+
+void S::ternary_operator() {
+ int i = 0;
+ m_b1 = (i > 20) ? true : false;
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in ternary expression result
+ // CHECK-FIXES: {{^ m_b1 = i > 20;$}}
+
+ m_b2 = (i > 20) ? false : true;
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in ternary expression result
+ // CHECK-FIXES: {{^ m_b2 = i <= 20;$}}
+
+ m_b3 = ((i > 20)) ? false : true;
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: {{.*}} in ternary expression result
+ // CHECK-FIXES: {{^ m_b3 = i <= 20;$}}
+}
+
+void S::operator_not_equal() {
+ int i = 0;
+ m_b1 = (i > 20);
+ if (false != m_b1) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(m_b1\) {$}}
+ i = 30;
+ } else {
+ i = 31;
+ }
+ m_b2 = (i > 20);
+ if (true != m_b2) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(!m_b2\) {$}}
+ i = 32;
+ } else {
+ i = 33;
+ }
+ m_b3 = (i > 20);
+ if (m_b3 != false) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(m_b3\) {$}}
+ i = 34;
+ } else {
+ i = 35;
+ }
+ m_b4 = (i > 20);
+ if (m_b4 != true) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: {{.*}} to boolean operator
+ // CHECK-FIXES: {{^ if \(!m_b4\) {$}}
+ i = 36;
+ } else {
+ i = 37;
+ }
+}
+
+void S::simple_conditional_assignment_statements() {
+ if (m_i > 10)
+ m_b1 = true;
+ else
+ m_b1 = false;
+ bool bb = false;
+ // CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: {{^ }}m_b1 = m_i > 10;{{$}}
+ // CHECK-FIXES: bool bb = false;
+
+ if (m_i > 20)
+ m_b2 = false;
+ else
+ m_b2 = true;
+ bool c2 = false;
+ // CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: {{^ }}m_b2 = m_i <= 20;{{$}}
+ // CHECK-FIXES: bool c2 = false;
+
+ // Unchanged: different variables.
+ if (m_i > 12)
+ m_b1 = true;
+ else
+ m_b2 = false;
+
+ // Unchanged: no else statement.
+ if (m_i > 15)
+ m_b3 = true;
+
+ // Unchanged: not boolean assignment.
+ int j;
+ if (m_i > 17)
+ j = 10;
+ else
+ j = 20;
+
+ // Unchanged: different variables assigned.
+ int k = 0;
+ m_b4 = false;
+ if (m_i > 10)
+ m_b4 = true;
+ else
+ k = 10;
+}
+
+void S::complex_conditional_assignment_statements() {
+ if (m_i > 30) {
+ m_b1 = true;
+ } else {
+ m_b1 = false;
+ }
+ m_b1 = false;
+ // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: {{^ }}m_b1 = m_i > 30;{{$}}
+ // CHECK-FIXES: m_b1 = false;
+
+ if (m_i > 40) {
+ m_b2 = false;
+ } else {
+ m_b2 = true;
+ }
+ m_b2 = false;
+ // CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional assignment
+ // CHECK-FIXES: {{^ }}m_b2 = m_i <= 40;{{$}}
+ // CHECK-FIXES: m_b2 = false;
+}
+
+// Unchanged: chained return statements, but ChainedConditionalReturn not set.
+void S::chained_conditional_assignment() {
+ if (m_i < 0)
+ m_b1 = true;
+ else if (m_i < 10)
+ m_b1 = false;
+ else if (m_i > 20)
+ m_b1 = true;
+ else
+ m_b1 = false;
+}
+
+bool S::non_null_pointer_condition() {
+ if (m_p) {
+ return true;
+ } else {
+ return false;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return m_p != nullptr;{{$}}
+
+bool S::null_pointer_condition() {
+ if (!m_p) {
+ return true;
+ } else {
+ return false;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return m_p == nullptr;{{$}}
+
+bool S::negated_non_null_pointer_condition() {
+ if (m_p) {
+ return false;
+ } else {
+ return true;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return m_p == nullptr;{{$}}
+
+bool S::negated_null_pointer_condition() {
+ if (!m_p) {
+ return false;
+ } else {
+ return true;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return m_p != nullptr;{{$}}
+
+bool S::integer_not_zero() {
+ if (m_i) {
+ return false;
+ } else {
+ return true;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}} return m_i == 0;{{$}}
+
+bool S::member_pointer_nullptr() {
+ if (m_m) {
+ return true;
+ } else {
+ return false;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return m_m != nullptr;{{$}}
+
+bool S::integer_member_implicit_cast() {
+ if (m_a->m) {
+ return true;
+ } else {
+ return false;
+ }
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: return m_a->m != 0;{{$}}
+
+bool operator!=(const A &, const A &) { return false; }
+bool S::expr_with_cleanups() {
+ if (m_ar != (A)m_ar)
+ return false;
+
+ return true;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: {{.*}} in conditional return
+// CHECK-FIXES: m_ar == (A)m_ar;{{$}}
Index: clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===================================================================
--- clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -45,7 +45,8 @@
const char IfAssignLocId[] = "if-assign-loc";
const char IfAssignBoolId[] = "if-assign";
const char IfAssignNotBoolId[] = "if-assign-not";
-const char IfAssignObjId[] = "if-assign-obj";
+const char IfAssignVarId[] = "if-assign-var";
+const char IfAssignMemVarId[] = "if-assign-mem-bar";
const char CompoundReturnId[] = "compound-return";
const char CompoundBoolId[] = "compound-bool";
const char CompoundNotBoolId[] = "compound-bool-not";
@@ -460,29 +461,33 @@
void SimplifyBooleanExprCheck::matchIfAssignsBool(MatchFinder *Finder,
bool Value, StringRef Id) {
- auto SimpleThen = binaryOperator(
- hasOperatorName("="),
- hasLHS(declRefExpr(hasDeclaration(decl().bind(IfAssignObjId)))),
- hasLHS(expr().bind(IfAssignVariableId)),
- hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId)));
- auto Then = anyOf(SimpleThen, compoundStmt(statementCountIs(1),
- hasAnySubstatement(SimpleThen)));
- auto SimpleElse = binaryOperator(
- hasOperatorName("="),
- hasLHS(declRefExpr(hasDeclaration(equalsBoundNode(IfAssignObjId)))),
- hasRHS(cxxBoolLiteral(equals(!Value))));
- auto Else = anyOf(SimpleElse, compoundStmt(statementCountIs(1),
- hasAnySubstatement(SimpleElse)));
+ const auto VarAssign =
+ declRefExpr(hasDeclaration(decl().bind(IfAssignVarId)));
+ const auto VarRef =
+ declRefExpr(hasDeclaration(equalsBoundNode(IfAssignVarId)));
+ const auto MemAssign = memberExpr(hasDeclaration(decl().bind(IfAssignVarId)));
+ const auto MemRef =
+ memberExpr(hasDeclaration(equalsBoundNode(IfAssignVarId)));
+ const auto SimpleThen =
+ binaryOperator(hasOperatorName("="), hasLHS(anyOf(VarAssign, MemAssign)),
+ hasLHS(expr().bind(IfAssignVariableId)),
+ hasRHS(cxxBoolLiteral(equals(Value)).bind(IfAssignLocId)));
+ const auto Then =
+ anyOf(SimpleThen,
+ compoundStmt(statementCountIs(1), hasAnySubstatement(SimpleThen)));
+ const auto SimpleElse =
+ binaryOperator(hasOperatorName("="), hasLHS(anyOf(VarRef, MemRef)),
+ hasRHS(cxxBoolLiteral(equals(!Value))));
+ const auto Else =
+ anyOf(SimpleElse,
+ compoundStmt(statementCountIs(1), hasAnySubstatement(SimpleElse)));
if (ChainedConditionalAssignment)
+ Finder->addMatcher(ifStmt(hasThen(Then), hasElse(Else)).bind(Id), this);
+ else
Finder->addMatcher(
- ifStmt(isExpansionInMainFile(), hasThen(Then), hasElse(Else)).bind(Id),
+ ifStmt(unless(hasParent(ifStmt())), hasThen(Then), hasElse(Else))
+ .bind(Id),
this);
- else
- Finder->addMatcher(ifStmt(isExpansionInMainFile(),
- unless(hasParent(ifStmt())), hasThen(Then),
- hasElse(Else))
- .bind(Id),
- this);
}
void SimplifyBooleanExprCheck::matchCompoundIfReturnsBool(MatchFinder *Finder,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
