Editorial: Eliminate weird Return notes

[jmdyck]

That is, eliminate NOTE-steps that tell you where a Return-step returns to.

Resolves #2400 by performing the transformation outlined in #2400 (comment)

This PR goes in roughly the same direction as #2429, but not as far, so maybe it has a better chance of being approved.

[bakkot]

This PR goes in roughly the same direction as https://github.com/tc39/ecma262/pull/2429[](https://github.com/jmdyck), but not as far, so maybe it has a better chance of being approved.

#2429 is mostly just waiting for us to finish #2547; it's not that anyone had a problem with it.

[jmdyck]

Oh, that reminds me, I had some problems with it ... :)

[michaelficarra]

Couldn't we do something like this?

Suggested change

	1. Resume _callerContext_ passing NormalCompletion(_value_). If _genContext_ is ever resumed again, let _result_ be the Completion Record with which it is resumed.
	1. Assert: If control reaches here, then _genContext_ is the running execution context again.
	1. Resume _callerContext_, passing NormalCompletion(_value_).
	1. Assert: If control reaches here, then _genContext_ is the running execution context again.
	1. Let _result_ be the Completion Record with which _genContext_ was resumed.

[jmdyck]

Writing Resume _callerContext_, passing NormalCompletion(_value_). as a complete step suggests (to me) that you can do that Resume and then immediately proceed to the next step of the AO. (Note that the spec has Resumes that, as far as I can tell, work exactly that way, in ScriptEvaluation, ExecuteModule, and PerformEval.)

Instead, I think it's important to emphasize that this Resume causes execution of this AO to pause indefinitely (possibly forever), and the only way for this step to complete (and control to then proceed to the next step) is for _genContext_ to be resumed again.

[michaelficarra]

I don't see how that's unclear from my suggestion. I still have the assert. I've just moved the alias declaration to its own step later.

[jmdyck]

I think the spec should be clear about whether a given Resume is 'blocking' or not. If (as in your suggestion) we don't try to convey that in the Resume step itself, then we should do so in its immediate neighborhood. The Assert step sort of helps, but I think we can do better. So if moving the Let to its own step is important, then how about this:

Suggested change

	1. Resume _callerContext_ passing NormalCompletion(_value_). If _genContext_ is ever resumed again, let _result_ be the Completion Record with which it is resumed.
	1. Assert: If control reaches here, then _genContext_ is the running execution context again.
	1. Resume _callerContext_ passing NormalCompletion(_value_).
	1. NOTE: The above step transfers control to _callerContext_ and pauses. The only way for it to un-pause and have control proceed to the subsequent steps in this algorithm is for _genContext_ to be resumed again, which might never happen.
	1. Assert: _genContext_ is the running execution context.
	1. Let _result_ be the Completion Record with which _genContext_ was just resumed.

I like that the Note is more explicit and explanatory.

[michaelficarra]

That works for me.

[jmdyck]

Done.

Thinking more about the content of the Note, I still like that it's being conveyed to the reader, but I'm concerned that it's being conveyed in a Note, which you should be able to delete/ignore without normative effect.

So I think we need to:

• give a normative semantics for blocking and non-blocking Resume, and
• syntactically distinguish them somehow.

However, I'm okay with leaving all that to a later PR. (Note that it should be somewhat easier if PR #2664 is merged.)

[michaelficarra]

Summary from editor call today is that we should separate out the deduplication (into its own PR if you want) and focus on the step refactoring. For all existing cases of this usage of "resume", we'd like to instead use a new phrasing that explicitly mentions a transfer of control.

[bakkot]

Probably in addition to "resume"; that is, both resume the execution context and then explicitly transfer control to it.

[jmdyck]

Summary from editor call today is that we should separate out the deduplication (into its own PR if you want) and focus on the step refactoring.

Okay, I've reset this branch back to the first commit.

For all existing cases of this usage of "resume", we'd like to instead use a new phrasing that explicitly mentions a transfer of control.

That's fine with me, but are you saying you'd like that in this PR?

The point of this PR is to eliminate the need for the weird Return notes -- it only touches the 3 'yield-side' transfers of control,
and uses pre-existing terminology.

I believe you're talking about including the 6 'next-side' transfers, and introducing new terminology (which you'd presumably want defined/explained). I'm happy to do that work, but it seems like a distinct PR to me.

---

Also, I'll point out that that PR will be easier if we first refactor all the control-transfers and associated execution-stack manipulations down to 2 abstract operations, e.g.:

• RunSuspendedContext for the next-side (see PR Editorial: Factor out AO RunSuspendedContext() #2664), and
• RunCallerContext for the yield-side (from the original form of this PR).

In the editors' call, there seemed to be some opposition to this plan, but I didn't really catch why.

[bakkot]

In the editors' call, there seemed to be some opposition to this plan, but I didn't really catch why.

There is always a cost to factoring stuff out, which is that it adds a layer of indirection before you can see what's actually being done. In this case, because the operation is quite rare and the thing the operation does is extremely weird, I don't think that cost is worth it.