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:
On 2013/05/24 00:15:30, adamk wrote:
v8 style nit: need two blank lines between functions.
Done.
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode1996
src/objects.cc:1996: uint32_t delete_count,
I've put a todo to go back and remove it. There are now three CLs
pending that aren't dependent on each other. I prefer to land them and
then go back and clean this up.
On 2013/05/24 00:15:30, adamk wrote:
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,
On 2013/05/24 00:15:30, adamk wrote:
I'd use ARRAY_SIZE(args) here, and below.
Done.
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode2015
src/objects.cc:2015:
On 2013/05/24 00:15:30, adamk wrote:
same whitespace nit, and so on below
Done.
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode11010
src/objects.cc:11010: return *hresult;
On 2013/05/24 00:15:30, adamk wrote:
v8 style puts this on the previous line
Done.
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;
On 2013/05/24 00:15:30, adamk wrote:
this would be slightly more readable as min(old_length, new_length)
Done.
https://codereview.chromium.org/15504002/diff/16001/src/objects.cc#newcode11030
src/objects.cc:11030: for (int i = 0; i < indices.length(); ++i)
On 2013/05/24 00:15:30, adamk wrote:
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.
Done.
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,
On 2013/05/24 00:15:30, adamk wrote:
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.
Done.
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.
