All done.
https://codereview.chromium.org/15504002/diff/2001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/15504002/diff/2001/src/objects.cc#newcode1993
src/objects.cc:1993: uint32_t index,
On 2013/05/20 23:13:13, adamk wrote:
Nit: indentation seems off here.
Done.
https://codereview.chromium.org/15504002/diff/2001/src/objects.cc#newcode1999
src/objects.cc:1999: if (object->IsJSGlobalObject()) {
Replaced with assert.
On 2013/05/20 23:13:13, adamk wrote:
Can this even happen? Do we ever emit splices for non-arrays?
https://codereview.chromium.org/15504002/diff/2001/src/objects.cc#newcode2004
src/objects.cc:2004: Handle<Object> delete_count_object =
You are correct. This is an artifact of having split the original
(large) patch into smaller ones. The receiver side of
EnqueueSpliceRecord has already landed, and there are two patches in
flight that depend on that signature. I'll follow-up with a clean-up and
remove the delete_count argument.
On 2013/05/20 23:13:13, adamk wrote:
It's not obvious why we need the delete_count here...why don't we just
pass the
correct deleted array?
https://codereview.chromium.org/15504002/diff/2001/src/objects.cc#newcode2022
src/objects.cc:2022: if (object->IsJSGlobalObject()) {
On 2013/05/20 23:13:13, adamk wrote:
Same question as above, can this happen?
Done.
https://codereview.chromium.org/15504002/diff/2001/src/objects.cc#newcode2038
src/objects.cc:2038: if (object->IsJSGlobalObject()) {
On 2013/05/20 23:13:13, adamk wrote:
ditto
Done.
https://codereview.chromium.org/15504002/diff/2001/src/objects.cc#newcode10811
src/objects.cc:10811: int_indices.Add(i);
On 2013/05/20 23:13:13, adamk wrote:
It seems odd to keep track of the indices twice. Any reason not to
store them
only as ints and then generate strings below when needed?
Done.
https://codereview.chromium.org/15504002/diff/2001/test/mjsunit/harmony/object-observe.js
File test/mjsunit/harmony/object-observe.js (right):
https://codereview.chromium.org/15504002/diff/2001/test/mjsunit/harmony/object-observe.js#newcode1040
test/mjsunit/harmony/object-observe.js:1040:
assertSame(spliceRecords.length, 1);
On 2013/05/20 23:13:13, adamk wrote:
Any reason not to use assertEquals() for everything except object
identity?
Done.
https://codereview.chromium.org/15504002/diff/2001/test/mjsunit/harmony/object-observe.js#newcode1050
test/mjsunit/harmony/object-observe.js:1050:
assertSame(splice.removed[499999900], 'hello');
On 2013/05/20 23:13:13, adamk wrote:
Probably also want to check splice.removed.length (which should be
999999900)
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.
