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

[Rui Ueyama]

On Fri, Feb 13, 2015 at 11:55 AM, Chandler Carruth <chandlerc at google.com>
wrote:

>
> 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.
>

Thanks, Chandler. Maybe we should replace all uses of lld::range with
llvm::iterator_range? Looks like there's no reason to not do.


>
> 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/20150217/10a8e455/attachment.html>