Bug 142707 – ES7: Implement Array.prototype.includes

• Component: New Bugs
• Status: RESOLVED FIXED

[Dean Jackson]

ES7: Implement Array.prototype.includes

[Radar WebKit Bug Importer]

<rdar://problem/20165824>

[Dean Jackson]

Created attachment 248667 [details]
Patch

[Build Bot]

Comment on attachment 248667 [details]
Patch

Attachment 248667 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6016985234145280

New failing tests:
js/Object-getOwnPropertyNames.html

[Build Bot]

Created attachment 248671 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Dean Jackson]

Created attachment 248672 [details]
Patch

[Geoffrey Garen]

Comment on attachment 248672 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=248672&action=review

r- because I found a missing exception check.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1101
> +EncodedJSValue JSC_HOST_CALL arrayProtoFuncIncludes(ExecState* exec)

Are you scarred enough from writing of in JavaScript that you chose C++ for this? :)

Seriously, though, JS might be better.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1104
> +    JSObject* thisObj = exec->thisValue().toThis(exec, StrictMode).toObject(exec);

I think that toObject can throw, in which case getLength below will has a sad. So, you need to check for exception before getLength and after. Please add a test for this. I think all you need is to pass in undefined or null as 'this'.

This is the kind of thing that is easier to get right if you write it in JS.

> Source/JavaScriptCore/runtime/ArrayPrototype.cpp:1112
> +        JSValue e = getProperty(exec, thisObj, index);

It's a bit of a shame that this access is always virtual, with no fast path for arrays. This kind of thing sometimes goes faster in the JIT, since it will specialize for arrays if that's what the website uses.

[Dean Jackson]

Created attachment 249208 [details]
Patch

[Geoffrey Garen]

Comment on attachment 249208 [details]
Patch

r=me

[Dean Jackson]

Committed r181871: <http://trac.webkit.org/changeset/181871>