I've not looked at the Clang side yet, but the LLVM side looks good.<div><br></div><div><div>+// This file implements a map that provider insertion order iteration. The</div><div>+// interface is purposefully minimal.</div>
</div><div><br></div><div>s/provider/provides/</div><div><br></div><div>In general, could you add some more details about how this is achieved? I would be very explicit (and cautioning) to users that the key will be duplicated in this way.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Sep 18, 2012 at 3:05 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">>> 2) Store vector<pair<const KeyT, ValueT>> as the vector storage, and make<br>
>> the densemap not even contain the keys.<br>
><br>
> ...<br>
><br>
>> #2 is appealing in interesting ways. While slower than a traditional<br>
>> DenseMap, it would have none of the traditional limitations. The DenseMap<br>
>> could actually be DensMap<unsigned, unsigned> where the key is the hash_code<br>
>> converted to an unsigned, and the value is the index. It also provides a<br>
>> natural way to support the pair-based value_type interface points. However,<br>
>> it would have strictly worse locality than your solution or #1.<br>
><br>
> This is interesting. We can pass a KeyInfoT that has an identity hash<br>
> to avoid hashing twice. I will try this as a plan B.<br>
<br>
</div>Looking at pr13868 I found other cases where we had non-deterministic<br>
output because of iteration on the very same objects that caused the<br>
problem I noticed before, so I decided to try #2.<br>
<br>
We cannot just use a DenseMap<unsigned, unsigned> or a hash collision<br>
would cause us to conclude that a key was already in the Map.<br>
<br>
An important observation is that in cases where we want to use this<br>
class the key is usually a simple object like a pointer. Given that I<br>
propose we just use a DenseMap<KeyT, unsigned> and a<br>
std::vector<std::pair<KeyT, ValueT> > and let users that need a more<br>
compact representation refactor/design something ad-hoc. At least this<br>
class becomes really hard to misuse.<br>
<br>
These patches should fix the original problem and pr13868.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div>