Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editorial: Handle abrupt completion in AsyncFromSyncIteratorContinuation #1470

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@MayaLekova
Copy link
Contributor

MayaLekova commented Mar 6, 2019

As agreed in the referenced issue, made sure AsyncFromSyncIteratorContinuation keeps its contract to return rejected promise on abrupt completion.

Fixes #1461.

@bakkot

This comment has been minimized.

Copy link
Contributor

bakkot commented Mar 6, 2019

A note: while this is editorial in the sense that the original spec was meaningless, the fact that there were previously at least two possible interpretations makes this a place where engines might diverge. As such, I think it would be good if we eventually produce a test262 test which exercised the number of ticks implied by this patch. (Probably not as a prerequisite for landing this, just at some point.)

@ljharb ljharb changed the title Editorial: Handle abrupt completion in AsyncFromSyncIteratorContinuation (#1461) Editorial: Handle abrupt completion in AsyncFromSyncIteratorContinuation Mar 6, 2019

@ljharb ljharb requested review from ljharb and tc39/ecma262-editors Mar 6, 2019

@ljharb

ljharb approved these changes Mar 6, 2019

@ljharb ljharb added the spec bug label Mar 6, 2019

@ljharb ljharb self-assigned this Mar 6, 2019

anba added a commit to anba/test262 that referenced this pull request Mar 11, 2019

@anba

This comment has been minimized.

Copy link
Collaborator

anba commented Mar 11, 2019

I think it would be good if we eventually produce a test262 test which exercised the number of ticks implied by this patch. (Probably not as a prerequisite for landing this, just at some point.)

Added as part of tc39/test262#2096.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Mar 13, 2019

Jason Orendorff
Bug 1534392 - Part 2: Fix error handling in %AsyncFromSyncIteratorPro…
…totype% methods. r=anba

This makes %AsyncFromSyncIteratorPrototype%.next/return/throw return a rejected
promise, not throw, when PromiseResolve throws, following the usual convention
for methods that return promises. This follows proposed spec change
<tc39/ecma262#1470>, which I expect will land with
little controversy.

Differential Revision: https://phabricator.services.mozilla.com/D23030

--HG--
extra : moz-landing-system : lando

mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Mar 13, 2019

Jason Orendorff
Bug 1534392 - Part 2: Fix error handling in %AsyncFromSyncIteratorPro…
…totype% methods. r=anba

This makes %AsyncFromSyncIteratorPrototype%.next/return/throw return a rejected
promise, not throw, when PromiseResolve throws, following the usual convention
for methods that return promises. This follows proposed spec change
<tc39/ecma262#1470>, which I expect will land with
little controversy.

Differential Revision: https://phabricator.services.mozilla.com/D23030

wardn pushed a commit to wardn/v8 that referenced this pull request Mar 19, 2019

[await] Update async iterators to return a rejected promise on error
This implements the behavior discussed and specified here:
tc39/ecma262#1461
tc39/ecma262#1470

As part of making this change, I realized that we didn't actually
toggle the behavior between the optimized and unoptimized version
based on the --harmony-await-optimization flag at all and just the
unoptimized version by default.

This patch removes the unoptimized version and uses the optimized
version as the default.

The other builtins that use this flag are not touched as part of this
CL, they will be updated separately.

Bug: v8:8998
Change-Id: I315e1b39dda91d0127b5e567986485d713eaa78d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1525872
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60310}
@littledan

This comment has been minimized.

Copy link
Member

littledan commented Mar 19, 2019

Now that this PR is editorial, has been approved by the editors, tests have been written, and it's been broadly reviewed online, is it ready to land? To me, it seems unnecessary to discuss this PR in committee at this point. Otherwise, what do you recommend as next steps?

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Mar 20, 2019

@littledan there's certainly no need to discuss an editorial PR in committee; however, to ensure that any issues that need to be backported into ES2019 land cleanly, we're not merging anything new for the time being. We'll address this during the editors' update in plenary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.