[PATCH] D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.


================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:386
 
-  bool BoolValue = Bool->getValue();
+  const bool BoolValue = Bool->getValue();
 
----------------
`const` on values is uncommon in clang-tidy code. Please keep that consistent.


================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:743
+  // if the next statement after the if contains a return statement of
+  // the correct form.  Ideally we'd be able to express this with the
+  // matchers, but that is currently impossible.
----------------
double space


================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:745
+  // matchers, but that is currently impossible.
+  //
+  const auto *If = dyn_cast<IfStmt>(SwitchCase->getSubStmt());
----------------
redundant empty comment line


================
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:747
+  const auto *If = dyn_cast<IfStmt>(SwitchCase->getSubStmt());
+  assert(If != nullptr);
+  const CXXBoolLiteralExpr *Lit = stmtReturnsBool(If, Negated);
----------------
I think this assertion does not hold true from the matcher.
The matcher requires only `hasDescendent(ifStmt())`, but that does not ensure the first stmt is the `ifStmt`.

e.g.
```
case 10: {
  loggingCall();
  if(foo) ...
```

Is this excluded?
}


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56303/new/

https://reviews.llvm.org/D56303