zinovy.nis marked an inline comment as done. zinovy.nis added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:85 // remove the inner `if`. - const auto *BinOpCond = dyn_cast<BinaryOperator>(InnerIf->getCond()); + const auto *BinOpCond = dyn_cast_or_null<BinaryOperator>(InnerIf->getCond()); + if (!BinOpCond) ---------------- aaron.ballman wrote: > zinovy.nis wrote: > > aaron.ballman wrote: > > > zinovy.nis wrote: > > > > aaron.ballman wrote: > > > > > Under what circumstances does `getCond()` return `nullptr`? > > > > `getCond()` is not null, but it can be `ExprWithCleanupsCond` leading > > > > the original `dyn_cast` to crash. That what this bug is about. > > > > > > > That's what I figured, but you changed `dyn_cast<>` to be > > > `dyn_cast_or_null<>` and that seems incorrect -- `getCond()` doesn't > > > return null so the `dyn_cast<>` was correct. > > `dyn_cast` asserts unless `getCond()` returns `BinaryExpression`, right? > > But `getCond()` can return (and it does so in the test case) > > `ExprWithCleanups` which is not a subclass of `BinaryExpression`. > > That's why I use `_or_null` version of `dyn_cast`. > > dyn_cast asserts unless getCond() returns BinaryExpression, right? > > Nope. > > `cast<>` asserts if the cast cannot be completed or if the cast input is null. > `dyn_cast<>` returns null if the cast cannot be completed and asserts if the > cast input is null. > There are `_or_null` variants of each which will accept a null input without > asserting. Yes, you are right, I definitely need a vacation :-) I confused `dyn_cast` with `cast` I've uploaded a fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91037/new/ https://reviews.llvm.org/D91037 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
