Editorial: describe behavior for algorithms without a "Return" #2397

bakkot · 2021-04-29T00:07:17Z

spec.html

ljharb · 2021-04-29T20:34:19Z

spec.html

@@ -3574,7 +3569,7 @@ <h1>Await</h1>
          1. Perform ! PerformPromiseThen(_promise_, _onFulfilled_, _onRejected_).
          1. Remove _asyncContext_ 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. 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. Return NormalCompletion(~unused~).


why does this one need an explicit return statement at all?

It's not strictly necessary, but I think it's helpful so that the "this" in the immediately following note is clear.

Because the subsequent NOTE-step needs something to refer to?
[was replying to ljharb, github didn't show me bakkot's response for some reason.]

I'm sure the note could be reworded to refer to the implicit return behavior without the need for a placeholder step, if that were desired.

Because the subsequent NOTE-step needs something to refer to?

Yes. It could probably be rephrased, but I like this wording.

ljharb · 2021-04-29T20:34:47Z

spec.html

+            1. [id="step-asyncfunctionstart-return-undefined"] Return NormalCompletion(~unused~).
          1. Push _asyncContext_ onto the execution context stack; _asyncContext_ is now the running execution context.
          1. Resume the suspended evaluation of _asyncContext_. Let _result_ be the value returned by the resumed computation.


is this correct, given that 2 lines down, the returned value is used?

I would not regard the assertion as using the value.

additionally, the step ID says "return undefined" - is it not important that it's "undefined" versus an unused value?

No, that's just an arbitrary label for making the later step reference work. I didn't change it because step IDs don't yet support oldids and I didn't want to break existing links.

bakkot · 2021-04-29T20:59:39Z

I note that this races with two new usages of Return. in #1668, and whichever lands second will need to be rebased. I'm happy to do that myself, I'm just commenting so we don't lose track.

michaelficarra · 2021-04-29T21:33:15Z

Thinking about this more, we could instead use a new kind of completion record here (just for procedure early exits) that doesn't have a [[Value]] slot, making the attempt to use the value of an AO that returns it an editorial type error through existing means.

bakkot · 2021-04-29T21:35:22Z

I would prefer not to further complicate the distinction between normal and abrupt completions, I think.

michaelficarra · 2021-04-29T21:49:23Z

Yeah, I think I agree. We might be able to make some improvements in this area once completion value reform is complete.

jmdyck · 2021-04-30T03:04:14Z

spec.html

-        <emu-alg>
-          1. Return NormalCompletion(*undefined*).
-        </emu-alg>
+        <p>If all steps in an abstract operation have been executed without returning, the abstract operation behaves as though there were an additional step reading "Return NormalCompletion(~unused~)." after the existing steps. ~unused~ is a special value which is never directly consumed.</p>


"If all steps in an abstract operation have been executed without returning..."

Well, not necessarily all steps, because if there are If-else-steps, we might have executed only one of the arms. Maybe "If an abstract operation completes execution without returning ..." ?

