Bug 5332 – Make HashMap/HashSet support non-POD types

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Maciej Stachowiak]

It would be really useful if HashMap and HashSet supported non-POD types, so that they could use 
SharedPtrs for instance as key and value types.

[Maciej Stachowiak]

Created attachment 4303 [details]
blind stab at it

JavaScriptCore and WebCore compile and work, but I did not performance test,
nor did I check that it works as advertised for any non-POD types, SharedPtr or
otherwise. For types besides SharedPtr you have to make your own HashTraits.

[Maciej Stachowiak]

Darin points out I should use swap. But I would need to figure out the right way to do swap generically 
while letting classes specialize it to avoid redundant ref/deref.

[Maciej Stachowiak]

Created attachment 5211 [details]
a real fix, that I actually tested and know works

Here's a real fix. I tested it and was careful to avoid gratuitous copying.

[Darin Adler]

Comment on attachment 5211 [details]
a real fix, that I actually tested and know works

I see some tabs in the ChangeLog.

There's a sentence fragment in the ChangeLog.

That space you removed from array_object.cpp is one I included intentionally.
When I have to add a space at the end of a template declaration, I like to add
a corresponding space at the start so it looks symmetrical. We could
standardize our handling of this -- I'd be happy to switch to your usage.

This line:

+	 ValueType tmp(key,mapped);

is missing a space after a comma.

In HashMap.h you sometimes say std::pair and other times just pair. You should
consistently use just pair.

It's a bit annoying that you asked someone on our team to fix the "is integer"
thing, and then I "coerced" Geoff into doing it this afternoon, and then you
removed the code that uses it!

I would have left out the swap member function for RefPtr, made the inline swap
function a friend, and had it use std::swap to swap the two m_ptr fields
instead of hand-writing a swap.

r=me

[Maciej Stachowiak]

I will fix the minor issues before landing but I think I may keep the swap() member function because it 
seems to be common practice to do it that way. I have locally merged my change with Geoff's isInteger fix 
which is great IMO.