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

Jump to bottom

Better iterator close #3075

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Better iterator close #3075

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
86ecfe9
Editorial: remove redundant type assertions from {Async}IteratorClose
bakkot May 25, 2023
f4547f4
Editorial: introduce IteratorCloseForThrow to simplify some cases of …
bakkot May 25, 2023
70a15ee
further split IteratorClose
bakkot May 25, 2023
fbdd3c1
npm run format
bakkot May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
99 changes: 67 additions & 32 deletions spec.html
View file
Open in desktop
Expand Up @@ -6936,25 +6936,55 @@ <h1>
<h1>
IteratorClose (
_iteratorRecord_: an Iterator Record,
_completion_: a Completion Record,
): a Completion Record
): either a normal completion containing ~unused~ or a throw completion
</h1>
<dl class="header">
<dt>description</dt>
<dd>It is used to notify an iterator that it should perform any actions it would normally perform when it has reached its completed state.</dd>
</dl>
<emu-alg>
1. Assert: _iteratorRecord_.[[Iterator]] is an Object.
1. Let _iterator_ be _iteratorRecord_.[[Iterator]].
1. Let _innerResult_ be Completion(GetMethod(_iterator_, *"return"*)).
1. If _innerResult_.[[Type]] is ~normal~, then
1. Let _return_ be _innerResult_.[[Value]].
1. If _return_ is *undefined*, return ? _completion_.
1. Set _innerResult_ to Completion(Call(_return_, _iterator_)).
1. If _completion_.[[Type]] is ~throw~, return ? _completion_.
1. If _innerResult_.[[Type]] is ~throw~, return ? _innerResult_.
1. Let _return_ be ? GetMethod(_iterator_, *"return"*).
1. If _return_ is *undefined*, return ~unused~.
1. Let _innerResult_ be ? Call(_return_, _iterator_).
1. If _innerResult_.[[Value]] is not an Object, throw a *TypeError* exception.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Delete the .[[Value]] here, because the previous line unwrapped it.

1. Return ? _completion_.
1. Return ~unused~.
</emu-alg>
</emu-clause>

<emu-clause id="sec-iteratorcloseforthrow" type="abstract operation">
<h1>
IteratorCloseForThrow (
_iteratorRecord_: an Iterator Record,
): ~unused~
</h1>
<dl class="header">
<dt>description</dt>
<dd>It is used to notify an iterator that it should perform any actions it would normally perform when it has reached its completed state. Any exceptions which occur during the process are suppressed.</dd>
</dl>
<emu-alg>
1. Perform Completion(IteratorClose(_iteratorRecord_)).
1. Return ~unused~.
</emu-alg>
</emu-clause>

<emu-clause id="sec-iteratorcloseforcompletion" type="abstract operation">
<h1>
IteratorCloseForCompletion (
_iteratorRecord_: an Iterator Record,
_completion_: a Completion Record,
jmdyck marked this conversation as resolved.
Show resolved Hide resolved
): a Completion Record
</h1>
<dl class="header">
<dt>description</dt>
<dd>It is used to notify an iterator that it should perform any actions it would normally perform when it has reached its completed state. It returns either the passed Completion Record or the one which arises from that action.</dd>
</dl>
<emu-alg>
1. If _completion_.[[Type]] is ~throw~, then
1. Perform IteratorCloseForThrow(_iteratorRecord_).
1. Else,
1. Perform ? IteratorClose(_iteratorRecord_).
1. Return _completion_.
</emu-alg>
</emu-clause>

