Handle Object.observe notifications for setting Array.length

Handle Object.observe notifications for setting Array.length

Also handles notification of deleted properties when an array
is truncated by setting length.

Committed: https://code.google.com/p/v8/source/detail?r=12905

[adamk, 2012-10-31 09:43:59]

Just sending out so folks can take a look, this will likely be rolled up in
Rafael's patch as he writes tests for arrays.

[adamk, 2012-11-08 14:09:36]

Ready for review.

[rossberg, 2012-11-08 14:26:22]

LGTM with nits.

https://chromiumcodereview.appspot.com/11338048/diff/7001/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/11338048/diff/7001/src/accessors.cc#ne...
src/accessors.cc:109: while (end_index && end_index-- > new_length) {
Can we make this a for loop?

https://chromiumcodereview.appspot.com/11338048/diff/7001/src/accessors.cc#ne...
src/accessors.cc:121: 
Nit: move the empty line down by 2.

https://chromiumcodereview.appspot.com/11338048/diff/7001/test/mjsunit/harmon...
File test/mjsunit/harmony/object-observe.js (right):

https://chromiumcodereview.appspot.com/11338048/diff/7001/test/mjsunit/harmon...
test/mjsunit/harmony/object-observe.js:340: Object.defineProperty(arr, '2',
{get: function(){}});
Perhaps add the case of a non-configurable accessor.

[adamk, 2012-11-08 14:48:01]

https://chromiumcodereview.appspot.com/11338048/diff/7001/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/11338048/diff/7001/src/accessors.cc#ne...
src/accessors.cc:109: while (end_index && end_index-- > new_length) {
Done, but I'm not sure it looks better (have to deal with unsigned math). Let me
know what you think.

https://chromiumcodereview.appspot.com/11338048/diff/7001/src/accessors.cc#ne...
src/accessors.cc:121: 
On 2012/11/08 14:26:22, rossberg wrote:
> Nit: move the empty line down by 2.

Done.

https://chromiumcodereview.appspot.com/11338048/diff/7001/test/mjsunit/harmon...
File test/mjsunit/harmony/object-observe.js (right):

https://chromiumcodereview.appspot.com/11338048/diff/7001/test/mjsunit/harmon...
test/mjsunit/harmony/object-observe.js:340: Object.defineProperty(arr, '2',
{get: function(){}});
On 2012/11/08 14:26:22, rossberg wrote:
> Perhaps add the case of a non-configurable accessor.

Done.

[rossberg, 2012-11-08 14:56:28]

https://chromiumcodereview.appspot.com/11338048/diff/10002/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/11338048/diff/10002/src/accessors.cc#n...
src/accessors.cc:108: for (uint32_t len = old_length; len > new_length; --len) {
If you want to avoid the off-by-one nuisance below, I think you could make this

for (uint32_t len = old_length-1; len+1 > new_length; --len)

[adamk, 2012-11-08 15:11:22]

https://chromiumcodereview.appspot.com/11338048/diff/10002/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/11338048/diff/10002/src/accessors.cc#n...
src/accessors.cc:108: for (uint32_t len = old_length; len > new_length; --len) {
On 2012/11/08 14:56:28, rossberg wrote:
> If you want to avoid the off-by-one nuisance below, I think you could make
this
> 
> for (uint32_t len = old_length-1; len+1 > new_length; --len)

Nope: first of all, need to add an if statement now to check that old_length >
0. And even then, this won't terminate if new_length is 0. The last time
through, len will be 0, so len + 1 is 1. Then on the last iteration, --len is
2^32.

[rossberg, 2012-11-08 15:16:12]

https://chromiumcodereview.appspot.com/11338048/diff/10002/src/accessors.cc
File src/accessors.cc (right):

https://chromiumcodereview.appspot.com/11338048/diff/10002/src/accessors.cc#n...
src/accessors.cc:108: for (uint32_t len = old_length; len > new_length; --len) {
On 2012/11/08 15:11:23, adamk wrote:
> On 2012/11/08 14:56:28, rossberg wrote:
> > If you want to avoid the off-by-one nuisance below, I think you could make
> this
> > 
> > for (uint32_t len = old_length-1; len+1 > new_length; --len)
> 
> Nope: first of all, need to add an if statement now to check that old_length >
> 0. And even then, this won't terminate if new_length is 0. The last time
> through, len will be 0, so len + 1 is 1. Then on the last iteration, --len is
> 2^32.

Note the +1. ;)

[adamk, 2012-11-08 15:18:24]

On 2012/11/08 15:11:22, adamk wrote:
> https://chromiumcodereview.appspot.com/11338048/diff/10002/src/accessors.cc
> File src/accessors.cc (right):
> 
>
https://chromiumcodereview.appspot.com/11338048/diff/10002/src/accessors.cc#n...
> src/accessors.cc:108: for (uint32_t len = old_length; len > new_length; --len)
{
> On 2012/11/08 14:56:28, rossberg wrote:
> > If you want to avoid the off-by-one nuisance below, I think you could make
> this
> > 
> > for (uint32_t len = old_length-1; len+1 > new_length; --len)
> 
> Nope: first of all, need to add an if statement now to check that old_length >
> 0. And even then, this won't terminate if new_length is 0. The last time
> through, len will be 0, so len + 1 is 1. Then on the last iteration, --len is
> 2^32.

Er, nevermind. Updated to match.

[adamk, 2012-11-08 15:37:05]

Added a real truncation test case and added TODOs for arrays with dictionary
elements.

[rossberg, 2012-11-08 15:59:34]

LGTM again, gonna land it.