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.

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

Coroutine design notes #52

Merged
merged 8 commits into from
Oct 19, 2020
Merged

Coroutine design notes #52

merged 8 commits into from
Oct 19, 2020

Conversation

samsartor
Copy link
Contributor

@samsartor samsartor commented Sep 5, 2020

I took a crack at a summary of the generalized coroutines design space! Please let me know if there are any questions I could do a better job of answering. I may be away from internet the next few days but I'll be around to answer them sooner than later.

Rendered

@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Sep 7, 2020
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Wow, this is really great. I'm sorry it took me so long to actually read it. I left some thoughts -- I'd be happy to merge as is too if you're done working on this ;)

src/design_notes/general_coroutines.md Outdated Show resolved Hide resolved
src/design_notes/general_coroutines.md Outdated Show resolved Hide resolved
src/design_notes/general_coroutines.md Outdated Show resolved Hide resolved
src/design_notes/general_coroutines.md Outdated Show resolved Hide resolved
src/design_notes/general_coroutines.md Outdated Show resolved Hide resolved
src/design_notes/general_coroutines.md Outdated Show resolved Hide resolved
src/design_notes/general_coroutines.md Show resolved Hide resolved
- Thus, the "generator" syntax proposed in [eRFC-2033][1] and currently
implemented behind the "generator" feature is actually a coroutine syntax for
the sake of these notes, *not a true generator*.
- Note also that "coroutines" here are really "semicoroutines" since they can
Copy link
Member

Choose a reason for hiding this comment

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

I wasn't aware of this distinction before and found this wiki entry helpful: https://en.wikipedia.org/wiki/Coroutine#Comparison_with_generators

Copy link
Contributor Author

@samsartor samsartor Sep 28, 2020

Choose a reason for hiding this comment

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

That is what @CAD97 linked to me as well to explain it! Although we make an additional distinction between generators and semicoroutines which Wikipedia does not seem to do.

@samsartor
Copy link
Contributor Author

I've pushed a new version of the notes which will hopefully address the feedback from @nikomatsakis and @cramertj. I left a lot of comments under those reviews but they have all been effectively merged into the notes, so I've marked the conversations as resolved.

@nikomatsakis
Copy link
Contributor

@samsartor thanks! will review.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! One nit.

- Exact inference rules may only be revealed by an attempt at implementation.
- If we forbid "borrowed data escaping into closure state", the inference
rules should be relatively simple: witnessing any borrow triggers
immovability.
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't really work. For example:

|x: &Foo| {
    let y = &x.field;
    yield;
    drop(y);
}

Here y is live across a yield but it does not require pinning.

Copy link
Contributor Author

@samsartor samsartor Oct 1, 2020

Choose a reason for hiding this comment

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

For that example to compile, we would need to allow borrowed inputs to escape into closure state. In that case, the inference is significantly more complex. But otherwise you'd get an error on the yield, something like:

3 |    yield;
  |    ---- borrowed input `x` escapes into closure state
4 |    drop(y);
  |         - borrow must enter state because it is used here

and the inference would never have to figure out that y is not borrowing from elsewhere in the witness.
I admit I'm not sure if the inference can be run after the escape check. But I'm assuming here that it can.

Obviously this error isn't the best. But since closures similarly forbid moving borrowed inputs into capture state, it is the easiest thing to do for the time being.

Copy link
Contributor

Choose a reason for hiding this comment

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

@samsartor hmm, so, yes. I agree that if &Foo here means "each time the closure is invoked, it will be supplied a fresh reference", then I guess we really can't keep the reference live across the yield. There's definitely some subtlety here. Anyway, I'm going to merge this PR, and thank you again for the effort to create it!

@nikomatsakis nikomatsakis merged commit 1406707 into rust-lang:master Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants