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.

Sign up for GitHub

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: describe behavior for algorithms without a "Return" #2397

Closed
wants to merge 3 commits into from
Closed

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

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e44be6
Editorial: describe behavior for algorithms without a "Return"
bakkot Apr 29, 2021
f126c16
no argument-less return
bakkot Apr 29, 2021
3443b6b
partially address jmdyck's comments
bakkot Apr 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -824,10 +824,7 @@ <h1>Implicit Completion Values</h1>
1. Assert: _completionRecord_ is a Completion Record.
1. Return _completionRecord_ as the Completion Record of this abstract operation.
</emu-alg>
<p>A &ldquo;return&rdquo; statement without a value in an algorithm step means the same thing as:</p>
<emu-alg>
1. Return NormalCompletion(*undefined*).
</emu-alg>
<p>If an abstract operation or syntax-directed operation completes execution without returning, it 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. Operations that may return NormalCompletion(~unused~) are typically invoked in a "Perform" step.</p>
Copy link
Collaborator

@jmdyck jmdyck Apr 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"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(...).)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So then what about the AsyncGeneratorEnqueue case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

@ljharb ljharb Apr 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<p>Any reference to a Completion Record value that is in a context that does not explicitly require a complete Completion Record value is equivalent to an explicit reference to the [[Value]] field of the Completion Record value unless the Completion Record is an abrupt completion.</p>
</emu-clause>

Expand Down Expand Up @@ -3525,7 +3522,7 @@ <h1>The Completion Record Specification Type</h1>
[[Value]]
</td>
<td>
any ECMAScript language value or ~empty~
any ECMAScript language value, ~empty~, or ~unused~
</td>
<td>
The value that was produced.
Expand Down Expand Up @@ -3572,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~).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

@jmdyck jmdyck Apr 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

1. NOTE: This returns to the evaluation of the operation that had most previously resumed evaluation of _asyncContext_.
</emu-alg>

Expand Down Expand Up @@ -3782,7 +3779,7 @@ <h1>PutValue ( _V_, _W_ )</h1>
1. [id="step-putvalue-toobject"] Let _baseObj_ be ! ToObject(_V_.[[Base]]).
1. Let _succeeded_ be ? _baseObj_.[[Set]](_V_.[[ReferencedName]], _W_, GetThisValue(_V_)).
1. If _succeeded_ is *false* and _V_.[[Strict]] is *true*, throw a *TypeError* exception.
1. Return.
1. Return *undefined*.
1. Else,
1. Let _base_ be _V_.[[Base]].
1. Assert: _base_ is an Environment Record.
Expand Down Expand Up @@ -39480,12 +39477,11 @@ <h1>AsyncFunctionStart ( _promiseCapability_, _asyncFunctionBody_ )</h1>
1. Else,
1. Assert: _result_.[[Type]] is ~throw~.
1. Perform ! Call(_promiseCapability_.[[Reject]], *undefined*, &laquo; _result_.[[Value]] &raquo;).
1. [id="step-asyncfunctionstart-return-undefined"] Return.
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.
Comment on lines +39480 to 39482
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not regard the assertion as using the value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

1. Assert: When we return here, _asyncContext_ has already been removed from the execution context stack and _runningContext_ is the currently running execution context.
1. Assert: _result_ is a normal completion with a value of *undefined*. The possible sources of completion values are Await or, if the async function doesn't await anything, step <emu-xref href="#step-asyncfunctionstart-return-undefined"></emu-xref> above.
1. Return.
1. Assert: _result_ is a normal completion with a value of ~unused~. The possible sources of completion values are Await or, if the async function doesn't await anything, step <emu-xref href="#step-asyncfunctionstart-return-undefined"></emu-xref> above.
</emu-alg>
</emu-clause>
</emu-clause>
Expand Down