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: > 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. ================ 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( ---------------- aaron.ballman wrote: > Oof, but handling it here may be tricky... A custom matcher could be made for this that could traverse a `UnaryOperator` ================ 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) { ---------------- aaron.ballman wrote: > Same here. This could be easily handled with some recursion. 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