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

Make resuming generators unsafe instead of the creation of immovable generators #49194

Merged
merged 1 commit into from
Mar 24, 2018

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Mar 20, 2018

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 20, 2018
@estebank estebank changed the title Make resuming generators unsafe instead of the creation of immovable generators. Fixes #47787 Make resuming generators unsafe instead of the creation of immovable generators Mar 20, 2018
@cramertj
Copy link
Member

cramertj commented Mar 20, 2018

Awesome! Thanks @Zoxc.

Looks like a few of the tests still need unsafe blocks around their calls to resume.

@withoutboats withoutboats added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 20, 2018
@Zoxc Zoxc force-pushed the unsafe-generator branch 4 times, most recently from 496c585 to 55d73db Compare March 20, 2018 21:16
@Zoxc
Copy link
Contributor Author

Zoxc commented Mar 20, 2018

Travis passes now.

@@ -18,7 +18,7 @@ impl<T: Generator<Return = ()>> Iterator for W<T> {
type Item = T::Yield;

fn next(&mut self) -> Option<Self::Item> {
match self.0.resume() {
match unsafe { self.0.resume() } {
Copy link
Member

@cramertj cramertj Mar 20, 2018

Choose a reason for hiding this comment

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

This impl isn't actually sound anymore since the iterator can move-- can you leave a comment? (I think it's fine to leave it because the Pin version will come soon, but there should probably be a note about it)

@cramertj
Copy link
Member

@bors r+

The Iterator for W<impl Generator> isn't sound anymore, but it's in a test (which correctly uses it without moving, so no UB will result) and will be replaced shortly, so I think it's fine to leave as-is.

@bors
Copy link
Contributor

bors commented Mar 20, 2018

📌 Commit 55d73db has been approved by cramertj

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 20, 2018
@Zoxc
Copy link
Contributor Author

Zoxc commented Mar 20, 2018

@bors r=cramertj

@cramertj I already added a comment =P

@bors
Copy link
Contributor

bors commented Mar 20, 2018

📌 Commit 57896ab has been approved by cramertj

@cramertj
Copy link
Member

I already added a comment =P

Nice :)

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 23, 2018
Make resuming generators unsafe instead of the creation of immovable generators

cc @withoutboats

Fixes rust-lang#47787
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
Make resuming generators unsafe instead of the creation of immovable generators

Fixes rust-lang#47787
kennytm added a commit to kennytm/rust that referenced this pull request Mar 24, 2018
Make resuming generators unsafe instead of the creation of immovable generators

cc @withoutboats

Fixes rust-lang#47787
bors added a commit that referenced this pull request Mar 24, 2018
@bors bors merged commit 57896ab into rust-lang:master Mar 24, 2018
@Zoxc Zoxc deleted the unsafe-generator branch March 24, 2018 21:55
Nemo157 added a commit to Nemo157/futures-await that referenced this pull request Mar 25, 2018
As of rust-lang/rust#49194 it's now unsafe to
resume a generator, but safe to create an immovable generator.

Fixes alexcrichton#74
Nemo157 added a commit to Nemo157/futures-await that referenced this pull request Mar 25, 2018
As of rust-lang/rust#49194 it's now unsafe to
resume a generator, but safe to create an immovable generator.

Fixes alexcrichton#74
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants