[PATCH] D74890: [Analysis] getParentLoop() documentation

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis marked 2 inline comments as done.
baziotis added inline comments.


================
Comment at: llvm/docs/LoopTerminology.rst:120-121
 
+Parent Loop - A loop is either a top level loop, that is it
+is not enclosed in any other loop or it is a sub-loop of another.
+A parent of a loop is the innermost loop in which it is enclosed in.
----------------
Meinersbur wrote:
> [grammar] "that is it is"
> 
> I also don't understand this part of a definition. Does it clarify in what positions a loop can be (it makes it an unusually structured dictionary definition -- I'd expect the first sentence to explain what a "parent loop" is followed by further explanations)? IMHO the LoopInfo being a [[ https://en.wikipedia.org/wiki/Tree_(graph_theory)#Forest | forest  ]] is enough information.
Ok, thanks for the comment, indeed the definition structure is kind of bad. Regarding the last note, I think it's not stated in the terminology (although it's stated in some parts of `LoopInfo.h`). Do you think it would be good to add this clarification?
Or maybe a last point in the introduction: "A loop can have only a single parent loop (in which it is entirely contained)". Something like that. Basically just to make obvious that a loop can have only a single parent.


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:106
   BlockT *getHeader() const { return getBlocks().front(); }
+  /// Return the parent loop if it exists or nullptr otherwise.
+
----------------
Meinersbur wrote:
> Another suggestion: "Return the parent loop or nullptr for topmost loops"
Ok. I'm going with "top level" for consistency with the terminology but please comment if I shouldn't.


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

https://reviews.llvm.org/D74890