Add detailed error explanation for E0504 #33386
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nrc (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| This error occurs when an attempt is made to move a borrowed variable into a | ||
| closure. For example: | ||
|
|
||
| ```compile_fail |
There was a problem hiding this comment.
No need to use threads here. It shadows a bit the explanation and makes the example harder than necessary. You could use:
struct FancyNum {
num: u8
}
fn main() {
let fancy_num = FancyNum { num: 5 };
let fancy_ref = &fancy_num;
let x = move || {
println!("child thread: {}", fancy_num.num);
// error: cannot move `fancy_num` into closure because it is borrowed
};
println!("main thread: {}", fancy_ref.num);
}There was a problem hiding this comment.
I wondered about that-- I was trying to give an example that showed a common use of moved closures, but since it sounds like you think it would be clearer without I'll remove it.
There was a problem hiding this comment.
I just hope it won't get in the way for readers. What do you think about this @steveklabnik?
There was a problem hiding this comment.
IMO we should have both examples. This error can crop up a lot in threaded situations, and an extra example addressing it specifically would be nice.
There was a problem hiding this comment.
What do you think about changing the Rc example into an Arc example and using thread::spawn there? It would highlight threads as well as giving a more motivating example for using reference counting (compared with the existing example, which uses reference counting for no reason).
|
Thanks for showing |
|
☔ The latest upstream changes (presumably #33376) made this pull request unmergeable. Please resolve the merge conflicts. |
cramertj
force-pushed
the
E0504
branch
5 times, most recently
from
e33fa5a to
3371b8a
Compare
|
@bors: r+ rollup looks good, thanks! |
|
📌 Commit 3371b8a has been approved by |
Add detailed error explanation for E0504 Part of rust-lang#32777
Add detailed error explanation for E0504 Part of rust-lang#32777
Add detailed error explanation for E0504 Part of rust-lang#32777
Add detailed error explanation for E0504 Part of rust-lang#32777
Add detailed error explanation for E0504 Part of rust-lang#32777
| @@ -386,6 +386,109 @@ fn foo(a: &mut i32) { | |||
| let bar = || { | |||
| inside_closure(a) | |||
| }; | |||
| ``` | |||
There was a problem hiding this comment.
this is causing compilation to fail; it makes the error above invalid. you need to include a closing }
|
@bors: r- see my inline comment; this was failing the rollup |
|
ff1f450 <- something like this would fix this PR |
Removed unnecessary use of threads from E0504 Cleaned up line ending on E0504 Added more examples for E0504 Changed to erroneous code wording Switched Rc example to thread/Arc example Added comments describing why errors no longer occur
|
@steveklabnik Fixed. Sorry about that. |
|
No worries! Thanks 😄 @bors: r+ rollup |
|
📌 Commit 38a5338 has been approved by |
Add detailed error explanation for E0504 Part of rust-lang#32777
Add detailed error explanation for E0504 Part of rust-lang#32777
Add detailed error explanation for E0504 Part of rust-lang#32777
Part of #32777