"Return" at end of algorithm: implicit or explicit?

[jmdyck]

Currently, there are some algorithms where, if an 'early' return doesn't occur, control just falls off the end of the steps. The spec doesn't define what this means. Should we define what it means (implicit Return step), or add an explicit Return to such cases?

[anba]

Good timing, I was about to submit the same issue, because the Temporal proposal uses the same pattern and I wasn't sure if it's actually allowed. 😄

Only the following abstract operations are missing a Return (I hope I haven't missed too many while skimming through the spec):

• 10.1.15 RequireInternalSlot ( O, internalSlot )
• 14.2.3 BlockDeclarationInstantiation ( code, env )
• 14.7.5.4 Runtime Semantics: ForDeclarationBindingInstantiation
• 15.10.3 PrepareForTailCall ( )
• 16.2.1.9 FinishDynamicImport ( referencingScriptOrModule, specifier, promiseCapability, completion )
• 23.2.3.23.1 SetTypedArrayFromTypedArray ( target, targetOffset, source )
• 23.2.3.23.2 SetTypedArrayFromArrayLike ( target, targetOffset, source )
• 23.2.5.1.2 InitializeTypedArrayFromTypedArray ( O, srcArray )
• 23.2.5.1.3 InitializeTypedArrayFromArrayBuffer ( O, buffer, byteOffset, length )
• 23.2.5.1.4 InitializeTypedArrayFromList ( O, values )
• 23.2.5.1.5 InitializeTypedArrayFromArrayLike ( O, arrayLike )
• 25.4.1.4 EnterCriticalSection ( WL )
• 25.4.1.5 LeaveCriticalSection ( WL )
• 25.4.1.6 AddWaiter ( WL, W )
• 25.4.1.7 RemoveWaiter ( WL, W )
• 25.4.1.10 NotifyWaiter ( WL, W )
• 27.6.3.3 AsyncGeneratorValidate ( generator, generatorBrand )

[anba]

FWIW, I think I'd prefer adding an explicit Return, because that is more common (*) throughout the spec.

(*) Especially when also including the two other equivalent forms Return undefined and Return NormalCompletion(undefined).

[ljharb]

RequireInternalSlot isn’t supposed to have a return value; it’s only supposed to be used with “Perform”. I would only expect an explicit return in an AO whose return value is used.

[anba]

RequireInternalSlot is a runtime semantics and as such always returns a completion record. And a completion record needs to have a value, but currently the spec doesn't define what it is (when no explicit Return step is present). The two obvious choices are probably NormalCompletion(undefined) or NormalCompletion(empty). If we go for the former, we might as well add an explicit Return. step to avoid having to define what abstract operations without an explicit Return step return.

[ljharb]

I would assume “empty”. It would be very bizarre to me to add an explicit return statement to AOs that shouldn’t be used to provide a value.

#1796 would surely make AOs like this not return a completion record at all; even prior to that, my opinion holds.

[bakkot]

Well, no, #1796 would still leave RequireInternalSlot returning a completion record, because the whole point of it is to return abrupt completions in some cases.

[anba]

It would be very bizarre to me to add an explicit return statement to AOs that shouldn’t be used to provide a value.

Hmm, this bizarreness is pretty common in the spec, though.

[ljharb]

Good point that it’d still need a completion record.

[bakkot]

I think I lean towards adding an explicit Return., but I also dislike returning a real value from AOs whose return value is not supposed to be consumed (except possibly to check abruptness).

One possibility: we could introduce an explicit value, like ~unused~, which is to be returned in such cases, and declare it to be an editorial error to use that value. Then we could change the

A “return” statement without a value in an algorithm step means the same thing as:

1. Return NormalCompletion(undefined).

bit to say it is Return NormalCompletion(~unused~) instead. (We'd have to ensure we never actually consume such return values, but I think that at least is worth doing anyway.)

I'd consider that an aesthetic improvement to the spec's type system, but I don't know if that would actually justify the increased complexity. Maybe?

[jmdyck]

~unused~ appeals to me, but I am compelled to point out that currently, we only allow language values and ~empty~ in the [[Value]] field of a completion record, so we'd have to either add ~unused~ or look the other way. (Of course, there are lots of similar cases where we look the other way, but I'm guessing most of those will be resolved by #1796. Will this one?)

[bakkot]

I expect #1796 to change the restriction to "anything except a completion record", which would resolve both those cases and this one, yeah.

[jmdyck]

Only the following abstract operations are missing a Return (I hope I haven't missed too many while skimming through the spec):

The only other ones I found are:

• 9.9.3 Execution
• 9.10 ClearKeptObjects ( )
• 9.11 AddToKeptObjects ( object )

[bakkot]

Editors discussed this at the call today, and were in favor of my ~unused~ suggestion above plus some other details. Specifically:

• Change Return. to mean Return NormalCompletion(~unused~)., and declare that it is an editorial error to use the ~unused~ value. I believe this requires only
  • changing step 8 of AsyncFunctionStart to assert the value is ~unused~ instead
  • changing 5.d of PutValue to explicitly return *undefined*
  • tweaking the definition of NormalCompletion to allow ~unused~, as pointed out above, pending resolution of Sorting out completion records #1796.
• Declare that if control reaches the end of an algorithm without returning, there is an implicit Return. step at that point, so that the pattern raised in this issue is explicitly allowed and given semantics (insofar as Return has semantics, anyway).
  • This will render step 9 of AsyncFunctionStart redundant, so we should also remove it at that point

I'll prepare a PR for this later today.