JonasToth added a comment.
Thanks for the patch. Is this revision dependent on D56303
<https://reviews.llvm.org/D56303> (or other way around)?
================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:464
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)));
----------------
The `const` for these locals in uncommon in clang-tidy, given its a value type.
I am not strongly against them, but would prefer consistency.
================
Comment at: test/clang-tidy/readability-simplify-bool-expr-members.cpp:4
+struct X {
+ explicit operator bool();
+};
----------------
I didn't see a test that utilized this struct, did I overlook it?
Implicit conversion to `bool`-cases would be interesting as well. Maybe
implicit conversion to integral in general.
================
Comment at: test/clang-tidy/readability-simplify-bool-expr-members.cpp:35
+ bool m_b3 = false;
+ bool m_b4 = false;
+ int *m_p = nullptr;
----------------
Would bitfields with a single bit be of interest as well?
================
Comment at: test/clang-tidy/readability-simplify-bool-expr-members.cpp:160
+ // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}} in ternary expression
result
+ // CHECK-FIXES: {{^ m_b2 = i <= 20;$}}
+
----------------
For this case the logical inversion is simple an obviously `<=`, but what if
the condition is more complex?
I would personally prefer `!(i < 20)` instead + complex logical expressions as
additional test. They would be interesting for the `if` cases, too.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56323/new/
https://reviews.llvm.org/D56323
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
