-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Implement Copy
/Clone
for async closures
#128201
Conversation
Could not assign reviewer from: |
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
Please add a test where a coroutine closure can't be cloned, but Just thinking out loud, but lmk if anything is wrong in my reasoning: Coroutine closures are slightly different from regular closures due to the lending behaviour, but since that is handled by borrowck, I don't see any issues. It's very different from coroutines that can't be cloned soundly anymore after ending up with self-borrows. |
For the purposes of cloning, coroutine-closures are actually precisely the same as regular closures, specifically in the aspect that I'll add a few test. |
7930c49
to
bc2d427
Compare
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
I added a "can't clone closure" test, and fleshed out the PR descr. |
This comment has been minimized.
This comment has been minimized.
bc2d427
to
5a9959f
Compare
Rollup of 9 pull requests Successful merges: - rust-lang#124941 (Stabilize const `{integer}::from_str_radix` i.e. `const_int_from_str`) - rust-lang#128201 (Implement `Copy`/`Clone` for async closures) - rust-lang#128210 (rustdoc: change title of search results) - rust-lang#128223 (Refactor complex conditions in `collect_tokens_trailing_token`) - rust-lang#128224 (Remove unnecessary range replacements) - rust-lang#128226 (Remove redundant option that was just encoding that a slice was empty) - rust-lang#128227 (CI: do not respect custom try jobs for unrolled perf builds) - rust-lang#128229 (Improve `extern "<abi>" unsafe fn()` error message) - rust-lang#128235 (Fix `Iterator::filter` docs) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#128201 - compiler-errors:closure-clone, r=oli-obk Implement `Copy`/`Clone` for async closures We can do so in the same cases that regular closures do. For the purposes of cloning, coroutine-closures are actually precisely the same as regular closures, specifically in the aspect that `Clone` impls care about which is the upvars. The only difference b/w coroutine-closures and regular closures is the type that they *return*, but this type has not been *created* yet, so we don't really have a problem. IDK why I didn't add this impl initially -- I went back and forth a bit on the internal representation for coroutine-closures before settling on a design which largely models regular closures. Previous (not published) iterations of coroutine-closures used to be represented as a special (read: cursed) kind of coroutine, which would probably suffer from the pitfalls that coroutines have that oli mentioned below in rust-lang#128201 (comment). r? oli-obk
We can do so in the same cases that regular closures do.
For the purposes of cloning, coroutine-closures are actually precisely the same as regular closures, specifically in the aspect that
Clone
impls care about which is the upvars. The only difference b/w coroutine-closures and regular closures is the type that they return, but this type has not been created yet, so we don't really have a problem.IDK why I didn't add this impl initially -- I went back and forth a bit on the internal representation for coroutine-closures before settling on a design which largely models regular closures. Previous (not published) iterations of coroutine-closures used to be represented as a special (read: cursed) kind of coroutine, which would probably suffer from the pitfalls that coroutines have that oli mentioned below in #128201 (comment).
r? oli-obk