Add new Harmony methods to Array.prototype object.

Add new Harmony methods to Array.prototype object.

Array.prototype.find
Array.prototype.findIndex

http://people.mozilla.org/~jorendorff/es6-draft.html

BUG=v8:2776, v8:2777
TEST=mjsunit/harmony/array-find,mjsunit/harmony/array-findindex
R=mstarzinger@chromium.org

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

[Michael Starzinger, 2013-08-01 14:55:45]

I'll take a look at this one.

[ioseb, 2013-08-01 15:40:28]

On 2013/08/01 14:55:45, Michael Starzinger wrote:
> I'll take a look at this one.

Thanks!

[Michael Starzinger, 2013-08-01 16:21:02]

First round of comments.

https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js
File src/harmony-array.js (right):

https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js#newco...
src/harmony-array.js:48: var length = ToUint32(array.length);
The access to "array.length" is observable and needs to happen before checking
whether "predicate" is callable, let's move lines 47 and 48 up to before the
check.

Also I am not sure if that's an oversight in the spec draft or not, but the
draft currently specifies ToInteger to be used on the array length.

https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js#newco...
src/harmony-array.js:56: if (%HasProperty(array, key)) {
Better use the "in" keyword here, that's more efficient and doesn't require a
preceding %ToName() call either.

https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js#newco...
src/harmony-array.js:58: if (%_CallFunction(thisArg, element, i, array,
predicate)) {
When using the optimized %_CallFunction intrinsic we need to make sure that the
receiver is correct. If you take a look at some other Array functions (e.g.
ArraySome) you can see how that is done. In general you can these functions as a
good template as they have been thoroughly tested.

https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js#newco...
src/harmony-array.js:69: function ArrayFindIndex(predicate /* thisArg */) {  //
length == 1
Most comments from above apply to this function as well.

https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
File test/mjsunit/harmony/array-find.js (right):

https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
test/mjsunit/harmony/array-find.js:281: 
nit: Empty line at end of file, ./tools/presubmit.py will complain.

https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
File test/mjsunit/harmony/array-findindex.js (right):

https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
test/mjsunit/harmony/array-findindex.js:1: // Copyright 2013 the V8 project
authors. All rights reserved.
nit: Too much white-space.

https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
test/mjsunit/harmony/array-findindex.js:281: 
nit: Empty line at end of file, ./tools/presubmit.py will complain.

[ioseb, 2013-08-01 19:43:00]

All done.

On 2013/08/01 16:21:02, Michael Starzinger wrote:
> First round of comments.
> 
> https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js
> File src/harmony-array.js (right):
> 
>
https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js#newco...
> src/harmony-array.js:48: var length = ToUint32(array.length);
> The access to "array.length" is observable and needs to happen before checking
> whether "predicate" is callable, let's move lines 47 and 48 up to before the
> check.
> 
> Also I am not sure if that's an oversight in the spec draft or not, but the
> draft currently specifies ToInteger to be used on the array length.
> 
>
https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js#newco...
> src/harmony-array.js:56: if (%HasProperty(array, key)) {
> Better use the "in" keyword here, that's more efficient and doesn't require a
> preceding %ToName() call either.
> 
>
https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js#newco...
> src/harmony-array.js:58: if (%_CallFunction(thisArg, element, i, array,
> predicate)) {
> When using the optimized %_CallFunction intrinsic we need to make sure that
the
> receiver is correct. If you take a look at some other Array functions (e.g.
> ArraySome) you can see how that is done. In general you can these functions as
a
> good template as they have been thoroughly tested.
> 
>
https://codereview.chromium.org/21079003/diff/2007/src/harmony-array.js#newco...
> src/harmony-array.js:69: function ArrayFindIndex(predicate /* thisArg */) { 
//
> length == 1
> Most comments from above apply to this function as well.
> 
>
https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
> File test/mjsunit/harmony/array-find.js (right):
> 
>
https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
> test/mjsunit/harmony/array-find.js:281: 
> nit: Empty line at end of file, ./tools/presubmit.py will complain.
> 
>
https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
> File test/mjsunit/harmony/array-findindex.js (right):
> 
>
https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
> test/mjsunit/harmony/array-findindex.js:1: // Copyright 2013 the V8 project
> authors. All rights reserved.
> nit: Too much white-space.
> 
>
https://codereview.chromium.org/21079003/diff/2007/test/mjsunit/harmony/array...
> test/mjsunit/harmony/array-findindex.js:281: 
> nit: Empty line at end of file, ./tools/presubmit.py will complain.

[Michael Starzinger, 2013-08-01 20:01:24]

LGTM. I'll land this tomorrow when I am back in the office.

[ioseb, 2013-08-01 20:01:54]

On 2013/08/01 20:01:24, Michael Starzinger wrote:
> LGTM. I'll land this tomorrow when I am back in the office.

Thank you!

[Michael Starzinger, 2013-08-02 10:57:56]

Committed patchset #3 manually as r16025 (presubmit successful).