-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Confusing error message with moving a value within a nested for loop #92531
Labels
A-borrow-checker
Area: The borrow checker
A-diagnostics
Area: Messages for errors, warnings, and lints
C-enhancement
Category: An issue proposing an enhancement or a PR with one.
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
@rustbot label +C-enhancement +D-terse +A-borrow-checker |
Current output:
|
estebank
added a commit
to estebank/rust
that referenced
this issue
Feb 26, 2024
Sometimes move errors are because of a misplaced `continue`, but we didn't surface that anywhere. Now when there are more than one set of nested loops we show them out and point at the `continue` and `break` expressions within that might need to go elsewhere. ``` error[E0382]: use of moved value: `foo` --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- | | | this reinitialization might get skipped | move occurs because `foo` has type `String`, which does not implement the `Copy` trait ... LL | for bar in &bars { | ---------------- inside of this loop ... LL | baz.push(foo); | --- value moved here, in previous iteration of loop ... LL | qux.push(foo); | ^^^ value used here after move | note: verify that your loop breaking logic is correct --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 | LL | for foo in foos { | --------------- ... LL | for bar in &bars { | ---------------- ... LL | continue; | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); LL ~ for bar in &bars { LL | ... LL | if foo == *bar { LL ~ value; | help: consider cloning the value if the performance cost is acceptable | LL | baz.push(foo.clone()); | ++++++++ ``` Fix rust-lang#92531.
estebank
added a commit
to estebank/rust
that referenced
this issue
Mar 1, 2024
Sometimes move errors are because of a misplaced `continue`, but we didn't surface that anywhere. Now when there are more than one set of nested loops we show them out and point at the `continue` and `break` expressions within that might need to go elsewhere. ``` error[E0382]: use of moved value: `foo` --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- | | | this reinitialization might get skipped | move occurs because `foo` has type `String`, which does not implement the `Copy` trait ... LL | for bar in &bars { | ---------------- inside of this loop ... LL | baz.push(foo); | --- value moved here, in previous iteration of loop ... LL | qux.push(foo); | ^^^ value used here after move | note: verify that your loop breaking logic is correct --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 | LL | for foo in foos { | --------------- ... LL | for bar in &bars { | ---------------- ... LL | continue; | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); LL ~ for bar in &bars { LL | ... LL | if foo == *bar { LL ~ value; | help: consider cloning the value if the performance cost is acceptable | LL | baz.push(foo.clone()); | ++++++++ ``` Fix rust-lang#92531.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Mar 17, 2024
…ion, r=Nadrieril Detect when move of !Copy value occurs within loop and should likely not be cloned When encountering a move error on a value within a loop of any kind, identify if the moved value belongs to a call expression that should not be cloned and avoid the semantically incorrect suggestion. Also try to suggest moving the call expression outside of the loop instead. ``` error[E0382]: use of moved value: `vec` --> $DIR/recreating-value-in-loop-condition.rs:6:33 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | while let Some(item) = iter(vec).next() { | ----------------------------^^^-------- | | | | | value moved here, in previous iteration of loop | inside of this loop | note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary --> $DIR/recreating-value-in-loop-condition.rs:1:17 | LL | fn iter<T>(vec: Vec<T>) -> impl Iterator<Item = T> { | ---- ^^^^^^ this parameter takes ownership of the value | | | in this function help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ while let Some(item) = value.next() { | ``` We use the presence of a `break` in the loop that would be affected by the moved value as a heuristic for "shouldn't be cloned". Fix rust-lang#121466. --- *Point at continue and break that might be in the wrong place* Sometimes move errors are because of a misplaced `continue`, but we didn't surface that anywhere. Now when there are more than one set of nested loops we show them out and point at the `continue` and `break` expressions within that might need to go elsewhere. ``` error[E0382]: use of moved value: `foo` --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- | | | this reinitialization might get skipped | move occurs because `foo` has type `String`, which does not implement the `Copy` trait ... LL | for bar in &bars { | ---------------- inside of this loop ... LL | baz.push(foo); | --- value moved here, in previous iteration of loop ... LL | qux.push(foo); | ^^^ value used here after move | note: verify that your loop breaking logic is correct --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 | LL | for foo in foos { | --------------- ... LL | for bar in &bars { | ---------------- ... LL | continue; | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); LL ~ for bar in &bars { LL | ... LL | if foo == *bar { LL ~ value; | help: consider cloning the value if the performance cost is acceptable | LL | baz.push(foo.clone()); | ++++++++ ``` Fix rust-lang#92531.
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Mar 18, 2024
…n, r=Nadrieril Detect when move of !Copy value occurs within loop and should likely not be cloned When encountering a move error on a value within a loop of any kind, identify if the moved value belongs to a call expression that should not be cloned and avoid the semantically incorrect suggestion. Also try to suggest moving the call expression outside of the loop instead. ``` error[E0382]: use of moved value: `vec` --> $DIR/recreating-value-in-loop-condition.rs:6:33 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | while let Some(item) = iter(vec).next() { | ----------------------------^^^-------- | | | | | value moved here, in previous iteration of loop | inside of this loop | note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary --> $DIR/recreating-value-in-loop-condition.rs:1:17 | LL | fn iter<T>(vec: Vec<T>) -> impl Iterator<Item = T> { | ---- ^^^^^^ this parameter takes ownership of the value | | | in this function help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ while let Some(item) = value.next() { | ``` We use the presence of a `break` in the loop that would be affected by the moved value as a heuristic for "shouldn't be cloned". Fix rust-lang#121466. --- *Point at continue and break that might be in the wrong place* Sometimes move errors are because of a misplaced `continue`, but we didn't surface that anywhere. Now when there are more than one set of nested loops we show them out and point at the `continue` and `break` expressions within that might need to go elsewhere. ``` error[E0382]: use of moved value: `foo` --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- | | | this reinitialization might get skipped | move occurs because `foo` has type `String`, which does not implement the `Copy` trait ... LL | for bar in &bars { | ---------------- inside of this loop ... LL | baz.push(foo); | --- value moved here, in previous iteration of loop ... LL | qux.push(foo); | ^^^ value used here after move | note: verify that your loop breaking logic is correct --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 | LL | for foo in foos { | --------------- ... LL | for bar in &bars { | ---------------- ... LL | continue; | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); LL ~ for bar in &bars { LL | ... LL | if foo == *bar { LL ~ value; | help: consider cloning the value if the performance cost is acceptable | LL | baz.push(foo.clone()); | ++++++++ ``` Fix rust-lang#92531.
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this issue
Mar 19, 2024
…ieril Detect when move of !Copy value occurs within loop and should likely not be cloned When encountering a move error on a value within a loop of any kind, identify if the moved value belongs to a call expression that should not be cloned and avoid the semantically incorrect suggestion. Also try to suggest moving the call expression outside of the loop instead. ``` error[E0382]: use of moved value: `vec` --> $DIR/recreating-value-in-loop-condition.rs:6:33 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | while let Some(item) = iter(vec).next() { | ----------------------------^^^-------- | | | | | value moved here, in previous iteration of loop | inside of this loop | note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary --> $DIR/recreating-value-in-loop-condition.rs:1:17 | LL | fn iter<T>(vec: Vec<T>) -> impl Iterator<Item = T> { | ---- ^^^^^^ this parameter takes ownership of the value | | | in this function help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ while let Some(item) = value.next() { | ``` We use the presence of a `break` in the loop that would be affected by the moved value as a heuristic for "shouldn't be cloned". Fix rust-lang/rust#121466. --- *Point at continue and break that might be in the wrong place* Sometimes move errors are because of a misplaced `continue`, but we didn't surface that anywhere. Now when there are more than one set of nested loops we show them out and point at the `continue` and `break` expressions within that might need to go elsewhere. ``` error[E0382]: use of moved value: `foo` --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- | | | this reinitialization might get skipped | move occurs because `foo` has type `String`, which does not implement the `Copy` trait ... LL | for bar in &bars { | ---------------- inside of this loop ... LL | baz.push(foo); | --- value moved here, in previous iteration of loop ... LL | qux.push(foo); | ^^^ value used here after move | note: verify that your loop breaking logic is correct --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 | LL | for foo in foos { | --------------- ... LL | for bar in &bars { | ---------------- ... LL | continue; | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); LL ~ for bar in &bars { LL | ... LL | if foo == *bar { LL ~ value; | help: consider cloning the value if the performance cost is acceptable | LL | baz.push(foo.clone()); | ++++++++ ``` Fix rust-lang/rust#92531.
lnicola
pushed a commit
to lnicola/rust-analyzer
that referenced
this issue
Apr 7, 2024
…ieril Detect when move of !Copy value occurs within loop and should likely not be cloned When encountering a move error on a value within a loop of any kind, identify if the moved value belongs to a call expression that should not be cloned and avoid the semantically incorrect suggestion. Also try to suggest moving the call expression outside of the loop instead. ``` error[E0382]: use of moved value: `vec` --> $DIR/recreating-value-in-loop-condition.rs:6:33 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | while let Some(item) = iter(vec).next() { | ----------------------------^^^-------- | | | | | value moved here, in previous iteration of loop | inside of this loop | note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary --> $DIR/recreating-value-in-loop-condition.rs:1:17 | LL | fn iter<T>(vec: Vec<T>) -> impl Iterator<Item = T> { | ---- ^^^^^^ this parameter takes ownership of the value | | | in this function help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ while let Some(item) = value.next() { | ``` We use the presence of a `break` in the loop that would be affected by the moved value as a heuristic for "shouldn't be cloned". Fix rust-lang/rust#121466. --- *Point at continue and break that might be in the wrong place* Sometimes move errors are because of a misplaced `continue`, but we didn't surface that anywhere. Now when there are more than one set of nested loops we show them out and point at the `continue` and `break` expressions within that might need to go elsewhere. ``` error[E0382]: use of moved value: `foo` --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- | | | this reinitialization might get skipped | move occurs because `foo` has type `String`, which does not implement the `Copy` trait ... LL | for bar in &bars { | ---------------- inside of this loop ... LL | baz.push(foo); | --- value moved here, in previous iteration of loop ... LL | qux.push(foo); | ^^^ value used here after move | note: verify that your loop breaking logic is correct --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 | LL | for foo in foos { | --------------- ... LL | for bar in &bars { | ---------------- ... LL | continue; | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); LL ~ for bar in &bars { LL | ... LL | if foo == *bar { LL ~ value; | help: consider cloning the value if the performance cost is acceptable | LL | baz.push(foo.clone()); | ++++++++ ``` Fix rust-lang/rust#92531.
RalfJung
pushed a commit
to RalfJung/rust-analyzer
that referenced
this issue
Apr 27, 2024
…ieril Detect when move of !Copy value occurs within loop and should likely not be cloned When encountering a move error on a value within a loop of any kind, identify if the moved value belongs to a call expression that should not be cloned and avoid the semantically incorrect suggestion. Also try to suggest moving the call expression outside of the loop instead. ``` error[E0382]: use of moved value: `vec` --> $DIR/recreating-value-in-loop-condition.rs:6:33 | LL | let vec = vec!["one", "two", "three"]; | --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait LL | while let Some(item) = iter(vec).next() { | ----------------------------^^^-------- | | | | | value moved here, in previous iteration of loop | inside of this loop | note: consider changing this parameter type in function `iter` to borrow instead if owning the value isn't necessary --> $DIR/recreating-value-in-loop-condition.rs:1:17 | LL | fn iter<T>(vec: Vec<T>) -> impl Iterator<Item = T> { | ---- ^^^^^^ this parameter takes ownership of the value | | | in this function help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = iter(vec); LL ~ while let Some(item) = value.next() { | ``` We use the presence of a `break` in the loop that would be affected by the moved value as a heuristic for "shouldn't be cloned". Fix rust-lang/rust#121466. --- *Point at continue and break that might be in the wrong place* Sometimes move errors are because of a misplaced `continue`, but we didn't surface that anywhere. Now when there are more than one set of nested loops we show them out and point at the `continue` and `break` expressions within that might need to go elsewhere. ``` error[E0382]: use of moved value: `foo` --> $DIR/nested-loop-moved-value-wrong-continue.rs:46:18 | LL | for foo in foos { | --- | | | this reinitialization might get skipped | move occurs because `foo` has type `String`, which does not implement the `Copy` trait ... LL | for bar in &bars { | ---------------- inside of this loop ... LL | baz.push(foo); | --- value moved here, in previous iteration of loop ... LL | qux.push(foo); | ^^^ value used here after move | note: verify that your loop breaking logic is correct --> $DIR/nested-loop-moved-value-wrong-continue.rs:41:17 | LL | for foo in foos { | --------------- ... LL | for bar in &bars { | ---------------- ... LL | continue; | ^^^^^^^^ this `continue` advances the loop at line 33 help: consider moving the expression out of the loop so it is only moved once | LL ~ let mut value = baz.push(foo); LL ~ for bar in &bars { LL | ... LL | if foo == *bar { LL ~ value; | help: consider cloning the value if the performance cost is acceptable | LL | baz.push(foo.clone()); | ++++++++ ``` Fix rust-lang/rust#92531.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-borrow-checker
Area: The borrow checker
A-diagnostics
Area: Messages for errors, warnings, and lints
C-enhancement
Category: An issue proposing an enhancement or a PR with one.
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.
I had code similar to the below (which is a minimal reproduction version of the original code), and its error was very confusing to me, enough so that I had to ask about what was wrong with it on the Rust discord, and even then it took a little while before anyone could get through to me what was even wrong.
Given the following code: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=c437992df178c2e794b09b37cc3b2ea3
The current output is:
The problem was that I meant to
continue
the outer loop, but I had forgotten that I was inside an inner loop. What made it all the more confusing is that it points tofoo
in the outer loop and says "this reinitialization might get skipped," which baffled me, because I couldn't ever imagine a loop variable not being reinitialized when the next iteration comes around.So the combination of "value moved here, in previous iteration of loop" combined with "this reinitialization might get skipped" made no sense to me.
I'm not really sure how this could be improved, but the fact that there are two loops involved makes this diagnostic confusing in my opinion. It might make sense to detect that such a situation is going on and be more careful about the wording, by saying something like "in previous iteration of inner loop."
The text was updated successfully, but these errors were encountered: