[PATCH] D43254: Use LLVM's DenseSet insteead of unordered_set.

[Rui Ueyama via llvm-commits]

On Tue, Feb 20, 2018 at 10:32 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Tue, Feb 13, 2018 at 1:49 PM Rui Ueyama via Phabricator via
> llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>> ruiu added inline comments.
>>
>>
>> ================
>> Comment at: lld/wasm/SymbolTable.cpp:37
>>  void SymbolTable::reportRemainingUndefines() {
>> -  std::unordered_set<Symbol *> Undefs;
>> +  DenseSet<Symbol *> Undefs;
>>    for (Symbol *Sym : SymVector) {
>> ----------------
>> efriedma wrote:
>> > ruiu wrote:
>> > > efriedma wrote:
>> > > > Don't you need a SetVector here?
>> > > Forgive my ignorance, but what's the difference between DenseSet and
>> SetVector?
>> > Iterating over a SetVector produces values in the order they were
>> inserted; iterating over a DenseSet uses an arbitrary ordering.  See
>> http://llvm.org/docs/ProgrammersManual.html#llvm-adt-setvector-h .
>> Ah, thanks. This code is to produce error messages, so the order of the
>> output of this code doesn't have to be deterministic, but it is still nice
>> if it's deterministic. I'll make a change to use SetVector.
>>
>
> I'm pretty sure in other cases across the LLVM project determinism of
> output, even for diagnostics, is considered necessary. (usually this comes
> up simply enough in test cases - if diagnostic order isn't deterministic,
> test cases fail on one platform and succeed on another, or fail randomly
> depending on hash of memory layout, etc)
>

Thanks. I didn't intend to create a lld-local rule here. If that's the case
in LLVM, we should follow that.


>
>>
>> https://reviews.llvm.org/D43254
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180221/456822d1/attachment.html>