Auto merge of #121652 - estebank:move-in-loop-break-condition, 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 https://github.com/rust-lang/rust/issues/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 https://github.com/rust-lang/rust/issues/92531.

7 files changed, 363 insertions, 38 deletions

diff --git a/tests/ui/borrowck/mut-borrow-in-loop-2.fixed b/tests/ui/borrowck/mut-borrow-in-loop-2.fixed
deleted file mode 100644
index cff3c372cdb..00000000000
--- a/tests/ui/borrowck/mut-borrow-in-loop-2.fixed
+++ /dev/null
@@ -1,35 +0,0 @@
-//@ run-rustfix
-#![allow(dead_code)]
-
-struct Events<R>(R);
-
-struct Other;
-
-pub trait Trait<T> {
-    fn handle(value: T) -> Self;
-}
-
-// Blanket impl. (If you comment this out, compiler figures out that it
-// is passing an `&mut` to a method that must be expecting an `&mut`,
-// and injects an auto-reborrow.)
-impl<T, U> Trait<U> for T where T: From<U> {
-    fn handle(_: U) -> Self { unimplemented!() }
-}
-
-impl<'a, R> Trait<&'a mut Events<R>> for Other {
-    fn handle(_: &'a mut Events<R>) -> Self { unimplemented!() }
-}
-
-fn this_compiles<'a, R>(value: &'a mut Events<R>) {
-    for _ in 0..3 {
-        Other::handle(&mut *value);
-    }
-}
-
-fn this_does_not<'a, R>(value: &'a mut Events<R>) {
-    for _ in 0..3 {
-        Other::handle(&mut *value); //~ ERROR use of moved value: `value`
-    }
-}
-
-fn main() {}
diff --git a/tests/ui/borrowck/mut-borrow-in-loop-2.rs b/tests/ui/borrowck/mut-borrow-in-loop-2.rs
index ba79b12042f..f530dfca1a3 100644
--- a/tests/ui/borrowck/mut-borrow-in-loop-2.rs
+++ b/tests/ui/borrowck/mut-borrow-in-loop-2.rs
@@ -1,4 +1,3 @@
-//@ run-rustfix
 #![allow(dead_code)]
 
 struct Events<R>(R);
diff --git a/tests/ui/borrowck/mut-borrow-in-loop-2.stderr b/tests/ui/borrowck/mut-borrow-in-loop-2.stderr
index 7b9a946f3ca..7a569d1da41 100644
--- a/tests/ui/borrowck/mut-borrow-in-loop-2.stderr
+++ b/tests/ui/borrowck/mut-borrow-in-loop-2.stderr
@@ -1,5 +1,5 @@
 error[E0382]: use of moved value: `value`
