Editorial: Eliminate "weird returns" from tail-call handling #2430
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As in issue #2400, a "weird return" is when an operation performing a
Return
transfers control to somewhere other than the operation's caller. This PR discusses weird returns in the spec's handling of tail calls, and suggests a way to eliminate them.In EvaluateCall, the last few steps say:
As an example, say that function A has a tail-call to function B. So in step 8, tailPosition is true when func is B, and "the above call" presumably refers to the invocation of
Call(B, ...)
in step 7. So the weird return might be:[[Call]]
internal method thatCall(B, ...)
invokes.But none of these has a Note to confirm the weird returns.
Nor is there a Note to say where they return to. When step 8 says "as if the following return has already occurred", it presumably refers to step 10, and yet it can't be that
Call(B)
(orB.[[Call]]
) simply returns to wherever step 10 normally returns (i.e., to some definition of theEvaluation
SDO), because that wouldn't make any difference. In order for tail-call handling to work, we must "skip over" (working outward):A.[[Call]]
's step that removes A's context from the stack. (This step is the crucial one to "skip", since PrepareForTailCall (for B) already removed A's context from the stack.)I suppose the interpretation that makes the most sense (in the world of weird returns) would be for
Call(B, ...)
to return to whereverCall(A, ...)
would have returned to (and so on recursively, if appropriate). However:I'm pretty sure there's nothing in the spec that tells me this.
If that were the intended interpretation, I'd expect EvaluateCall's step 8 to say something more like:
There's an alternative interpretation, in which
B.[[Call]]
is deemed to return to whereverA.[[Call]]
would have returned. It's probably equivalent to the interpretation given above, but it's a bit fishy that I can't tell which was intended.Note that neither
Call
nor[[Call]]
actually "know" if the call in question is a tail call, so they theoretically wouldn't be able to identify the occasions when a Return will be a weird return. (Although EvaluateCall could be tweaked to pass down that information.)Maybe readers are supposed to intuit this interpretation based on the stack-pop that PrepareForTailCall does. But if we use the assumed mental model from my original comment in Editorial: Notes that tell you where a
Return
step returns to #2400, PrepareForTailCall's removal of A's context from the execution context stack would cause PrepareForTailCall to modify its "operation return-target". Modify it to what is unclear, since A's context was never really "resumed", but that's kind of beside the point, since I think it's pretty clear that PrepareForTailCall must simply return to its caller (EvaluateCall). Perhaps there's a different mental model that covers both the cases in Editorial: Notes that tell you where aReturn
step returns to #2400 and the ones here, but I don't think it's worth looking for. I think we should eliminate weird returns.If we want to adopt a model in which weird returns do not occur, then the spec will have to use a different mechanism to handle tail calls. This PR suggests one way to do so. The changes are fairly minor.
As far as I can tell, the crucial thing is to prevent double-popping. That is, if PrepareForTailCall removes a context from the execution context stack, then the step that would 'normally' remove it must be prevented from doing so. I think the simplest way to accomplish this is to have these removal steps (which all occur in
[[Call]]
methods) first check if the running execution context is the one they 'expect', and pop it only if it is.