Expand All @@ -6967,7 +6997,7 @@ <h1>IfAbruptCloseIterator ( _value_, _iteratorRecord_ )</h1>
<p>means the same thing as:</p>
<emu-alg>
1. Assert: _value_ is a Completion Record.
1. If _value_ is an abrupt completion, return ? IteratorClose(_iteratorRecord_, _value_).
1. If _value_ is an abrupt completion, return ? IteratorCloseForCompletion(_iteratorRecord_, _value_).
1. Else, set _value_ to _value_.[[Value]].
</emu-alg>
</emu-clause>
Expand All @@ -6984,7 +7014,6 @@ <h1>
<dd>It is used to notify an async iterator that it should perform any actions it would normally perform when it has reached its completed state.</dd>
</dl>
<emu-alg>
1. Assert: _iteratorRecord_.[[Iterator]] is an Object.
1. Let _iterator_ be _iteratorRecord_.[[Iterator]].
1. Let _innerResult_ be Completion(GetMethod(_iterator_, *"return"*)).
1. If _innerResult_.[[Type]] is ~normal~, then
Expand Down Expand Up @@ -9392,7 +9421,7 @@ <h1>
<emu-alg>
1. Let _iteratorRecord_ be ? GetIterator(_value_, ~sync~).
1. Let _result_ be Completion(IteratorBindingInitialization of |ArrayBindingPattern| with arguments _iteratorRecord_ and _environment_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorClose(_iteratorRecord_, _result_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorCloseForCompletion(_iteratorRecord_, _result_).
1. Return ? _result_.
</emu-alg>
<emu-grammar>ObjectBindingPattern : `{` `}`</emu-grammar>
Expand Down Expand Up @@ -20594,13 +20623,14 @@ <h1>
<emu-grammar>ArrayAssignmentPattern : `[` `]`</emu-grammar>
<emu-alg>
1. Let _iteratorRecord_ be ? GetIterator(_value_, ~sync~).
1. Return ? IteratorClose(_iteratorRecord_, NormalCompletion(~unused~)).
1. Perform ? IteratorClose(_iteratorRecord_).
1. Return ~unused~.
</emu-alg>
<emu-grammar>ArrayAssignmentPattern : `[` Elision `]`</emu-grammar>
<emu-alg>
1. Let _iteratorRecord_ be ? GetIterator(_value_, ~sync~).
1. Let _result_ be Completion(IteratorDestructuringAssignmentEvaluation of |Elision| with argument _iteratorRecord_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorClose(_iteratorRecord_, _result_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorCloseForCompletion(_iteratorRecord_, _result_).
1. Return _result_.
</emu-alg>
<emu-grammar>ArrayAssignmentPattern : `[` Elision? AssignmentRestElement `]`</emu-grammar>
Expand All @@ -20612,22 +20642,22 @@ <h1>
1. Assert: _iteratorRecord_.[[Done]] is *true*.
1. Return ? _status_.
1. Let _result_ be Completion(IteratorDestructuringAssignmentEvaluation of |AssignmentRestElement| with argument _iteratorRecord_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorClose(_iteratorRecord_, _result_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorCloseForCompletion(_iteratorRecord_, _result_).
1. Return _result_.
</emu-alg>
<emu-grammar>ArrayAssignmentPattern : `[` AssignmentElementList `]`</emu-grammar>
<emu-alg>
1. Let _iteratorRecord_ be ? GetIterator(_value_, ~sync~).
1. Let _result_ be Completion(IteratorDestructuringAssignmentEvaluation of |AssignmentElementList| with argument _iteratorRecord_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorClose(_iteratorRecord_, _result_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorCloseForCompletion(_iteratorRecord_, _result_).
1. Return _result_.
</emu-alg>
<emu-grammar>ArrayAssignmentPattern : `[` AssignmentElementList `,` Elision? AssignmentRestElement? `]`</emu-grammar>
<emu-alg>
1. Let _iteratorRecord_ be ? GetIterator(_value_, ~sync~).
1. Let _status_ be Completion(IteratorDestructuringAssignmentEvaluation of |AssignmentElementList| with argument _iteratorRecord_).
1. If _status_ is an abrupt completion, then
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorClose(_iteratorRecord_, _status_).
1. If _iteratorRecord_.[[Done]] is *false*, perform IteratorCloseForThrow(_iteratorRecord_).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is _status_ necessarily a throw completion here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes: IteratorDestructuringAssignmentEvaluation can only return a throw completion or a normal completion, so guarding on abruptness of its result suffices.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes: IteratorDestructuringAssignmentEvaluation can only return a throw completion or a normal completion,

So we should change its declared return-type?

1. Return ? _status_.
1. If |Elision| is present, then
1. Set _status_ to Completion(IteratorDestructuringAssignmentEvaluation of |Elision| with argument _iteratorRecord_).
Expand All @@ -20636,7 +20666,7 @@ <h1>
1. Return ? _status_.
1. If |AssignmentRestElement| is present, then
1. Set _status_ to Completion(IteratorDestructuringAssignmentEvaluation of |AssignmentRestElement| with argument _iteratorRecord_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorClose(_iteratorRecord_, _status_).
1. If _iteratorRecord_.[[Done]] is *false*, return ? IteratorCloseForCompletion(_iteratorRecord_, _status_).
1. Return ? _status_.
</emu-alg>
</emu-clause>
Expand Down Expand Up @@ -22028,7 +22058,8 @@ <h1>
1. Return ? _status_.
1. Else,
1. Assert: _iterationKind_ is ~iterate~.
1. Return ? IteratorClose(_iteratorRecord_, _status_).
1. Perform IteratorCloseForThrow(_iteratorRecord_).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is _status_ necessarily a throw completion here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes: _status_ here can only arise from DestructuringAssignmentEvaluation, BindingInitialization, Evaluation of _lhs_ (which is either a LeftHandSideExpression, a ForBinding, a ForDeclaration, or a ForBinding; for each of these Evaluation to throw or normal completions), PutValue, ForDeclarationBindingInitialization, or InitializeReferencedBinding. And those all can only return a throw completion or a normal completion, so guarding on abruptness of `status suffices.

Copy link
Collaborator

Choose a reason for hiding this comment

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

DestructuringAssignmentEvaluation, BindingInitialization, PutValue, ForDeclarationBindingInitialization, and InitializeReferencedBinding are all declared to return normal or abrupt. Can we change abrupt to throw for them?

And for the Evaluation of _lhs_, maybe when _lhsRef_ is abrupt, there could be an Assert that it's a throw completion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(This reminded me of #2676 and #2676 (comment) in particular.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah jeez, you're right, I always forget about yield. These are wrong.

1. Return ? _status_.
1. Let _result_ be Completion(Evaluation of _stmt_).
1. Set the running execution context's LexicalEnvironment to _oldEnv_.
1. If LoopContinues(_result_, _labelSet_) is *false*, then
Expand All @@ -22038,7 +22069,7 @@ <h1>
1. Assert: _iterationKind_ is ~iterate~.
1. Set _status_ to Completion(UpdateEmpty(_result_, _V_)).
1. If _iteratorKind_ is ~async~, return ? AsyncIteratorClose(_iteratorRecord_, _status_).
1. Return ? IteratorClose(_iteratorRecord_, _status_).
1. Return ? IteratorCloseForCompletion(_iteratorRecord_, _status_).
1. If _result_.[[Value]] is not ~empty~, set _V_ to _result_.[[Value]].
</emu-alg>
</emu-clause>
Expand Down Expand Up @@ -23884,9 +23915,9 @@ <h1>Runtime Semantics: Evaluation</h1>
1. Else, set _received_ to Completion(GeneratorYield(_innerResult_)).
1. Else,
1. NOTE: If _iterator_ does not have a `throw` method, this throw is going to terminate the `yield*` loop. But first we need to give _iterator_ a chance to clean up.
1. Let _closeCompletion_ be Completion Record { [[Type]]: ~normal~, [[Value]]: ~empty~, [[Target]]: ~empty~ }.
1. Let _closeCompletion_ be NormalCompletion(~unused~).
1. If _generatorKind_ is ~async~, perform ? AsyncIteratorClose(_iteratorRecord_, _closeCompletion_).
1. Else, perform ? IteratorClose(_iteratorRecord_, _closeCompletion_).
1. Else, perform ? IteratorClose(_iteratorRecord_).
1. NOTE: The next step throws a *TypeError* to indicate that there was a `yield*` protocol violation: _iterator_ does not have a `throw` method.
1. Throw a *TypeError* exception.
1. Else,
Expand Down Expand Up @@ -37380,8 +37411,8 @@ <h1>Array.from ( _items_ [ , _mapfn_ [ , _thisArg_ ] ] )</h1>
1. Let _k_ be 0.
1. Repeat,
1. If _k_ ≥ 2<sup>53</sup> - 1, then
1. Let _error_ be ThrowCompletion(a newly created *TypeError* object).
1. Return ? IteratorClose(_iteratorRecord_, _error_).
1. Perform IteratorCloseForThrow(_iteratorRecord_).
1. Throw a *TypeError* exception.
1. Let _Pk_ be ! ToString(𝔽(_k_)).
1. Let _next_ be ? IteratorStep(_iteratorRecord_).
1. If _next_ is *false*, then
Expand Down Expand Up @@ -40522,8 +40553,8 @@ <h1>
1. If _next_ is *false*, return _target_.
1. Let _nextItem_ be ? IteratorValue(_next_).
1. If _nextItem_ is not an Object, then
1. Let _error_ be ThrowCompletion(a newly created *TypeError* object).
1. Return ? IteratorClose(_iteratorRecord_, _error_).
1. Perform IteratorCloseForThrow(_iteratorRecord_).
1. Throw a *TypeError* exception.
1. Let _k_ be Completion(Get(_nextItem_, *"0"*)).
1. IfAbruptCloseIterator(_k_, _iteratorRecord_).
1. Let _v_ be Completion(Get(_nextItem_, *"1"*)).
Expand Down Expand Up @@ -44637,7 +44668,8 @@ <h1>Promise.all ( _iterable_ )</h1>
1. IfAbruptRejectPromise(_iteratorRecord_, _promiseCapability_).
1. Let _result_ be Completion(PerformPromiseAll(_iteratorRecord_, _C_, _promiseCapability_, _promiseResolve_)).
1. If _result_ is an abrupt completion, then
1. If _iteratorRecord_.[[Done]] is *false*, set _result_ to Completion(IteratorClose(_iteratorRecord_, _result_)).
1. If _iteratorRecord_.[[Done]] is *false*, then
1. Perform IteratorCloseForThrow(_iteratorRecord_).
1. IfAbruptRejectPromise(_result_, _promiseCapability_).
1. Return ? _result_.
</emu-alg>
Expand Down Expand Up @@ -44740,7 +44772,8 @@ <h1>Promise.allSettled ( _iterable_ )</h1>
1. IfAbruptRejectPromise(_iteratorRecord_, _promiseCapability_).
1. Let _result_ be Completion(PerformPromiseAllSettled(_iteratorRecord_, _C_, _promiseCapability_, _promiseResolve_)).
1. If _result_ is an abrupt completion, then
1. If _iteratorRecord_.[[Done]] is *false*, set _result_ to Completion(IteratorClose(_iteratorRecord_, _result_)).
1. If _iteratorRecord_.[[Done]] is *false*, then
1. Perform IteratorCloseForThrow(_iteratorRecord_).
1. IfAbruptRejectPromise(_result_, _promiseCapability_).
1. Return ? _result_.
</emu-alg>
Expand Down Expand Up @@ -44867,7 +44900,8 @@ <h1>Promise.any ( _iterable_ )</h1>
1. IfAbruptRejectPromise(_iteratorRecord_, _promiseCapability_).
1. Let _result_ be Completion(PerformPromiseAny(_iteratorRecord_, _C_, _promiseCapability_, _promiseResolve_)).
1. If _result_ is an abrupt completion, then
1. If _iteratorRecord_.[[Done]] is *false*, set _result_ to Completion(IteratorClose(_iteratorRecord_, _result_)).
1. If _iteratorRecord_.[[Done]] is *false*, then
1. Perform IteratorCloseForThrow(_iteratorRecord_).
1. IfAbruptRejectPromise(_result_, _promiseCapability_).
1. Return ? _result_.
</emu-alg>
Expand Down Expand Up @@ -44963,7 +44997,8 @@ <h1>Promise.race ( _iterable_ )</h1>
1. IfAbruptRejectPromise(_iteratorRecord_, _promiseCapability_).
1. Let _result_ be Completion(PerformPromiseRace(_iteratorRecord_, _C_, _promiseCapability_, _promiseResolve_)).
1. If _result_ is an abrupt completion, then
1. If _iteratorRecord_.[[Done]] is *false*, set _result_ to Completion(IteratorClose(_iteratorRecord_, _result_)).
1. If _iteratorRecord_.[[Done]] is *false*, then
1. Perform IteratorCloseForThrow(_iteratorRecord_).
1. IfAbruptRejectPromise(_result_, _promiseCapability_).
1. Return ? _result_.
</emu-alg>
Expand Down