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: Notes that tell you where a Return step returns to #2400

Closed
jmdyck opened this issue May 3, 2021 · 6 comments · Fixed by #2665
Closed

Editorial: Notes that tell you where a Return step returns to #2400

jmdyck opened this issue May 3, 2021 · 6 comments · Fixed by #2665

Comments

@jmdyck
Copy link
Collaborator

jmdyck commented May 3, 2021

Normally, when an operation executes a Return step, it transfers control back to the operation that invoked it. In fact, this is so normal that the spec doesn't even bother saying it.

So it's pretty weird that when the operation GeneratorYield executes the Return at step 9, it doesn't return to its caller.

What makes it even weirder is that the only thing in the spec that tells you that it doesn't return to its caller is a Note (step 10), but Notes are supposed to be non-normative. If you remove the operation's Notes, there's nothing to tell you that step 9 isn't a garden-variety Return.

(As for the Return at 8.a, it's harder to say what the normal interpretation would be, because it's less clear what would constitute its "caller" if/when it executes, though I'd say that the GeneratorResume that will have just resumed it would have a better claim than whatever originally called GeneratorYield. So again, the Note at 8.b is necessary to convey the intended semantics of the Return step.)

The spec has 2 other places where a Note tells you where a Return returns to:

Given that Notes should be non-normative, what are we to make of these cases?


For brevity, ORT = "operation return-target", i.e. where a Return-step in an abstract operation will return to.

In order for the Return steps to behave as the Notes indicate, the assumed mental model appears to be this:

If algorithm A invokes operation B, and B has a step of the form:

Remove _context_ from the execution context stack
 and restore the execution context that is at the top of the execution context stack
 as the running execution context.

then when B executes that step, this causes B to modify its ORT. Specifically, B's ORT changes from:

  • A, the algorithm that invoked B
    to:
  • R, the operation that had "most previously" (which I read as "most recently") resumed evaluation of context. I believe this is also the operation that was executing when the (newly) running execution context was last "running".

(E.g., when B is GeneratorYield, A might be a definition of the Evaluation SDO for a YieldExpression, and R is GeneratorResume.)

Roughly speaking, the assumption appears to be that the ORT is an attribute of each execution context, so when you change the running execution context, you change where the current operation will return to.

(1)
It's odd that the spec would assume this mental model, because as far as I can tell, it doesn't do anything to give you that mental model. I had to reverse-engineer it from Notes.

(2)
As a model, it leaves questions unanswered:

  • How does an execution context's ORT attribute get set in the first place? Does it ever change?
  • What does it mean for an execution context to have single ORT when lots of operations might execute during the 'lifetime' of an execution context? They can't all return to the same place.
  • What about operations that execute in the absence of an execution context?
  • Does an execution context have other implied attributes that affect the execution of operations?

(3)
Even if we could answer those questions, it just seems to me like a bad model, where the semantics of something as basic as a Return step involve the current state of the execution context stack. In my opinion, we should only use Return when it has garden-variety semantics. If anything else is happening, we should use different phrasing. I'm working on a PR to that effect.

@jmdyck
Copy link
Collaborator Author

jmdyck commented May 5, 2021

In Await and GeneratorYield and (since the merge of PR #2413) AsyncGeneratorYield, the relevant pseudo-code is roughly of the form:

  1. Remove contextA from the execution context stack and restore the execution context that is at the top of the execution context stack as the running execution context.
  2. Set the code evaluation state of contextA such that when evaluation is resumed with a Completion v the following steps will be performed:
    1. A-STEPS using v
    1. Return A-VALUE
    1. NOTE: returns to whatever called this abstract operation
  3. Return B-VALUE.
  4. NOTE: This returns to the evaluation of the operation that had most previously resumed evaluation of contextA.

I believe we could editorially change this to (something like):

  1. Remove contextA [etc]
  2. Let contextB be the running execution context.
  3. NOTE: contextB is the execution context that most recently resumed the evaluation of contextA.
  4. Resume contextB passing B-VALUE. Let v be the value returned by the resumed computation. If contextA is ever resumed again, let v be the value with which it was resumed.
  5. Assert: If control reaches here, contextA is once again the running execution context.
  6. A-STEPS using v
  7. Return A-VALUE. [which returns to whatever called this abstract operation]

(So one odd Return has become a garden-variety Return, and one has become a Resume.)


However, [AsyncGeneratorYield](https://tc39.es/ecma262/#sec-asyncgeneratoryield) doesn't fit this pattern. Before the outer `Return` occurs, there's a call to `AsyncGeneratorResolve`, and there's no semantics-preserving place to put it in my suggested rewrite.

Mind you, this call to AsyncGeneratorResolve is somewhat concerning anyway: it occurs after genContext has been popped from the execution context stack, but before control has been transferred to the underlying context. So the circumstances of its execution are a bit puzzling.

@jmdyck
Copy link
Collaborator Author

jmdyck commented May 5, 2021

Oh, also, in the case of Await, I believe it could become a proper abstract operation rather than a shorthand.

@bakkot
Copy link
Contributor

bakkot commented May 5, 2021

The "Resume ... passing ..." language could be usefully extended to GeneratorStart as well. GeneratorStart currently has (roughly, skipping some cases and details)

1. Set the code evaluation state of _genContext_ such that when evaluation is resumed for that execution context the following steps will be performed:
  1. Let _result_ be the result of evaluating _generatorBody_.
  1. Assert: If we return here, the generator either threw an exception or performed either an implicit or explicit return.
  1. Remove _genContext_ from the execution context stack and restore the execution context that is at the top of the execution context stack as the running execution context.
  1. Return CreateIterResultObject(_result_.[[Value]], *true*).

That final Return goes to step 9 of GeneratorResume (or step 10 of GeneratorResumeAbrupt), which reads

1. Resume the suspended evaluation of _genContext_ using NormalCompletion(_value_) as the result of the operation that suspended it. Let _result_ be the value returned by the resumed computation.

Modulo an unfortunately normative change to the Realm used for the IterResultObject (which is wildly inconsistent in implementations), we could rewrite GeneratorStart as

1. Set the code evaluation state of _genContext_ such that when evaluation is resumed for that execution context the following steps will be performed:
  1. Let _result_ be the result of evaluating _generatorBody_.
  1. Assert: If we return here, the generator either threw an exception or performed either an implicit or explicit return.
  1. Let _iterResult_ be CreateIterResultObject(_result_.[[Value]], *true*).
  1. Remove _genContext_ from the execution context stack and restore the execution context that is at the top of the execution context stack as the running execution context.
  1. Let _outerContext_ be the running execution context.
  1. Resume _outerContext_ passing _iterResult_.
  1. Assert: Control does not reach here. _genContext_ can be discarded at this point.

This also applies to AsyncFunctionStart and (I think) AsyncGeneratorStart, though in those cases the value with which the outer context is resumed will not be consumed (except in AsyncFunctionStart, to assert it is *undefined*). But even so, changing the Return to an explicit Resume plus an assertion that control does not pass that point would be an improvement.

@brabalan
Copy link
Contributor

brabalan commented May 5, 2021

1. Remove _contextA_ [etc]
2. Let _contextB_ be the running execution context.
3. NOTE: _contextB_ is the execution context that most recently resumed the evaluation of _contextA_.

Why don’t we store a reference to _contextB_ in _contextA_ when resuming it? This way we could say Let _contextA_.[[lastResumed]] be the running execution context.

@jmdyck
Copy link
Collaborator Author

jmdyck commented May 5, 2021

Why don’t we store a reference to _contextB_ in _contextA_ when resuming it? This way we could say Let _contextA_.[[lastResumed]] be the running execution context.

The current text says that control "returns to the evaluation of the operation that had most previously resumed evaluation of contextA", which suggests that contextA has to remember where that is, and your suggestion would be a way to make that more explicit.

But really, control just transfers to what is now the running execution context, i.e., the context that was "under" contextA on the stack, which by design is also the context that most recently resumed contextA. In effect, the execution context stack does all the remembering we need.

@brabalan
Copy link
Contributor

brabalan commented May 6, 2021

OK, I missed that "running execution context" is a synonym to "top of the stack". It makes sense now, thanks.

jmdyck added a commit to jmdyck/ecma262 that referenced this issue Feb 15, 2022
That is, eliminate NOTE-steps that tell you
where a Return-step returns to.

Resolves tc39#2400
by performing the transformation outlined in:
tc39#2400 (comment)
jmdyck added a commit to jmdyck/ecma262 that referenced this issue Feb 28, 2022
That is, eliminate NOTE-steps that tell you
where a Return-step returns to.

Resolves tc39#2400
by performing the transformation outlined in:
tc39#2400 (comment)
jmdyck added a commit to jmdyck/ecma262 that referenced this issue Nov 4, 2022
That is, eliminate NOTE-steps that tell you
where a Return-step returns to.

Resolves tc39#2400
by performing the transformation outlined in:
tc39#2400 (comment)
@ljharb ljharb closed this as completed in 1bb98b3 Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants