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

Suggest cloning Arc/Rc #104232

Open
ibraheemdev opened this issue Nov 10, 2022 · 6 comments · May be fixed by #124595
Open

Suggest cloning Arc/Rc #104232

ibraheemdev opened this issue Nov 10, 2022 · 6 comments · May be fixed by #124595
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ibraheemdev
Copy link
Member

ibraheemdev commented Nov 10, 2022

fn main() {
    let x = Arc::new(1);
    foo(x);
    bar(x);
}

fn foo(_: Arc<usize>) {}
fn bar(_: Arc<usize>) {}
error[[E0382]](https://doc.rust-lang.org/stable/error-index.html#E0382): use of moved value: `x`
 --> src/main.rs:6:9
  |
4 |     let x = Arc::new(1);
  |         - move occurs because `x` has type `std::sync::Arc<usize>`, which does not implement the `Copy` trait
5 |     foo(x);
  |         - value moved here
6 |     bar(x);
  |         ^ value used here after move

The help message could mention that Arc implements Clone. We don't usually suggest cloning values in diagnostics, but for Arc/Rc we probably should.

Another common case (this one is probably harder to suggest a fix for):

fn main() {
    let x = Arc::new(1);
    for _ in 0..4 {
        std::thread::spawn(move || {
            println!("{}", x);
        });
    }
}
error[[E0382]](https://doc.rust-lang.org/stable/error-index.html#E0382): use of moved value: `x`
 --> src/main.rs:6:28
  |
4 |     let x = Arc::new(1);
  |         - move occurs because `x` has type `std::sync::Arc<i32>`, which does not implement the `Copy` trait
5 |     for _ in 0..4 {
6 |         std::thread::spawn(move || {
  |                            ^^^^^^^ value moved into closure here, in previous iteration of loop
7 |             println!("{}", x);
  |                            - use occurs due to use in closure
@ibraheemdev ibraheemdev added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 10, 2022
@chenyukang
Copy link
Member

@rustbot claim

@estebank
Copy link
Contributor

@chenyukang make sure to account for Rc as well! :)

@chenyukang
Copy link
Member

This is covered by #103908, could be closed now.

@estebank
Copy link
Contributor

@chenyukang oh! Great to hear, sorry for making you waste your time :-/

There are a couple of improvements I would like to make on top of that merged PR though, which you might be interested in doing: I would like to customize the message depending on the type size (if smaller than lets say a KB do not talk about perf cost), whether the type implements Drop (to mention that the drop guard might change what is being done) and special case Arc and Rc and tell people to clone without any equivocation. We might want to use this ticket to track that work?

@estebank
Copy link
Contributor

Triage: The first case is now handled, but the second case isn't.

@estebank estebank added D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. labels Jan 13, 2023
estebank added a commit to estebank/rust that referenced this issue May 1, 2024
```
error[E0382]: borrow of moved value: `x`
  --> $DIR/moves-based-on-type-capture-clause-bad.rs:9:20
   |
LL |     let x = "Hello world!".to_string();
   |         - move occurs because `x` has type `String`, which does not implement the `Copy` trait
LL |     thread::spawn(move || {
   |                   ------- value moved into closure here
LL |         println!("{}", x);
   |                        - variable moved due to use in closure
LL |     });
LL |     println!("{}", x);
   |                    ^ value borrowed here after move
   |
   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: clone the value before moving it into the closure
   |
LL ~     let value = x.clone();
LL ~     thread::spawn(move || {
LL ~         println!("{}", value);
   |
```

Fix rust-lang#104232.
@estebank estebank linked a pull request May 1, 2024 that will close this issue
@estebank
Copy link
Contributor

estebank commented May 1, 2024

Current output:

error[E0382]: use of moved value: `x`
 --> src/main.rs:5:9
  |
3 |     let x = Arc::new(1);
  |         - move occurs because `x` has type `Arc<usize>`, which does not implement the `Copy` trait
4 |     foo(x);
  |         - value moved here
5 |     bar(x);
  |         ^ value used here after move
  |
note: consider changing this parameter type in function `foo` to borrow instead if owning the value isn't necessary
 --> src/main.rs:8:11
  |
8 | fn foo(_: Arc<usize>) {}
  |    ---    ^^^^^^^^^^ this parameter takes ownership of the value
  |    |
  |    in this function
help: clone the value to increment its reference count
  |
4 |     foo(x.clone());
  |          ++++++++
error[E0382]: use of moved value: `x`
 --> src/main.rs:5:28
  |
3 |     let x = Arc::new(1);
  |         - move occurs because `x` has type `Arc<i32>`, which does not implement the `Copy` trait
4 |     for _ in 0..4 {
  |     ------------- inside of this loop
5 |         std::thread::spawn(move || {
  |                            ^^^^^^^ value moved into closure here, in previous iteration of loop
6 |             println!("{}", x);
  |                            - use occurs due to use in closure

After #124595, the output will be

error[E0382]: use of moved value: `qwer`
 --> f203.rs:5:28
  |
3 |     let qwer = Arc::new(1);
  |         ---- move occurs because `qwer` has type `Arc<i32>`, which does not implement the `Copy` trait
4 |     for _ in 0..4 {
  |     ------------- inside of this loop
5 |         std::thread::spawn(move || {
  |                            ^^^^^^^ value moved into closure here, in previous iteration of loop
6 |             println!("{}", qwer);
  |                            ---- use occurs due to use in closure
  |
help: clone the value before moving it into the closure
  |
5 ~         let value = qwer.clone();
6 ~         std::thread::spawn(move || {
7 ~             println!("{}", value);
  |

estebank added a commit to estebank/rust that referenced this issue May 1, 2024
```
error[E0382]: borrow of moved value: `x`
  --> $DIR/moves-based-on-type-capture-clause-bad.rs:9:20
   |
LL |     let x = "Hello world!".to_string();
   |         - move occurs because `x` has type `String`, which does not implement the `Copy` trait
LL |     thread::spawn(move || {
   |                   ------- value moved into closure here
LL |         println!("{}", x);
   |                        - variable moved due to use in closure
LL |     });
LL |     println!("{}", x);
   |                    ^ value borrowed here after move
   |
   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: clone the value before moving it into the closure
   |
LL ~     let value = x.clone();
LL ~     thread::spawn(move || {
LL ~         println!("{}", value);
   |
```

Fix rust-lang#104232.
estebank added a commit to estebank/rust that referenced this issue May 1, 2024
```
error[E0382]: borrow of moved value: `x`
  --> $DIR/moves-based-on-type-capture-clause-bad.rs:9:20
   |
LL |     let x = "Hello world!".to_string();
   |         - move occurs because `x` has type `String`, which does not implement the `Copy` trait
LL |     thread::spawn(move || {
   |                   ------- value moved into closure here
LL |         println!("{}", x);
   |                        - variable moved due to use in closure
LL |     });
LL |     println!("{}", x);
   |                    ^ value borrowed here after move
   |
   = note: this error originates in the macro `$crate::format_args_nl` which comes from the expansion of the macro `println` (in Nightly builds, run with -Z macro-backtrace for more info)
help: clone the value before moving it into the closure
   |
LL ~     let value = x.clone();
LL ~     thread::spawn(move || {
LL ~         println!("{}", value);
   |
```

Fix rust-lang#104232.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants