[webkit-reviews] review granted: [Bug 5332] Make HashMap/HashSet support non-POD types : [Attachment 5211] a real fix, that I actually tested and know works

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Wed Dec 21 20:24:03 PST 2005


Darin Adler <darin at apple.com> has granted Maciej Stachowiak <mjs at apple.com>'s
request for review:
Bug 5332: Make HashMap/HashSet support non-POD types
http://bugzilla.opendarwin.org/show_bug.cgi?id=5332

Attachment 5211: a real fix, that I actually tested and know works
http://bugzilla.opendarwin.org/attachment.cgi?id=5211&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
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



More information about the webkit-reviews mailing list