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: do not use "Return" to transfer control between execution contexts #2429

Closed
wants to merge 2 commits into from

Conversation

bakkot
Copy link
Contributor

@bakkot bakkot commented Jun 6, 2021

Fixes #2400.

Depends on #2413. Also I'm not 100% sure this all makes sense, so marking as a draft for now.

TODO: ensure the Let steps are on their own line (if coherent).

spec.html Outdated
1. Set the code evaluation state of _asyncContext_ such that when evaluation is resumed with a Completion _completion_, the following steps of the algorithm that invoked Await will be performed, with _completion_ available.
1. Return.
1. NOTE: This returns to the evaluation of the operation that had most previously resumed evaluation of _asyncContext_.
1. Let _outerContext_ be the running execution context.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you name _outerContext in the previous step (the one where _asyncContext is removed from the stack) and thus skip this step?

spec.html Outdated
1. NOTE: This returns to the evaluation of the |YieldExpression| that originally called this abstract operation.
1. Return NormalCompletion(_iterNextObj_).
1. NOTE: This returns to the evaluation of the operation that had most previously resumed evaluation of _genContext_.
1. Let _outerContext_ be the running execution context.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, _outerContext_ could be named in the previous step.

spec.html Outdated
1. NOTE: When the above step returns, it returns to the evaluation of the |YieldExpression| production that originally called this abstract operation.
1. Return *undefined*.
1. NOTE: This returns to the evaluation of the operation that had most previously resumed evaluation of _genContext_.
1. Let _outerContext_ be the running execution context.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

spec.html Outdated
1. NOTE: _outerContext_ is the execution context for the evaluation of the operation that had most previously resumed evaluation of _asyncContext_.
1. Resume _outerContext_.
1. Assert: If control reaches here, _asyncContext_ is once again the running execution context.
1 Let _completion_ be the Completion Record with which _asyncContext_ was resumed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1 Let _completion_ be the Completion Record with which _asyncContext_ was resumed.
1. Let _completion_ be the Completion Record with which _asyncContext_ was resumed.

spec.html Outdated
1. Let _outerContext_ be the running execution context.
1. NOTE: _outerContext_ is the execution context for the evaluation of the operation that had most previously resumed evaluation of _asyncContext_.
1. Resume _outerContext_.
1. Assert: If control reaches here, _asyncContext_ is once again the running execution context.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. Assert: If control reaches here, _asyncContext_ is once again the running execution context.
1. Assert: If control reaches here, then _asyncContext_ is once again the running execution context.

spec.html Outdated
1. Let _outerContext_ be the running execution context.
1. NOTE: _outerContext_ is the execution context for the evaluation of the operation that had most previously resumed evaluation of _genContext_.
1. Resume _outerContext_ passing NormalCompletion(_iterNextObj_).
1. Assert: If control reaches here, _genContext_ is once again the running execution context.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. Assert: If control reaches here, _genContext_ is once again the running execution context.
1. Assert: If control reaches here, then _genContext_ is once again the running execution context.

spec.html Outdated
1. Let _outerContext_ be the running execution context.
1. NOTE: _outerContext_ is the execution context for the evaluation of the operation that had most previously resumed evaluation of _genContext_.
1. Resume _outerContext_.
1. Assert: If control reaches here, _genContext_ is once again the running execution context.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. Assert: If control reaches here, _genContext_ is once again the running execution context.
1. Assert: If control reaches here, then _genContext_ is once again the running execution context.

spec.html Outdated
1. Once a generator enters the ~completed~ state it never leaves it and its associated execution context is never resumed. Any execution state associated with _generator_ can be discarded at this point.
1. If _result_.[[Type]] is ~normal~, let _resultValue_ be *undefined*.
1. Else if _result_.[[Type]] is ~return~, let _resultValue_ be _result_.[[Value]].
1. NOTE: once a generator enters the ~completed~ state it never leaves it and its associated execution context is never resumed. Any execution state associated with _generator_ can be discarded at this point.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
1. NOTE: once a generator enters the ~completed~ state it never leaves it and its associated execution context is never resumed. Any execution state associated with _generator_ can be discarded at this point.
1. NOTE: Once a generator enters the ~completed~ state it never leaves it and its associated execution context is never resumed. Any execution state associated with _generator_ can be discarded at this point.

@bakkot bakkot force-pushed the rewrite-asyncgenerator-state-machine branch from d17e46d to 18100ca Compare July 4, 2021 23:11
@ljharb ljharb force-pushed the rewrite-asyncgenerator-state-machine branch from 18100ca to f469441 Compare July 7, 2021 23:45
Base automatically changed from rewrite-asyncgenerator-state-machine to master July 7, 2021 23:50
@bakkot bakkot force-pushed the execution-context-return branch 2 times, most recently from 525c4d7 to 6a7e4a6 Compare July 25, 2021 00:40
1. Return.
1. NOTE: This returns to the evaluation of the operation that had most previously resumed evaluation of _asyncContext_.
1. Let _outerContext_ be the running execution context.
1. NOTE: _outerContext_ is the execution context for the evaluation of the operation that had most previously resumed evaluation of _asyncContext_.
Copy link
Collaborator

@jmdyck jmdyck Feb 15, 2022

Choose a reason for hiding this comment

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

It's unclear what "most previously" is supposed to mean.

If it means "the longest ago", then that's wrong, because the execution context that first resumed evaluation of asyncContext isn't even necessarily on the stack, let alone in that position on the stack.

And if it means "most recently", then that got a problem too. It's true that, in the status quo, the most recent Resume the suspended evaluation of _asyncContext_ step must have come from the execution context that's below it on the stack. However, this PR introduces "resume" steps that go in the 'other direction', so if asyncContext's function itself calls an Async/GeneratorFunction, then it may have been resumed several times since outerContext resumed it.

So unless you want to introduce terminology that distinguishes the two 'directions' of resumes, I think it might be hard to describe how outerContext relates to asyncContext (other than being directly below it on the stack, of course, but that isn't really NOTE-worthy).

1. Set the code evaluation state of _asyncContext_ such that when evaluation is resumed with a Completion _completion_, the following steps of the algorithm that invoked Await will be performed, with _completion_ available.
1. Return.
1. NOTE: This returns to the evaluation of the operation that had most previously resumed evaluation of _asyncContext_.
1. Let _outerContext_ be the running execution context.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently, the spec only uses "outer" when talking about environments, which is appropriate because we think of outer and inner scopes. But for these execution contexts, outer/inner doesn't seem like the right metaphor. Elsewhere, the spec calls them "caller" + "callee". (Which makes sense, though the fact that they only differ by one letter makes them a bit hard to distinguish visually.)

@bakkot
Copy link
Contributor Author

bakkot commented Jan 18, 2023

Closing in favor of #2665 etc.

@bakkot bakkot closed this Jan 18, 2023
@ljharb ljharb deleted the execution-context-return branch January 18, 2023 23:20
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.

Editorial: Notes that tell you where a Return step returns to
3 participants