[PATCH] D82895: [NFC][LoopInfo] Document empty()

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added a comment.

In D82895#2130151 <https://reviews.llvm.org/D82895#2130151>, @fhahn wrote:

> Hm, adding multiple ways to check the same thing may lead to other problems. IMO it will be more confusing if some places use `!L->getParentLoop()` and other `L->isOutermost()`.


There are often multiple ways for the same goal, for instance `LoadInst::getPointerOperand()` and `LoadInst::getOperand(0)`. One makes the intention clearer than the other.

> Same for `isInnerMost()`/`empty()`.

Since these are identical, I'd tend to remove one. In these case I'd remove `empty()` since `Loop` does not represent a container.



================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:160
   bool empty() const { return getSubLoops().empty(); }
+  bool isInnermost() const { return empty(); }
+  // Outermost is the same as top-level.
----------------
`isInnermost` should be documented using a doc-string as well (or grouped together with `empty()` using `/// @{` / `/// @}`)


================
Comment at: llvm/include/llvm/Analysis/LoopInfo.h:161
+  bool isInnermost() const { return empty(); }
+  // Outermost is the same as top-level.
+  bool isOutermost() const { return getParentLoop() == nullptr; }
----------------
make this a doc-string as well?


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

https://reviews.llvm.org/D82895