[PATCH] D25404: [ADT] Let MapVector handle non-copyable values.

[Tim Shen via llvm-commits]

timshen added inline comments.


================
Comment at: llvm/include/llvm/ADT/MapVector.h:113
+      Vector.push_back(
+          std::make_pair(std::move(KV.first), std::move(KV.second)));
+      I = Vector.size() - 1;
----------------
std::move(KV)?


================
Comment at: llvm/include/llvm/ADT/MapVector.h:117
+    }
+    return std::make_pair(begin() + I, false);
+  }
----------------
Do you want to static_assert on random_access_iterator_tag in the class?


================
Comment at: llvm/include/llvm/ADT/MapVector.h:159
     size_t Index = Next - Vector.begin();
     for (auto &I : Map) {
       assert(I.second != Index && "Index was already erased!");
----------------
This O(n) behavior is unfortunate. It could have been O(1), by swapping the to-be-erased element with back(), then pop_back().

Just a comment, no change required. :)


https://reviews.llvm.org/D25404