[webkit-reviews] review requested: [Bug 21818] Should be able to use AtomicString as the key for a HashMap and HashSet : [Attachment 24633] Patch adding assert back

[bugzilla-daemon at webkit.org]

Simon Fraser <simon.fraser at apple.com> has asked Darin Adler <darin at apple.com>
for review:
Bug 21818: Should be able to use AtomicString as the key for a HashMap and
HashSet
https://bugs.webkit.org/show_bug.cgi?id=21818

Attachment 24633: Patch adding assert back
https://bugs.webkit.org/attachment.cgi?id=24633&action=edit

------- Additional Comments from Simon Fraser <simon.fraser at apple.com>
So the assert in existingHash() does fire in one particular case: when the
empty string is used as a hash key. The reason is that AtomicString::add(const
char* c) doesn't actually add the StringImpl::empty() to the HashSet:
StringImpl::empty() returns a singleton, so it's guaranteed to self-compare.
Because of this, hash() may never be called on this instance.

So in order to avoid the assertion in existingHash(), I force the hash to be
computed on the StringImpl returned by StringImpl::empty() in
AtomicString::add(). Is this the best place to do this, or should I do it in
the StringImpl() ctor, or in the emptyString() method?