Summary: | Array.prototype.find and findIndex should skip holes | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | JavaScriptCore | Assignee: | Antoine Quint <graouts> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, dbates, ggaren, graouts, joepeck, oliver | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 145361 | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2014-05-07 12:31:58 PDT
Is it a problem to expose Array.prototype.find() with this bug? I winder if it should be disabled until this is fixed. Yes, let's disable it. I'll look into this tomorrow! (In reply to comment #0) > From: <https://webkit.org/b/130966> Implement Array.prototype.find() > > > Source/JavaScriptCore/builtins/Array.prototype.js:210 > > + for (var i = 0; i < length; i++) { > > + if (callback.@call(thisArg, array[i], i, array)) > > + return array[i]; > > + } > > Should we only call the callback if "i in array" for sparse arrays? > <http://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.find> I really think all that is needed is: if (!(i in array)) continue; I just haven't had the time to try it. (In reply to comment #4) > (In reply to comment #0) > > From: <https://webkit.org/b/130966> Implement Array.prototype.find() > > > > > Source/JavaScriptCore/builtins/Array.prototype.js:210 > > > + for (var i = 0; i < length; i++) { > > > + if (callback.@call(thisArg, array[i], i, array)) > > > + return array[i]; > > > + } > > > > Should we only call the callback if "i in array" for sparse arrays? > > <http://people.mozilla.org/~jorendorff/es6-draft.html#sec-array.prototype.find> > > I really think all that is needed is: > > if (!(i in array)) > continue; > > I just haven't had the time to try it. That's all the needed -- the code should essentially match all the other array methods That, and a test, I'll have a patch out within 24 hours. For .find(), I was thinking of adding a test where we count the number of callbacks to check that we indeed only call the callback where there are entries. (In reply to comment #7) > For .find(), I was thinking of adding a test where we count the number of callbacks to check that we indeed only call the callback where there are entries. Look at the tests for other array prototype functions - you want to check the number and order of callbacks Created attachment 231819 [details]
Patch
Comment on attachment 231819 [details]
Patch
r=me
Comment on attachment 231819 [details] Patch Clearing flags on attachment: 231819 Committed r169162: <http://trac.webkit.org/changeset/169162> All reviewed patches have been landed. Closing bug. |