njames93 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()))
----------------
aaron.ballman wrote:
> 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!
Personally I don't think the extra complexity is worth it. Usages of `!true`, I
expect are rather low but likely show up in a few places, however I doubt
anyone would use `!!true`.
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
