https://codereview.chromium.org/15504002/diff/33001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/15504002/diff/33001/src/objects.cc#newcode11031
src/objects.cc:11031: uint32_t add_count = new_length > old_length ?
new_length - old_length : 0;
Max(new_length - old_length, 0)
https://codereview.chromium.org/15504002/diff/33001/src/objects.cc#newcode11032
src/objects.cc:11032: uint32_t delete_count = new_length < old_length ?
old_length - new_length : 0;
Max(old_length - new_length, 0)
https://codereview.chromium.org/15504002/diff/33001/src/objects.cc#newcode11034
src/objects.cc:11034: if (delete_count) {
Nit: delete_count > 0
https://codereview.chromium.org/15504002/diff/33001/src/objects.cc#newcode11036
src/objects.cc:11036: while (i--) {
Can we make this a for loop?
https://codereview.chromium.org/15504002/diff/33001/src/objects.cc#newcode12069
src/objects.cc:12069: Handle<Object> new_length_handle;
Move this and new_length into conditional below, to avoid additional
register pressure on ia32.
https://codereview.chromium.org/15504002/diff/33001/src/objects.cc#newcode12095
src/objects.cc:12095: bool array_extended = false;
Can we initialize this properly instead of mutating later, i.e.:
bool array_extended = self->IsJSArray() && ...;
But actually, I'd simplify control flow by making it one 'if/else' and
duplicate the single EnqueueChangeRecord call.
https://codereview.chromium.org/15504002/diff/33001/src/v8natives.js
File src/v8natives.js (right):
https://codereview.chromium.org/15504002/diff/33001/src/v8natives.js#newcode929
src/v8natives.js:929: new_length < old_length ? new_length : old_length,
Perhaps use Math.min
https://codereview.chromium.org/15504002/diff/33001/src/v8natives.js#newcode932
src/v8natives.js:932: new_length > old_length ? new_length - old_length
: 0);
Perhaps use Math.max
https://codereview.chromium.org/15504002/diff/33001/test/mjsunit/harmony/object-observe.js
File test/mjsunit/harmony/object-observe.js (right):
https://codereview.chromium.org/15504002/diff/33001/test/mjsunit/harmony/object-observe.js#newcode1005
test/mjsunit/harmony/object-observe.js:1005: { object: arr3, name:
'length', type: 'reconfigured' }
Not sure I understand, why is it correct that this a separate change
now?
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.
