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

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jul 7 07:56:14 PDT 2018


dexonsmith added a comment.

In https://reviews.llvm.org/D49030#1155065, @labrinea wrote:

> In https://reviews.llvm.org/D49030#1155000, @chandlerc wrote:
>
> > We already have SetVector which is widely used for these patterns. If we need both, we need a clear explanation of the difference and why we need both (IE, why users of one can't use the other).
>
>
> The remove operation on a SetVector can cost linear time and this might be an issue in some cases. Please have a look at the review comments of https://reviews.llvm.org/D48372


Assuming we end up with two ADTs, we'll need: better names, clear header documentation of what's different, and an explanation in https://llvm.org/docs/ProgrammersManual.html.

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.


https://reviews.llvm.org/D49030





More information about the llvm-commits mailing list