xazax.hun added inline comments.
================ Comment at: clang/include/clang/Analysis/CFG.h:860 + /// child CFG blocks in a depth-first manner and see if all paths eventually + /// end up in an immediate sink block. + bool isInevitablySinking() const; ---------------- Is it obvious what sink means in CFG's context? Maybe we should mention what our definition of sink is? Think about non-csa devs too :) ================ Comment at: clang/lib/Analysis/CFG.cpp:5628 + + // FIXME: Throw-expressions are currently generating sinks during analysis: + // they're not supported yet, and also often used for actually terminating ---------------- Are there other nodes that are treated as sinks? If so I wonder if there is a way to keep the list updated. Oh, I just saw that this code is just moved around. ================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1743 + + if (Then->isInevitablySinking() || Else->isInevitablySinking()) + return true; ---------------- Do we consider a block assert like if both branches are inevitable sinking? ================ Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1750 + // [B1] -> [B2] -> [B3] -> [sink] + // assert(A && B || C); \ \ \ + // -------------------> [go on with the execution] ---------------- 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 :) 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
