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

Rustc identifies use of moved value correctly in while let loop match correctly, but the suggested fix results in infinite loop. #121466

Closed
philjb opened this issue Feb 22, 2024 · 1 comment · Fixed by #121652
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@philjb
Copy link

philjb commented Feb 22, 2024

Code

use futures::StreamExt;

#[tokio::main]
async fn main() {
        let vec = vec!["one", "two", "three"];
        while let Some(item) = futures::stream::iter(vec).next().await {
            println!("{:?}", item);
        }
}

Current output

Compiling playground v0.0.1 (/playground)
error[E0382]: use of moved value: `vec`
 --> src/lib.rs:6:54
  |
5 |         let vec = vec!["one", "two", "three"];
  |             --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait
6 |         while let Some(item) = futures::stream::iter(vec).next().await {
  |         ---------------------------------------------^^^--------------
  |         |                                            |
  |         |                                            value moved here, in previous iteration of loop
  |         inside of this loop
  |
help: consider cloning the value if the performance cost is acceptable
  |
6 |         while let Some(item) = futures::stream::iter(vec.clone()).next().await {
  |                                                         ++++++++

For more information about this error, try `rustc --explain E0382`.
error: could not compile `playground` (lib) due to 1 previous error

Desired output

A slightly different example:


use futures::StreamExt;

#[tokio::main]
async fn main() {
        let vec = vec!["one", "two", "three"];
        while let item = futures::stream::iter(vec).next().await {
            println!("{:?}", item);
        }
}

produces an "irrefutable loop" warning at least

Standard Error
   Compiling playground v0.0.1 (/playground)
warning: irrefutable `while let` pattern
 --> src/lib.rs:6:15
  |
6 |         while let item = futures::stream::iter(vec).next().await {
  |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: this pattern will always match, so the loop will never exit
  = help: consider instead using a `loop { ... }` with a `let` inside it
  = note: `#[warn(irrefutable_let_patterns)]` on by default

error[E0382]: use of moved value: `vec`
 --> src/lib.rs:6:48
  |
5 |         let vec = vec!["one", "two", "three"];
  |             --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait
6 |         while let item = futures::stream::iter(vec).next().await {
  |         ---------------------------------------^^^--------------
  |         |                                      |
  |         |                                      value moved here, in previous iteration of loop
  |         inside of this loop
  |
help: consider cloning the value if the performance cost is acceptable
  |
6 |         while let item = futures::stream::iter(vec.clone()).next().await {
  |                                                   ++++++++

For more information about this error, try `rustc --explain E0382`.
warning: `playground` (lib) generated 1 warning
error: could not compile `playground` (lib) due to 1 previous error; 1 warning emitted
Standard Output

Rationale and extra context

I'm a rust beginner: When I mistakenly wrote code matching irrefutable_let_patterns the warning was helpful to me that I had made a mistake.

But I still had a flaw in that I should have made the stream outside the while let match stanza - The code is re-initializain the loop variable on each iteration. The rustc suggestion to add a clone() on vec as vec was moved in the previous loop iteration is "technically correct" but the suggestion creates another irrefutable loop which doesn't produce such a warning. I know this pattern is harder to recognize as it involves the r-value instead of the match side (while let x =).

while let Some(item) = futures::stream::iter(vec.clone()).next().await

Since I'm a beginner I didn't realize that when rustc said value moved here, in previous iteration of loop that I had put use of vec in the wrong place. It should have been.

let mut s = futures::stream::iter(vec);
while let Some(item) = s.next().await {
...

While I ran into this when using an async stream, I think it a general situation whenever the right hand side creates a new and same object on the right side. Here's an example with an iterator along.

fn main() {
        let vec = vec!["one", "two", "three"];
        while let Some(item) = vec.iter().next() {
            println!("{:?}", item);
        }
}

It's a problem in the class of loop variable issues. I come from golang which says a similar issue that everone hits once or twice (and hopefully recognizes when!). Goland is introducing a way to systematically addres this class of error: https://go.dev/blog/loopvar-preview

I was encourage to report this by @carols10cents.

Other cases

No response

Rust Version

1.76.0 - I used the playground to reproduce it.

Anything else?

No response

@philjb philjb 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 Feb 22, 2024
@philjb philjb changed the title Rustc identifies us of moved value correctly in while let loop match correctly, but the fix results in infinite loop. Rustc identifies use of moved value correctly in while let loop match correctly, but the suggested fix results in infinite loop. Feb 22, 2024
@philjb
Copy link
Author

philjb commented Feb 22, 2024

Possibly related other issues I found

@jieyouxu jieyouxu added D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue labels Feb 22, 2024
@estebank estebank self-assigned this Feb 22, 2024
estebank added a commit to estebank/rust that referenced this issue Feb 26, 2024
…ely 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.
estebank added a commit to estebank/rust that referenced this issue Mar 1, 2024
…ely 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.
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.
@bors bors closed this as completed in 14473ad Mar 18, 2024
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-diagnostics Area: Messages for errors, warnings, and lints D-incorrect Diagnostics: A diagnostic that is giving misleading or incorrect information. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue 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