[PATCH] D130891: [Docs] Improve cycle and closed path definitions

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/docs/CycleTerminology.rst:28
+   with respect to this particular DFS. The header is always an entry node.
+4. In any depth-first search starting from the entry, the set of cycles
+   found in the CFG is the same. These are the *top-level cycles*
----------------
jsilvanus wrote:
> foad wrote:
> > Would it make sense to swap 3 and 4, so you can define the "top-level cycles" without even mentioning DFS?
> You mean one could define "top-level cycles" without mentioning a particular DFS choice?
> 
> The current order has the advantage that entries and headers are adjacent, resembling their close relationship.
> But I do not have a strong opinion here.
> You mean one could define "top-level cycles" without mentioning a particular DFS choice?

Yes. The sentence "In any depth-first search starting from the entry, the set of cycles found in the CFG is the same" is really strange. Why would you even mention DFS here??? It should be clear from (1) that cycles have nothing to do with any DFS ordering. It's like saying "the set of cycles found in the CFG is the same regardless of whether Denmark and Canada share a land border" :)

I guess the only reason that (4) mentions a DFS is that (3) has already mentioned DFS. That's why I suggested reordering them. But perhaps we could keep the order the same and still remove DFS from (4)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130891/new/

https://reviews.llvm.org/D130891