[PATCH] D49030: Add OrderedSet, with constant-time insertion and removal, and random access iteration.

Alexandros Lamprineas via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 10 02:18:37 PDT 2018


labrinea added a comment.

In https://reviews.llvm.org/D49030#1155072, @dexonsmith wrote:

> Personally, I'd be in favour of changing MapVector to just do this, since it already has an index.  I'd also be in favour of adding an index to SetVector to match.  There'd be some work to do in auditing/updating users, but I doubt there are many calls to `erase()` and I'm skeptical that anyone relies on the current order for something other than determinism.


I think we should not change MapVector. Its underlying data structure is a vector with <key,value> pairs but we only need to store a single value. Moreover, both MapVector and SetVector provide insertion order iteration and their users might rely on that. I am not sure we should change that. SmallSet might be a good alternative for lower insertion/deletion complexity, but at the moment it does not provide iteration over the underlying data structures. It uses a SmallVector for small data sets and expands it to an std::set if it grows too much. Is there a way to abstract those underlying iterators? If not I think we should keep this new ADT and rename/document it properly.


https://reviews.llvm.org/D49030





More information about the llvm-commits mailing list