[llvm-commits] PATCH: Refactored hashing for constant unique map

[Chandler Carruth]

On Sun, Feb 5, 2012 at 4:43 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>wrote:

>
> On Feb 5, 2012, at 4:31 PM, Talin wrote:
>
> > Patch to refactor the hashing functions for the constant unique map, to
> avoid code duplication.
>
> Wouldn't it give nicer code if you kept the intermediate state as a class
> member?
>
> Something like:
>
> IncrementalHash Hash
> Hash.add(foo);
> Hahs.add(bar);
> return Hash.finish();


If you're interested in building nice, generalized hashing infrastructure
in Support, I think we can and should have a much better interface than
this or the one Talin is proposing.

I would look at the proposed standard interfaces for hashing that Jeffrey
and I are proposing for the next C++ standard. We could almost certainly
implement something very similar to it, although there are a few subtleties
we would need to work out surrounding name lookup.

Also, I don't really want such a weak hash function in Support. The
standard libraries actually have much higher quality hash functions now.

Finally, we already have some hashing  routines in the DenseSet
implementation. I feel this refactoring is only the tip of the ice berg,
and isn't really factored properly for use in the rest of the cases.

However, as Nick hinted, I think the best solution might be to change the
code Talin originally touched to use FoldingSetNodeID, which was carefully
designed for such situations it seems.... A generalized hash function often
isn't designed in that way.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120205/01465460/attachment.html>