https://codereview.chromium.org/15504002/diff/16001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode1992
src/objects.cc:1992:
v8 style nit: need two blank lines between functions.
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode1996
src/objects.cc:1996: uint32_t delete_count,
I know we discussed this before, but can you avoid passing delete_count
around? I don't think you even need to set length explicitly, since
you're going through SetElement() anyway (which will update the length
appropriately).
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode2011
src/objects.cc:2011: isolate->factory()->undefined_value(), 5, args,
I'd use ARRAY_SIZE(args) here, and below.
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode2015
src/objects.cc:2015:
same whitespace nit, and so on below
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode11010
src/objects.cc:11010: return *hresult;
v8 style puts this on the previous line
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode11025
src/objects.cc:11025: uint32_t index = new_length > old_length ?
old_length : new_length;
this would be slightly more readable as min(old_length, new_length)
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode11030
src/objects.cc:11030: for (int i = 0; i < indices.length(); ++i)
Note that these indices are in descending order due to the way they were
generated. So you might be better off iterating back-to-front.
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode12094
src/objects.cc:12094:
CHECK(new_length_handle->ToArrayIndex(&new_length));
We should really find a better way to avoid dropping CHECKs everywhere.
Not in this patch, though.
https://codereview.chromium.org/15504002/diff/16001/src/objects.h
File src/objects.h (right):
https://codereview.chromium.org/15504002/diff/16001/src/objects.h#newcode2403
src/objects.h:2403: static void EnqueueSpliceRecord(Handle<JSArray>
object,
Is there any need to expose these as part of the JSObject interface? If
they're only used in objects.cc, they could just be free file-scoped
functions instead. I mention it because now that they take JSArrays,
they seem a bit odd in JSObject.
https://codereview.chromium.org/15504002/
--
--
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.
