sgatev added inline comments.
================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:157
+// Returns whether `Block` is a "back edge" in the CFG. Such a block has only
+// one successor, the start of the loop.
----------------
Let's start function comments with `///` throughout the file.
================
Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:164
+
+// Returns the predecessor to `Block` that is a "back edge", if one exists.
+//
----------------
================
Comment at:
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:167-168
+// If this function returns a non-null pointer, that means `Block` dominates
the
+// back edge block. (That is, all paths from the entry block to the back edge
+// block must go through `Block`.) It also means that there are only two
+// predecessors; the other is along the path from the entry block to `Block`.
----------------
================
Comment at:
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:168-169
+// back edge block. (That is, all paths from the entry block to the back edge
+// block must go through `Block`.) It also means that there are only two
+// predecessors; the other is along the path from the entry block to `Block`.
+static const CFGBlock *findBackEdge(const CFGBlock *Block) {
----------------
li.zhe.hua wrote:
> xazax.hun wrote:
> > Is this also true when we have multiple `continue` statements in the loop?
> Yes. The end of the loop, and each of the `continue` statements, target the
> back edge block. They all get funneled through that back edge to `Block`,
> such that `Block` only has two predecessors. However, I haven't verified this
> in the CFG code, only by not finding a counterexample.
Does that hold for back edges stemming from `goto` statements?
================
Comment at:
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:270
+
+TEST(DataflowAnalysisTest, ConvergesOnWidenAnalysis) {
+ std::string Code = R"(
----------------
There's already a set of "widening" tests -
http://google3/third_party/llvm/llvm-project/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp;l=527-712;rcl=462638952
What do you think about refactoring those so that we have tests that exercise
the framework with both `join` and `widen`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131646/new/
https://reviews.llvm.org/D131646
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