Also, it's not just abstract operations, there's at least one syntax-directed operation (ForDeclarationBindingInstantiation) that this applies to. (Or maybe you consider SDOs a subset of AOs, which the spec isn't entirely clear on.)

"~unused~ is a special value which is never directly consumed."

So it might be indirectly consumed? What does that even mean?

Maybe "~unused~ is an ad hoc value that is only used for this purpose."

Also maybe "Operations that return ~unused~ are typically invoked in a Perform step." (I only found one exception, where AsyncGeneratorEnqueue has Let _check_ be AsyncGeneratorValidate(...).)

I'd say "Operations that return ~unused~ must only be invoked in a Perform step.`

So then what about the AsyncGeneratorEnqueue case?

@jmdyck

Well, not necessarily all steps, because if there are If-else-steps, we might have executed only one of the arms.

I was modeling the if-else as a single step (with the arms being substeps), but I can see how your interpretation is plausible.

Maybe "... completes execution without returning ..." ?

I'd be OK with that, though I don't love it. Open to other phrasings here.

I originally said something like "if control reaches the end of the algorithm", but I felt that was maybe too confusing, since we normally talk about control-flow of the JS being executed rather than of spec steps.

Also, it's not just abstract operations, there's at least one syntax-directed operation (ForDeclarationBindingInstantiation) that this applies to. (Or maybe you consider SDOs a subset of AOs, which the spec isn't entirely clear on.)

For avoidance of doubt I'll list SDOs explicitly.

So it might be indirectly consumed? What does that even mean?

It means the completion containing it is consumed, or it's stored in a binding within a macro but the binding is not used after the macro, etc. I am content with this phrasing personally; does anyone else want to weigh in?

@ljharb

Operations that return ~unused~ must only be invoked in a Perform step

That's not a guarantee I want to make: it is reasonable to have instances of the pattern

1. Let _result_ be SomeProcedure(whatever). 1. If _result_ is an abrupt completion, then 1. Do some cleanup.

Hmm, that's a good question/point about those cases. I don't have a suggestion.

I was modeling the if-else as a single step (with the arms being substeps),

Indeed, that's how structured programming wants us to think of it, but when each Else has a different step-number, it's harder to suggest that they're not different steps.

Maybe "... completes execution without returning ..." ?

I'd be OK with that, though I don't love it.

Yeah, I'm not thrilled with it either. The problem is, the existing spec gives us almost no terminology/model for the execution of algorithms.

I originally said something like "if control reaches the end of the algorithm", but I felt that was maybe too confusing, since we normally talk about control-flow of the JS being executed rather than of spec steps.

Yeah, I don't see any examples of the spec using "control" in the context of performing spec steps. At least for "execute" there are a handful of examples.

bakkot · 2021-06-11T18:22:54Z

This should wait on #2413 / #2429, which touch several of the same places.

jmdyck · 2022-02-10T01:31:57Z

I think, once PR #2547 lands, you won't need this PR any more, because there won't be any algorithms that complete without a Return.

Or I suppose you could say that if it happens it's an editorial error.

jmdyck · 2022-03-10T15:37:31Z

If you rebase to main (now that #2547 has landed), I think you'll find that there isn't much left in this PR.

Note that 9.10.3 Execution doesn't have a "Return" step, so you might want to add a Return ~unused~. On the other hand, it's a weird case, so maybe "Return" doesn't mean anything?

bakkot · 2023-02-15T23:14:45Z

Fixed by #2547 instead.

Editorial: describe behavior for algorithms without a "Return"

4e44be6

bakkot mentioned this pull request Apr 29, 2021

Linting the spec tc39/ecmarkup#173

Open

ljharb added the editorial change label Apr 29, 2021

michaelficarra reviewed Apr 29, 2021

View reviewed changes

spec.html Show resolved Hide resolved

no argument-less return

f126c16

ljharb reviewed Apr 29, 2021

View reviewed changes

jmdyck reviewed Apr 30, 2021

View reviewed changes

partially address jmdyck's comments

3443b6b

radar210love007 approved these changes May 9, 2021

View reviewed changes

syg approved these changes Jun 2, 2021

View reviewed changes

michaelficarra approved these changes Jun 2, 2021

View reviewed changes

ljharb force-pushed the master branch 3 times, most recently from 3d0c24c to 7a79833 Compare June 29, 2021 02:21

bakkot mentioned this pull request Feb 9, 2022

third batch of suggested changes for PR #2547 #2657

Merged

bakkot closed this Feb 15, 2023

bakkot deleted the return branch February 15, 2023 23:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editorial: describe behavior for algorithms without a "Return" #2397

Editorial: describe behavior for algorithms without a "Return" #2397

bakkot commented Apr 29, 2021

ljharb Apr 29, 2021

bakkot Apr 29, 2021

jmdyck Apr 29, 2021 •

edited

Loading

ljharb Apr 29, 2021

bakkot Apr 29, 2021

ljharb Apr 29, 2021

bakkot Apr 29, 2021

ljharb Apr 29, 2021

bakkot Apr 29, 2021

bakkot commented Apr 29, 2021

michaelficarra commented Apr 29, 2021

bakkot commented Apr 29, 2021

michaelficarra commented Apr 29, 2021

jmdyck Apr 30, 2021 •

edited

Loading

ljharb Apr 30, 2021

jmdyck Apr 30, 2021

bakkot Apr 30, 2021

ljharb Apr 30, 2021 •

edited

Loading

jmdyck Apr 30, 2021

bakkot commented Jun 11, 2021 •

edited

Loading

jmdyck commented Feb 10, 2022

jmdyck commented Mar 10, 2022

bakkot commented Feb 15, 2023

Editorial: describe behavior for algorithms without a "Return" #2397

Editorial: describe behavior for algorithms without a "Return" #2397

Conversation

bakkot commented Apr 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmdyck Apr 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bakkot commented Apr 29, 2021

michaelficarra commented Apr 29, 2021

bakkot commented Apr 29, 2021

michaelficarra commented Apr 29, 2021

jmdyck Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb Apr 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bakkot commented Jun 11, 2021 • edited Loading

jmdyck commented Feb 10, 2022

jmdyck commented Mar 10, 2022

bakkot commented Feb 15, 2023

jmdyck Apr 29, 2021 •

edited

Loading

jmdyck Apr 30, 2021 •

edited

Loading

ljharb Apr 30, 2021 •

edited

Loading

bakkot commented Jun 11, 2021 •

edited

Loading