Bug 211147

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:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

All

OS:

All

Attachments:

Description	Flags
Patch	none
Patch	none

Alexey Shvayka

Reported 2020-04-28 14:39:46 PDT

AsyncFromSyncIterator methods should not pass absent values

Attachments
Patch (4.93 KB, patch) 2020-04-28 14:51 PDT, Alexey Shvayka	no flags	Details Formatted Diff Diff
Patch (4.96 KB, patch) 2020-04-28 15:06 PDT, Alexey Shvayka	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Alexey Shvayka

Comment 1 2020-04-28 14:51:49 PDT

Created attachment 397887 [details] Patch

Alexey Shvayka

Comment 2 2020-04-28 15:06:25 PDT

Created attachment 397888 [details] Patch Use ternary + @call instead of @apply.

Ross Kirsling

Comment 3 2020-04-28 15:24:18 PDT Comment hidden (obsolete)

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.

Ross Kirsling

Comment 4 2020-04-28 15:25:03 PDT Comment hidden (obsolete)

Oops, I commented on the old patch...will mark obsolete.

Ross Kirsling

Comment 5 2020-04-28 16:10:06 PDT

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`.

Alexey Shvayka

Comment 6 2020-04-29 13:20:09 PDT

(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.

EWS

Comment 7 2020-04-29 14:39:45 PDT

Committed r260915: <https://trac.webkit.org/changeset/260915> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397888 [details].

Radar WebKit Bug Importer

Comment 8 2020-04-29 14:40:19 PDT

<rdar://problem/62618464>

Note You need to log in before you can comment on or make changes to this bug.