Summary: | ES7: Implement Array.prototype.includes | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dean Jackson <dino> | ||||||||||
Component: | New Bugs | Assignee: | Dean Jackson <dino> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | buildbot, rniwa, webkit-bug-importer, windrakerltd | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Dean Jackson
2015-03-14 21:39:10 PDT
Created attachment 248667 [details]
Patch
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 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
Created attachment 248672 [details]
Patch
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. Created attachment 249208 [details]
Patch
Comment on attachment 249208 [details]
Patch
r=me
Committed r181871: <http://trac.webkit.org/changeset/181871> |