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

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 17 09:32:39 PST 2018


labath marked an inline comment as done.
labath added inline comments.


================
Comment at: include/llvm/ADT/IntervalMap.h:1140
+  /// the range [a;b]
+  bool contains(KeyT a, KeyT b) {
+    assert(Traits::nonEmpty(a, b));
----------------
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)?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55760





More information about the llvm-commits mailing list