Skip to content

Allow generators to impl Clone/Copy - #95137

Closed
canndrew wants to merge 12 commits into
rust-lang:masterfrom
canndrew:clone-generators
Closed

Allow generators to impl Clone/Copy#95137
canndrew wants to merge 12 commits into
rust-lang:masterfrom
canndrew:clone-generators

Conversation

@canndrew

@canndrewcanndrew commented Mar 20, 2022

Copy link
Copy Markdown
Contributor

This PR allows generators to implement Clone/Copy if all their upvars and all their locals which are held across a yield implement Clone/Copy. This only applies to non-static generators, and so excludes generators which contain self-references and the generators that async blocks/fns desugar to,

r? @oli-obk

@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 20, 2022
@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 20, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjorn3bjorn3 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, 2022
@canndrewcanndrew mentioned this pull request Mar 27, 2022
Open
3 tasks
oli-obk
oli-obk requested changes Mar 28, 2022
View reviewed changes

@oli-obkoli-obk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd like to see some tests on various kinds of async blocks, too. Even if we're just testing the diagnostics that way.

Comment threadcompiler/rustc_mir_transform/src/shim.rs Outdated
Comment threadcompiler/rustc_mir_transform/src/shim.rs Outdated
Comment threadcompiler/rustc_trait_selection/src/traits/select/mod.rs Outdated
Comment threadcompiler/rustc_trait_selection/src/traits/select/mod.rs Outdated
Comment threadcompiler/rustc_trait_selection/src/traits/select/mod.rs Outdated
@canndrew

canndrew commented Apr 10, 2022

Copy link
Copy Markdown
ContributorAuthor

Sorry for the delay. I've done the requested changes.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@canndrew

canndrew commented Apr 10, 2022

Copy link
Copy Markdown
ContributorAuthor

I've also added a test which checks that feature(generator_clone) doesn't interfere with the clonability of async blocks and doesn't effect the error message of trying to clone an async block. And I've also rebased off of master in order to fix the ui tests which were failing due to minor changes in diagnostics.

oli-obk
oli-obk reviewed Apr 12, 2022
View reviewed changes
Comment threadcompiler/rustc_trait_selection/src/traits/select/mod.rs
Comment on lines +1909 to +1913
let all = substs
.as_generator()
.upvar_tys()
.chain(iter::once(substs.as_generator().witness()))
.collect::<Vec<_>>();

@oli-obkoli-obkApr 12, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not due to this PR, but I really wish we had ways to track where an inference var got resolved and then bubble this span up here. In the current design there is no way to point at the local that causes the generator to stop being clone/copy, all we can do is keep pointing at the entire generator :(

@JohnCSimonJohnCSimon 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 8, 2022
@bors

bors commented May 15, 2022

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #96883) made this pull request unmergeable. Please resolve the merge conflicts.

@oli-obkoli-obk removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 11, 2022
@oli-obkoli-obk added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2022
@oli-obk

oli-obk commented Jun 11, 2022

Copy link
Copy Markdown
Contributor

Oof I forgot about this. Changed the label so it shows up in my queue

@oli-obk

oli-obk commented Jun 21, 2022

Copy link
Copy Markdown
Contributor

ok, did another pass. This lgtm now. r=me after a rebase

@oli-obkoli-obk changed the title WIP: Allow generators to impl Clone/CopyAllow generators to impl Clone/CopyJun 21, 2022
@oli-obk

oli-obk commented Jun 21, 2022

Copy link
Copy Markdown
Contributor

@rustbot author

@rustbotrustbot 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 Jun 21, 2022
@JohnCSimon

JohnCSimon commented Jul 24, 2022

Copy link
Copy Markdown

ok, did another pass. This lgtm now. r=me after a rebase

ping from triage:
@canndrew
Can you please rebase so we can move forward?

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

@JohnCSimon

JohnCSimon commented Sep 11, 2022

Copy link
Copy Markdown

@canndrew

Ping from triage: I'm closing this due to inactivity, Please reopen when you are ready to continue with this.
Note: if you do please open the PR BEFORE you push to it, else you won't be able to reopen - this is a quirk of github.
Thanks for your contribution.

@rustbot label: +S-inactive

@rustbotrustbot added the S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. label Sep 11, 2022
@crlf0710crlf0710 mentioned this pull request Sep 11, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 11, 2022
@bors
Auto merge ofrust-lang#101673- crlf0710:generator_clone, r=oli-obk
6f0c4a6
Allow generators to impl Clone/Copy
Revives rust-lang#95137. It's a pity that the original pr didn't land because the implementation is almost complete! All credits goes to `@canndrew,` and i just resolved the merge conflicts and updated the feature gate version number.
r? `@oli-obk`
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obkoli-obkoli-obk requested changes

Assignees

@oli-obkoli-obk

Labels

S-inactiveStatus: Inactive and waiting on the author. This is often applied to closed PRs.S-waiting-on-authorStatus: This is awaiting some action (such as code changes or more information) from the author.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@canndrew@rust-log-analyzer@bors@oli-obk@JohnCSimon@rust-highfive@bjorn3@rustbot