aaron.ballman added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:66
+  if (const auto *Negated = Result.Nodes.getNodeAs<UnaryOperator>(Id)) {
+    if (Negated->getOpcode() == UO_LNot &&
+        isa<CXXBoolLiteralExpr>(Negated->getSubExpr()))
----------------
I'd like to see this handled recursively instead so that we properly handle 
more esoteric logic (but it still shows up from time to time) like `!!foo`: 
https://codesearch.isocpp.org/cgi-bin/cgi_ppsearch?q=%21%21&search=Search

This is a case where I don't think the simplification should happen -- e.g., 
the code is usually subtly incorrect when converted to remove the `!!`. It's 
less clear to me whether the same is true for something like `!!!` being 
converted to `!`, but that's not a case I'm really worried about either.


================
Comment at: 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:74
+internal::BindableMatcher<Stmt> literalOrNegatedBool(bool Value) {
+  return expr(anyOf(cxxBoolLiteral(equals(Value)),
+                    unaryOperator(hasUnaryOperand(ignoringParenImpCasts(
----------------
Oof, but handling it here may be tricky...


================
Comment at: 
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:289
   }
+  if (const auto *Unary = dyn_cast<UnaryOperator>(Ret->getRetValue())) {
+    if (Unary->getOpcode() == UO_LNot) {
----------------
Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86176

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to