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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D82895#2129028 <https://reviews.llvm.org/D82895#2129028>, @jdoerfert wrote:

> > I think that isInnermost() would make even more sense.
>
> I'm fine with adding `bool isInnermost() const { return empty(); }` and while we are at it, `bool isOutermost()`.
>
> @fhahn @Meinersbur @Whitney @etiotto Objections?


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()`. Same for `isInnerMost()`/`empty()`.

Loops form a hierarchical structure and personally I think the existing helpers make sense in that context. It is also consistent with similar data structures (for example, I don't think there are `isLeaf`/`isRoot` helpers for DominatorTree). I would prefer to keep the interface leaner (with the  new docstring for `empty` the docs are quite explicit), but it is not something I feel too strongly about in this case if others feel the benefit outweighs the cost/churn. If they get added, I think the users should be updated, otherwise things get more confusing than they are now.


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

https://reviews.llvm.org/D82895