baloghadamsoftware added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1743 + + if (Then->isInevitablySinking() || Else->isInevitablySinking()) + return true; ---------------- xazax.hun wrote: > Do we consider a block assert like if both branches are inevitable sinking? No, I think `(Then->isInevitablySinking() != Else->isInevitablySinking())` would serve better here. ================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1750 + // [B1] -> [B2] -> [B3] -> [sink] + // assert(A && B || C); \ \ \ + // -------------------> [go on with the execution] ---------------- xazax.hun wrote: > I wonder if the CFG is correct for your example. If A evaluates to false, we > still check C before the sink. If A evaluates to true we still check B before > going on. But maybe it is just late for me :) I think an arrow from `[B1]` to `[B3]` is missing for the `A == false` case. ================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1760 + + using namespace ast_matchers; + auto IsSubStmtOfCondition = ---------------- I would avoid using matchers in the core infrastructure. My experience says they are quite expensive to create and destroy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65287/new/ https://reviews.llvm.org/D65287 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