-  --> $DIR/mut-borrow-in-loop-2.rs:31:23
+  --> $DIR/mut-borrow-in-loop-2.rs:30:23
    |
 LL | fn this_does_not<'a, R>(value: &'a mut Events<R>) {
    |                         ----- move occurs because `value` has type `&mut Events<R>`, which does not implement the `Copy` trait
@@ -9,12 +9,18 @@ LL |         Other::handle(value);
    |                       ^^^^^ value moved here, in previous iteration of loop
    |
 note: consider changing this parameter type in function `handle` to borrow instead if owning the value isn't necessary
-  --> $DIR/mut-borrow-in-loop-2.rs:9:22
+  --> $DIR/mut-borrow-in-loop-2.rs:8:22
    |
 LL |     fn handle(value: T) -> Self;
    |        ------        ^ 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 = Other::handle(value);
+LL ~     for _ in 0..3 {
+LL ~         value;
+   |
 help: consider creating a fresh reborrow of `value` here
    |
 LL |         Other::handle(&mut *value);
diff --git a/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs b/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs
new file mode 100644
index 00000000000..0235b291df5
--- /dev/null
+++ b/tests/ui/moves/nested-loop-moved-value-wrong-continue.rs
@@ -0,0 +1,50 @@
+fn foo() {
+    let foos = vec![String::new()];
+    let bars = vec![""];
+    let mut baz = vec![];
+    let mut qux = vec![];
+    for foo in foos { for bar in &bars { if foo == *bar {
+    //~^ NOTE this reinitialization might get skipped
+    //~| NOTE move occurs because `foo` has type `String`
+    //~| NOTE inside of this loop
+    //~| HELP consider moving the expression out of the loop
+    //~| NOTE in this expansion of desugaring of `for` loop
+        baz.push(foo);
+        //~^ NOTE value moved here
+        //~| HELP consider cloning the value
+        continue;
+        //~^ NOTE verify that your loop breaking logic is correct
+        //~| NOTE this `continue` advances the loop at $DIR/nested-loop-moved-value-wrong-continue.rs:6:23
+    } }
+    qux.push(foo);
+    //~^ ERROR use of moved value
+    //~| NOTE value used here
+    }
+}
+
+fn main() {
+    let foos = vec![String::new()];
+    let bars = vec![""];
+    let mut baz = vec![];
+    let mut qux = vec![];
+    for foo in foos {
+    //~^ NOTE this reinitialization might get skipped
+    //~| NOTE move occurs because `foo` has type `String`
+        for bar in &bars {
+        //~^ NOTE inside of this loop
+        //~| HELP consider moving the expression out of the loop
+        //~| NOTE in this expansion of desugaring of `for` loop
+            if foo == *bar {
+                baz.push(foo);
+                //~^ NOTE value moved here
+                //~| HELP consider cloning the value
+                continue;
+                //~^ NOTE verify that your loop breaking logic is correct
+                //~| NOTE this `continue` advances the loop at line 33
+            }
+        }
+        qux.push(foo);
+        //~^ ERROR use of moved value
+        //~| NOTE value used here
+    }
+}
diff --git a/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr b/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr
new file mode 100644
index 00000000000..3247513d42c
--- /dev/null
+++ b/tests/ui/moves/nested-loop-moved-value-wrong-continue.stderr
@@ -0,0 +1,83 @@
+error[E0382]: use of moved value: `foo`
+  --> $DIR/nested-loop-moved-value-wrong-continue.rs:19:14
+   |
+LL |     for foo in foos { for bar in &bars { if foo == *bar {
+   |         ---           ---------------- inside of this loop
+   |         |
+   |         this reinitialization might get skipped
+   |         move occurs because `foo` has type `String`, which does not implement the `Copy` trait
+...
+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:15:9
+   |
+LL |     for foo in foos { for bar in &bars { if foo == *bar {
+   |     ---------------   ----------------
+...
+LL |         continue;
+   |         ^^^^^^^^ this `continue` advances the loop at $DIR/nested-loop-moved-value-wrong-continue.rs:6:23: 18:8
+help: consider moving the expression out of the loop so it is only moved once
+   |
+LL ~     for foo in foos { let mut value = baz.push(foo);
+LL ~     for bar in &bars { if foo == *bar {
+LL |
+ ...
+LL |
+LL ~         value;
+   |
+help: consider cloning the value if the performance cost is acceptable
+   |
+LL |         baz.push(foo.clone());
+   |                     ++++++++
+
+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());
+   |                             ++++++++
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0382`.
diff --git a/tests/ui/moves/recreating-value-in-loop-condition.rs b/tests/ui/moves/recreating-value-in-loop-condition.rs
new file mode 100644
index 00000000000..ad012ff2978
--- /dev/null
+++ b/tests/ui/moves/recreating-value-in-loop-condition.rs
@@ -0,0 +1,65 @@
+fn iter<T>(vec: Vec<T>) -> impl Iterator<Item = T> {
+    vec.into_iter()
+}
+fn foo() {
+    let vec = vec!["one", "two", "three"];
+    while let Some(item) = iter(vec).next() { //~ ERROR use of moved value
+    //~^ HELP consider moving the expression out of the loop so it is only moved once
+        println!("{:?}", item);
+    }
+}
+fn bar() {
+    let vec = vec!["one", "two", "three"];
+    loop {
+    //~^ HELP consider moving the expression out of the loop so it is only moved once
+        let Some(item) = iter(vec).next() else { //~ ERROR use of moved value
+            break;
+        };
+        println!("{:?}", item);
+    }
+}
+fn baz() {
+    let vec = vec!["one", "two", "three"];
+    loop {
+    //~^ HELP consider moving the expression out of the loop so it is only moved once
+        let item = iter(vec).next(); //~ ERROR use of moved value
+        //~^ HELP consider cloning
+        if item.is_none() {
+            break;
+        }
+        println!("{:?}", item);
+    }
+}
+fn qux() {
+    let vec = vec!["one", "two", "three"];
+    loop {
+    //~^ HELP consider moving the expression out of the loop so it is only moved once
+        if let Some(item) = iter(vec).next() { //~ ERROR use of moved value
+            println!("{:?}", item);
+            break;
+        }
+    }
+}
+fn zap() {
+    loop {
+        let vec = vec!["one", "two", "three"];
+        loop {
+        //~^ HELP consider moving the expression out of the loop so it is only moved once
+            loop {
+                loop {
+                    if let Some(item) = iter(vec).next() { //~ ERROR use of moved value
+                        println!("{:?}", item);
+                        break;
+                    }
+                }
+            }
+        }
+    }
+}
+fn main() {
+    foo();
+    bar();
+    baz();
+    qux();
+    zap();
+}
diff --git a/tests/ui/moves/recreating-value-in-loop-condition.stderr b/tests/ui/moves/recreating-value-in-loop-condition.stderr
new file mode 100644
index 00000000000..75c9633bc0a
--- /dev/null
+++ b/tests/ui/moves/recreating-value-in-loop-condition.stderr
@@ -0,0 +1,157 @@
+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() {
+   |
+
+error[E0382]: use of moved value: `vec`
+  --> $DIR/recreating-value-in-loop-condition.rs:15:31
+   |
+LL |     let vec = vec!["one", "two", "three"];
+   |         --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait
+LL |     loop {
+   |     ---- inside of this loop
+LL |
+LL |         let Some(item) = iter(vec).next() else {
+   |                               ^^^ value moved here, in previous iteration of 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 ~     loop {
+LL |
+LL ~         let Some(item) = value.next() else {
+   |
+
+error[E0382]: use of moved value: `vec`
+  --> $DIR/recreating-value-in-loop-condition.rs:25:25
+   |
+LL |     let vec = vec!["one", "two", "three"];
+   |         --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait
+LL |     loop {
+   |     ---- inside of this loop
+LL |
+LL |         let item = iter(vec).next();
+   |                         ^^^ value moved here, in previous iteration of 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 ~     loop {
+LL |
+LL ~         let item = value.next();
+   |
+help: consider cloning the value if the performance cost is acceptable
+   |
+LL |         let item = iter(vec.clone()).next();
+   |                            ++++++++
+
+error[E0382]: use of moved value: `vec`
+  --> $DIR/recreating-value-in-loop-condition.rs:37:34
+   |
+LL |     let vec = vec!["one", "two", "three"];
+   |         --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait
+LL |     loop {
+   |     ---- inside of this loop
+LL |
+LL |         if let Some(item) = iter(vec).next() {
+   |                                  ^^^ value moved here, in previous iteration of 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 ~     loop {
+LL |
+LL ~         if let Some(item) = value.next() {
+   |
+
+error[E0382]: use of moved value: `vec`
+  --> $DIR/recreating-value-in-loop-condition.rs:50:46
+   |
+LL |         let vec = vec!["one", "two", "three"];
+   |             --- move occurs because `vec` has type `Vec<&str>`, which does not implement the `Copy` trait
+LL |         loop {
+   |         ---- inside of this loop
+LL |
+LL |             loop {
+   |             ---- inside of this loop
+LL |                 loop {
+   |                 ---- inside of this loop
+LL |                     if let Some(item) = iter(vec).next() {
+   |                                              ^^^ value moved here, in previous iteration of 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
+note: verify that your loop breaking logic is correct
+  --> $DIR/recreating-value-in-loop-condition.rs:52:25
+   |
+LL |     loop {
+   |     ----
+LL |         let vec = vec!["one", "two", "three"];
+LL |         loop {
+   |         ----
+LL |
+LL |             loop {
+   |             ----
+LL |                 loop {
+   |                 ----
+...
+LL |                         break;
+   |                         ^^^^^ this `break` exits the loop at line 49
+help: consider moving the expression out of the loop so it is only moved once
+   |
+LL ~         let mut value = iter(vec);
+LL ~         loop {
+LL |
+LL |             loop {
+LL |                 loop {
+LL ~                     if let Some(item) = value.next() {
+   |
+
+error: aborting due to 5 previous errors
+
+For more information about this error, try `rustc --explain E0382`.