[PATCH] D121817: [llvm][ADT] Add a method to MapVector for erasing a range of elements.

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added a comment.

I agree with the complexity analysis here. I don't find a better solution if we don't add new data structures like a map from `VectorType::iterator` to `MapType::iterator`. It might benefit for `erase` but it is redundant for other methods. I think this patch is beneficial. And I feel it is good to comment that `MapVector` is not good at `erase()`.



================
Comment at: llvm/include/llvm/ADT/MapVector.h:49
   using size_type = typename VectorType::size_type;
+  using difference_type = typename VectorType::difference_type;
 
----------------
This looks not necessary.


================
Comment at: llvm/include/llvm/ADT/MapVector.h:207
+    difference_type NumElts = std::distance(S, E);
+    assert(NumElts > 0 && "Range end is not greater than range start!");
+    for (auto I = S; I != E; ++I)
----------------
efriedma wrote:
> Asserting that you can't erase an empty range seems weird to me; standard library containers treat it as a no-op.
We could also assert `begin() < S && S < E && E < end()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121817