LGTM, just a couple of nits.
https://codereview.chromium.org/48913008/diff/170002/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/48913008/diff/170002/src/objects.cc#newcode4802
src/objects.cc:4802: JSObject::SetHiddenProperty(
nit: The explicit name-space prefix "JSObject::" shouldn't be necessary
as we are already in the same class. Applies a couple of times below as
well.
https://codereview.chromium.org/48913008/diff/170002/src/objects.cc#newcode4808
src/objects.cc:4808: Handle<Object> hash =
JSObject::GetOrCreateIdentityHash(object);
On 2013/11/04 15:12:01, rafaelw wrote:
I *think* this is right. It's a bit hard for me to follow the
CALL_AND_RETRY_OR_DIE logic. Please verify.
AFAICT, the return value of JSObject::GetOrCreateIdentityHash should
always be a Smi. So it seems to me
JSObject::GetIdentityHash(Handle<JSObject>) is deprecated by now and the
calls to it can be replaced with JSObject::GetOrCreateIdentityHash()
instead.
I only see one call-site in api.cc and that one should actually used
JSReceiver::GetOrCreateIdentityHash() instead so that we can avoid
making JSObject::GetOrCreateIdentityHash() public.
https://codereview.chromium.org/48913008/diff/170002/src/objects.cc#newcode4962
src/objects.cc:4962: Object* inline_value;
nit: The local "inline_value" variable is (almost) not used, let's drop
it.
https://codereview.chromium.org/48913008/diff/170002/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/48913008/diff/170002/src/objects.h#newcode1507
src/objects.h:1507: MUST_USE_RESULT Object* GetHash();
nit: I think we can drop the MUST_USE_RESULT here, it's not necessarily
a bug if the return value is no used.
https://codereview.chromium.org/48913008/diff/170002/src/objects.h#newcode2007
src/objects.h:2007: inline MUST_USE_RESULT Object* GetIdentityHash();
nit: Likewise.
https://codereview.chromium.org/48913008/diff/170002/test/cctest/test-dictionary.cc
File test/cctest/test-dictionary.cc (right):
https://codereview.chromium.org/48913008/diff/170002/test/cctest/test-dictionary.cc#newcode218
test/cctest/test-dictionary.cc:218: // Calling Put() should request GC
by returning a failure.
nit: Let's move the comment up one line, easier to read.
https://codereview.chromium.org/48913008/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
