Added dictionary that can use objects as keys.

Added dictionary that can use objects as keys.

R=vegorov@chromium.org
TEST=cctest/test-dictionary

Committed: http://code.google.com/p/v8/source/detail?r=8619

[Michael Starzinger (Google), 2011-07-12 09:46:45]

This re-factors the GetIdentityHash implementation into the internal API and
adds a simplistic unit test for the new ObjectDictionary.

The only thing missing is for Dictionary::Add to check for an invalid identity
hash code and return a failure in that case. But I wanted to get an early review
to see if this is heading into the right direction.

[Vyacheslav Egorov (Chromium), 2011-07-12 10:42:07]

http://codereview.chromium.org/7349005/diff/1/src/objects-inl.h
File src/objects-inl.h (right):

http://codereview.chromium.org/7349005/diff/1/src/objects-inl.h#newcode4313
src/objects-inl.h:4313: if (maybe_hash->IsFailure()) return 0;
If we are trying to put an object to a dictionary and GC is required to create
it's hash then we'll put object into a dictionary as if it has 0 hash. Later
when will search for an object we might successfully create hash (!= 0) and will
not find an object that we put with hash == 0.

I recommend creating utility methods in ObjectDictionary that first guarantee
that key has a hash (or return failure) and then delegate to Dictionary methods.
Here you should assert that hash was assigned to the key.

http://codereview.chromium.org/7349005/diff/1/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7349005/diff/1/src/objects.cc#newcode2969
src/objects.cc:2969: CHECK(!result->IsFailure());
I think it can return a failure (adding local property might require an
allocation).

Please use

if (result->IsFailure()) return result; 

instead.

[Michael Starzinger (Google), 2011-07-12 11:50:19]

Thanks for the review. Incorporated suggested changes.

[Vyacheslav Egorov (Chromium), 2011-07-12 13:30:32]

LGTM

http://codereview.chromium.org/7349005/diff/4001/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7349005/diff/4001/src/objects.cc#newcode11535
src/objects.cc:11535: MaybeObject* ObjectDictionary::AddChecked(JSObject* key,
Object* value) {
AddChecked is a bit confusing name but I can't invent a better name.

HashKeyAndAdd might be better but it's still pretty confusing.

[Michael Starzinger (Google), 2011-07-12 15:05:06]

http://codereview.chromium.org/7349005/diff/4001/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7349005/diff/4001/src/objects.cc#newcode11535
src/objects.cc:11535: MaybeObject* ObjectDictionary::AddChecked(JSObject* key,
Object* value) {
Maybe we can come up with a better name once we actually use it aside from the
unit test.

On 2011/07/12 13:30:32, Vyacheslav Egorov wrote:
> AddChecked is a bit confusing name but I can't invent a better name.
> HashKeyAndAdd might be better but it's still pretty confusing.