-
Notifications
You must be signed in to change notification settings - Fork 226
Update spec for yield*. #4514
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.
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
Update spec for yield*. #4514
Conversation
Was very under-specified. This change changes the responsiveness of a `yield*` to be that of `StreamController.addStream`, meaning that it *immediately* reacts to pause and cancel and forwards it to the inner stream. Unlike `await for`, a `yield*` is at the yield for the entire duration of the inner stream, and can cancel at any time, not only when control reaches a `yield` inside the loop. Experience has taught us that with `async*` functions, `await for` and `yield*`, back-pressure using `cancel` or `pause` *must* be acted on immediately, to avoid a stream computation continuing when it has nothing meaningful to do, and the caller knows that and tries to stop it. For example, if you do `stream.first`, it should cancel after the first event, before starting on the computation of the next event, because then it's too late to cancel. Canonical example: ```dart // Stream with one event and no done event. Stream<int> oneValue => Stream<int>.multi((c) => c.add(1)); Stream<int> clone1(Stream<int> stream) async* { await for (var event in stream) yield event; } Stream<int> clone2(Stream<int> stream) async* { yield* stream; } void main() async { print(await oneValue.first); // 1 print(await clone1(oneValue).first); print(await clone2(oneValue).first); print(await clone2(clone1(oneValue)).first); } ``` If the `first` code's cancel doesn't reach the `clone1` before it goes back to the `await for` loop, the cancel will never be processed because control never reaches a `yield` again.
@eernstg WDYT? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a couple of comments.
|
||
\LMHash{}% | ||
If $m$ is marked \code{\ASYNC*} (\ref{functions}), then: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't comment on line 19751, so I'll put it here: What happens if the evaluation of e
in line 19751 does not complete normally? The interesting case would be async*
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The general rule is that if any step of the execution of a statement completes non-normally, unless something else is explicitly stated (like it is for try
/catch
), the execution of the statement completes in the same non-normal way.
So when await for (var x in (throw "Banana")) { ... }
evaluates throw "Banana"
, the (throw "Banana")
statement and then the enitire await for
statement completes throwing "Banana" and a stack trace.
Was very under-specified.
This change changes the responsiveness of a
yield*
to be that ofStreamController.addStream
, meaning that it immediately reacts to pause and cancel and forwards it to the inner stream. Unlikeawait for
, ayield*
is at the yield for the entire duration of the inner stream, and can cancel at any time, not only when control reaches ayield
inside the loop.Experience has taught us that with
async*
functions,await for
andyield*
, back-pressure usingcancel
orpause
must be acted on immediately, to avoid a stream computation continuing when it has nothing meaningful to do, and the caller knows that and tries to stop it.For example, if you do
stream.first
, it should cancel after the first event, before starting on the computation of the next event, because then it's too late to cancel. Canonical example:If the
first
code's cancel doesn't reach theclone1
before it goes back to theawait for
loop, the cancel will never be processed because control never reaches ayield
again.Contribution guidelines:
dart format
.Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.