Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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 Zoxc force-pushed the unsafe-generator branch from 55d73db to 57896ab Compare March 20, 2018 23:10
@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
# for free to join this conversation on GitHub. Already have an account? # 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