[PATCH] D55760: [ADT] IntervalMap: add contains(a, b) method

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

I think it'll be really useful to have this helper around. Thanks, lgtm with the name change!



================
Comment at: include/llvm/ADT/IntervalMap.h:1140
+  /// the range [a;b]
+  bool contains(KeyT a, KeyT b) {
+    assert(Traits::nonEmpty(a, b));
----------------
labath wrote:
> vsk wrote:
> > When I read ‘contains’, I tend to assume ‘contains entire subset/subrange’. Would ‘containsSubrangeOf’ or some variant help readability?
> Good question. I chose `contains` because this is similar in functionality to `std::map::contains`, but it is true that this whole class is not very STL-like, so maybe it's not good to try to appear like it is.
> 
> If we're going to go with a custom name, then maybe `overlaps` could do the trick (there is a `IntervalMapOverlaps` class at the bottom of this file, which is doing something similar, but for two maps)?
'overlaps' sgtm.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55760