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

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis marked an inline comment as done.
baziotis added a comment.

In D82895#2137834 <https://reviews.llvm.org/D82895#2137834>, @Meinersbur wrote:

> 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.


This the orthogonality vs expressiveness and I'm sure we all value each under certain circumstances. That said, I too agree that if we were to take
a step towards orthogonality, that would be to replace `empty()`, rather than not introducing `isInnermost/Outermost`.



================
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; }
----------------
Meinersbur wrote:
> make this a doc-string as well?
I added it as non-doc comment intentionally but probably bad thought. I'll fix that.


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

https://reviews.llvm.org/D82895