[lld] r228968 - [ELF] Insert wrap symbols into a set.

[Chandler Carruth]

On Fri, Feb 13, 2015 at 11:30 AM, Rui Ueyama <ruiu at google.com> wrote:

> Returning a const reference to a set is not wrong at all, and it involves
> less moving parts, no?
>
> The other reason why I prefer a set over range here is because I don't
> want to see more uses of range in LLD at this moment. I believe range is
> designed after N3350, which didn't make it to the standard yet. It doesn't
> feel great that this generic library lives under LLD. If we really need
> this one, we should move the file to LLVM support directory. Otherwise, I
> don't like to build a local rule to use range in LLD. After all other LLVM
> projects are not using this range.
>

FWIW, LLVM projects are using ranges pretty heavily now, and we added
llvm::iterator_range to LLVM's ADT library. So I'm pretty happy with
increased usage of ranges in LLD.

However ranges != sets.

Honestly, std::set is the wrong data type here no matter what.

Do you need to preserve order? Use a sorted vector, and return a range (or
just an arrayref), and *document* that it is a sorted order range. That is
a really useful concept, and also allows fast lookups. It will be much
faster than std::set.

Do you not need the order, just need to test whether things are members?
Use a SmallPtrSet or a DenseSet or just use a sorted vector as above. All
of them will be faster than std::set.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150213/03b8f81a/attachment.html>