[llvm-commits] [patch] Add a MapVector class

Chandler Carruth chandlerc at google.com
Tue Sep 18 15:37:14 PDT 2012


I've not looked at the Clang side yet, but the LLVM side looks good.

+// This file implements a map that provider insertion order iteration. The
+// interface is purposefully minimal.

s/provider/provides/

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.


On Tue, Sep 18, 2012 at 3:05 PM, Rafael EspĂ­ndola <
rafael.espindola at gmail.com> wrote:

> >> 2) Store vector<pair<const KeyT, ValueT>> as the vector storage, and
> make
> >> the densemap not even contain the keys.
> >
> > ...
> >
> >> #2 is appealing in interesting ways. While slower than a traditional
> >> DenseMap, it would have none of the traditional limitations. The
> DenseMap
> >> could actually be DensMap<unsigned, unsigned> where the key is the
> hash_code
> >> converted to an unsigned, and the value is the index. It also provides a
> >> natural way to support the pair-based value_type interface points.
> However,
> >> it would have strictly worse locality than your solution or #1.
> >
> > This is interesting. We can pass a KeyInfoT that has an identity hash
> > to avoid hashing twice. I will try this as a plan B.
>
> Looking at pr13868 I found other cases where we had non-deterministic
> output because of iteration on the very same objects that caused the
> problem I noticed before, so I decided to try #2.
>
> We cannot just use a DenseMap<unsigned, unsigned> or a hash collision
> would cause us to conclude that a key was already in the Map.
>
> An important observation is that in cases where we want to use this
> class the key is usually a simple object like a pointer. Given that I
> propose we just use a DenseMap<KeyT, unsigned> and a
> std::vector<std::pair<KeyT, ValueT> > and let users that need a more
> compact representation refactor/design something ad-hoc. At least this
> class becomes really hard to misuse.
>
> These patches should fix the original problem and pr13868.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120918/802f1cf1/attachment.html>


More information about the llvm-commits mailing list