| Summary: | AsyncFromSyncIterator methods should not pass absent values | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alexey Shvayka <ashvayka> | ||||||
| Component: | JavaScriptCore | Assignee: | Alexey Shvayka <ashvayka> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Minor | CC: | ews-watchlist, joepeck, keith_miller, mark.lam, msaboff, ross.kirsling, saam, tzagallo, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Alexey Shvayka
2020-04-28 14:39:46 PDT
Created attachment 397887 [details]
Patch
Created attachment 397888 [details]
Patch
Use ternary + @call instead of @apply.
Comment on attachment 397887 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397887&action=review > Source/JavaScriptCore/builtins/AsyncFromSyncIteratorPrototype.js:41 > - var nextResult = @getByIdDirectPrivate(this, "nextMethod").@call(syncIterator, value); > + var nextResult = @getByIdDirectPrivate(this, "nextMethod").@apply(syncIterator, arguments); This gets the job done, but the idea is just that `value` is optional, right? Using `arguments` made me think that we're expecting any number of values, before I rechecked the PR. Oops, I commented on the old patch...will mark obsolete. Comment on attachment 397888 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397888&action=review > Source/JavaScriptCore/builtins/AsyncFromSyncIteratorPrototype.js:42 > + var nextResult = @argumentCount() === 0 ? nextMethod.@call(syncIterator) : nextMethod.@call(syncIterator, value); Seems good as long as we're meant to distinguish explicit `undefined`. (In reply to Ross Kirsling from comment #5) Thanks for taking a look, Ross. > Seems good as long as we're meant to distinguish explicit `undefined`. We are: the spec PR uses "is present" wording. There are a few test262 cases failing if we do `=== @undefined`, but not as many as I would like. A few more tests are coming in https://github.com/tc39/test262/pull/2604. Committed r260915: <https://trac.webkit.org/changeset/260915> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397888 [details]. |