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

Wiring of return value is underspecified #31

Closed
bakkot opened this issue Aug 11, 2022 · 2 comments · Fixed by #32
Closed

Wiring of return value is underspecified #31

bakkot opened this issue Aug 11, 2022 · 2 comments · Fixed by #32
Labels
question Further information is requested

Comments

@bakkot
Copy link
Contributor

bakkot commented Aug 11, 2022

fromAsync makes use of an Abstract Closure passed to AsyncFunctionStart. What is the return type of this closure? The use of ? implies it must be a Completion Record, but there's also a bare Return A. which returns a value not wrapped in a completion record. In normal built-in functions this would be implicitly wrapped in NormalCompletion by the rules in this clause, but a.) that doesn't apply within the Abstract Closure (rather, it's the thing which allows the final Return promiseCapability.[[Promise]] line to work and b.) that wouldn't do the right thing anyway, because the logic in AsyncBlockStart treats an invocation returning a normal completion as if it had explicitly returned undefined (which is how normal functions work) - an actual return would be a return completion.

I see three possible ways forward:

  1. in the closure within fromAsync, explicitly wrap each Return _value_. as Return Completion Record { [[Type]]: ~return~, [[Value]]: _value_, [[Target]]: empty }
  2. in AsyncFunctionStart, have a different path for Abstract Closures vs built-in functions
  3. define "built-in async function" rigorously in a way which would allow you to write the steps of a built-in function, but using Await

Of these I think the third option is cleanest, especially if we add more built-in async stuff (as the iterator helpers proposal will do).

@js-choi
Copy link
Collaborator

js-choi commented Aug 17, 2022

You mentioned that you generously might sketch the third approach in tc39/proposal-iterator-helpers#218. The proposal-iterator-helpers specification uses language such as “…is a built-in async function which, when called, performs the following steps…” But as you know, the current Array.fromAsync specification does not use that language; it instead uses an Abstract Closure passed to AsyncFunctionStart. So rigorously defining “built-in async functions” would help the proposal-iterator-helpers spec, but it would not help the current Array.fromAsync spec.

So is the third choice proposing that we rewrite the Array.fromAsync spec to use the same “…is a built-in async function which…” language as proposal-iterator-helpers? (See also #23.) But this still would require that we properly define “built-in async function”.

In the meantime, perhaps we could use the first choice as a stopgap for Array.fromAsync.

There are only two “Return A” lines in the current Array.fromAsync algorithm. We could easily replace them with “Return Completion Record { [[Type]]: ~return~, [[Value]]: A, [[Target]]: ~empty~ }”.

That may be good enough for Stage 3 (my goal for Stage 3 is the September plenary). And we could always switch its definition to use a “built-in async function” as an editorial change later, once tc39/proposal-iterator-helpers#218 gets resolved.

@bakkot
Copy link
Contributor Author

bakkot commented Aug 17, 2022

So is the third choice proposing that we rewrite the Array.fromAsync spec to use the same “…is a built-in async function which…” language as proposal-iterator-helpers? (See also #23.) But this still would require that we properly define “built-in async function”.

Yes, that was the suggestion. And yeah, it is ok to do that refactoring later.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants