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()))
----------------
njames93 wrote:
> aaron.ballman wrote:
> > 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.
> As this is only looking for boolean literals that shouldn't matter
> `!!true` is identical to `true`.
> I get putting `!!` in front of expressions that aren't boolean has an effect, 
> but we aren't looking for that in this case.
Oh, derp, I saw "SimplifyBooleanExprCheck" and assumed it was simplifying 
arbitrary boolean expressions, not just literals. I am now far less worried 
about supporting this case, it's up to you if you want to do the extra work. 
Thank you for clarifying and sorry for my think-o!


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