Implementing __defineSetter__ and __defineGetter__ with an array index as arg...

Implementing __defineSetter__ and __defineGetter__ with an array index as argument as described in issue 242. For performance reasons, the change is not applied to arrays. This conforms to the behavior of Safari.

Committed: http://code.google.com/p/v8/source/detail?r=1507

[Mads Ager (chromium), 2009-03-13 10:27:14]

LGTM

http://codereview.chromium.org/46030/diff/1/2
File src/objects.cc (right):

http://codereview.chromium.org/46030/diff/1/2#newcode2602
Line 2602: // Update the array length if this JSObject is an array.
Since we do not do this for arrays, we can remove this code.

http://codereview.chromium.org/46030/diff/1/2#newcode5356
Line 5356: if (structure->get(kSetterIndex)->IsJSFunction()) {
I believe that we throw exceptions when setting named properties that have only
a getter.  Should we do the same here instead of falling through?

http://codereview.chromium.org/46030/diff/1/3
File src/objects.h (right):

http://codereview.chromium.org/46030/diff/1/3#newcode1998
Line 1998: // Set and existing entry or add a new one if needed.
I know this is not your comment, but could you fix:

and -> an

http://codereview.chromium.org/46030/diff/1/3#newcode2028
Line 2028: // kRequiresSlowElementsLimit or set_requires_slow_elements() has
been called.
Maybe state the real reason here: if a getter or setter with a number key has
been defined?

http://codereview.chromium.org/46030/diff/1/5
File test/mjsunit/indexed-accessors.js (right):

http://codereview.chromium.org/46030/diff/1/5#newcode28
Line 28: // Test that getters can be defined and called with an index as a
parameter.
getters -> getters and setters

[olehougaard, 2009-03-13 11:39:46]

http://codereview.chromium.org/46030/diff/1/2
File src/objects.cc (right):

http://codereview.chromium.org/46030/diff/1/2#newcode2602
Line 2602: // Update the array length if this JSObject is an array.
On 2009/03/13 10:27:14, Mads Ager wrote:
> Since we do not do this for arrays, we can remove this code.

Fixed.

http://codereview.chromium.org/46030/diff/1/2#newcode5356
Line 5356: if (structure->get(kSetterIndex)->IsJSFunction()) {
On 2009/03/13 10:27:14, Mads Ager wrote:
> I believe that we throw exceptions when setting named properties that have
only
> a getter.  Should we do the same here instead of falling through?

That's true. I've fixed it and added a test for it.

http://codereview.chromium.org/46030/diff/1/3
File src/objects.h (right):

http://codereview.chromium.org/46030/diff/1/3#newcode1998
Line 1998: // Set and existing entry or add a new one if needed.
On 2009/03/13 10:27:14, Mads Ager wrote:
> I know this is not your comment, but could you fix:
> 
> and -> an

Fixed.

http://codereview.chromium.org/46030/diff/1/3#newcode2028
Line 2028: // kRequiresSlowElementsLimit or set_requires_slow_elements() has
been called.
On 2009/03/13 10:27:14, Mads Ager wrote:
> Maybe state the real reason here: if a getter or setter with a number key has
> been defined?

Fixed.