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

Draft of spec for jspi #37

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Draft of spec for jspi #37

wants to merge 1 commit into from

Conversation

fgmccabe
Copy link
Collaborator

This is a draft of a specification for jspi.

document/js-api/index.bs Outdated Show resolved Hide resolved
document/js-api/index.bs Outdated Show resolved Hide resolved
document/js-api/index.bs Show resolved Hide resolved
document/js-api/index.bs Outdated Show resolved Hide resolved
document/js-api/index.bs Outdated Show resolved Hide resolved
1. If [=list/size=] of |ret| is 1 and [$IsPromise$](|ret|.\[[Value]][0]):
1. Let |promise| be |ret|.\[[Value]][0].
1. Suspend |async_context|, removing it from the ambient [=Execution Context Stack=].
1. If |async_context| is not performing the steps in [=run a Promising function=] then trigger a WebAssembly trap.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedent for this kind of introspection on performed steps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is based on a similar operation in ecmascript: https://tc39.es/ecma262/#await

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know this precedent. Which step are you referring to in Await?

document/js-api/index.bs Outdated Show resolved Hide resolved
document/js-api/index.bs Outdated Show resolved Hide resolved
1. Let |rejected| be an [=AbstractClosure=] with parameters (|e|) that captures |async_context| and performs the following steps when called:
1. Converts |e| to the WebAssembly exception |we|.
1. Push |async_context| on to the [=ExecutionContext=] stack.
1. Resume the suspended evaluation of asyncContext using throw |we| as the result of the operation that suspended it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. Resume the suspended evaluation of asyncContext using throw |we| as the result of the operation that suspended it.
1. Resume the suspended evaluation of |async_context| using throw |we| as the result of the operation that suspended it.

Is this the correct behavior in the presence of WebAssembly exception handling? I would expect that when EH is available, a rejected promise would resume the Wasm with a thrown JS exception, not just skip the Wasm completely.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Plan to use language similar to that in EH js API text.

document/js-api/index.bs Outdated Show resolved Hide resolved
1. If [=list/size=] of |ret| is 1 and [$IsPromise$](|ret|.\[[Value]][0]):
1. Let |promise| be |ret|.\[[Value]][0].
1. Suspend |async_context|, removing it from the ambient [=Execution Context Stack=].
1. If |async_context| is not performing the steps in [=run a Promising function=] then trigger a WebAssembly trap.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know this precedent. Which step are you referring to in Await?

1. If |ret|.\[[Type]] is <emu-const>throw</emu-const>, then trigger a WebAssembly trap, and propagate |ret|.\[[Value]] to the enclosing JavaScript.
1. If [=list/size=] of |ret| is 1 and [$IsPromise$](|ret|.\[[Value]][0]):
1. Let |promise| be |ret|.\[[Value]][0].
1. Suspend |async_context|, removing it from the ambient [=Execution Context Stack=].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am deeply uncomfortable with this. I understand where @fgmccabe is coming from, and that the powers of the Suspend verb is underspecified in 262 itself. But I am not comfortable with the heavy lifting it's doing that it's able to effectively pause any embedder spec of ecma262. The current usage of Suspend in ecma262 itself is only thinks about suspension of 262 steps. If it had general suspension powers, then it would also be able to suspend in the middle of e.g. web API functions. That is not an implementable thing today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants