Rollup merge of rust-lang#73534 - estebank:borrowck-suggestions, r=matthewjasper

Provide suggestions for some moved value errors

When encountering an used moved value where the previous move happened
in a `match` or `if let` pattern, suggest using `ref`. Fix rust-lang#63988.

When encountering a `&mut` value that is used in multiple iterations of
a loop, suggest reborrowing it with `&mut *`. Fix rust-lang#62112.

Author: Manishearth

src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs

@@ -156,6 +156,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                         format!("variable moved due to use{}", move_spans.describe()),
                     );
                 }
+                if let UseSpans::PatUse(span) = move_spans {
+                    err.span_suggestion_verbose(
+                        span.shrink_to_lo(),
+                        &format!(
+                            "borrow this field in the pattern to avoid moving {}",
+                            self.describe_place(moved_place.as_ref())
+                                .map(|n| format!("`{}`", n))
+                                .unwrap_or_else(|| "the value".to_string())
+                        ),
+                        "ref ".to_string(),
+                        Applicability::MachineApplicable,
+                    );
+                }
+
                 if Some(DesugaringKind::ForLoop) == move_span.desugaring_kind() {
                     let sess = self.infcx.tcx.sess;
                     if let Ok(snippet) = sess.source_map().span_to_snippet(move_span) {
@@ -198,11 +212,28 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 _ => true,
             };
 
-            if needs_note {
-                let mpi = self.move_data.moves[move_out_indices[0]].path;
-                let place = &self.move_data.move_paths[mpi].place;
+            let mpi = self.move_data.moves[move_out_indices[0]].path;
+            let place = &self.move_data.move_paths[mpi].place;
+            let ty = place.ty(self.body, self.infcx.tcx).ty;
+
+            if is_loop_move {
+                if let ty::Ref(_, _, hir::Mutability::Mut) = ty.kind {
+                    // We have a `&mut` ref, we need to reborrow on each iteration (#62112).
+                    err.span_suggestion_verbose(
+                        span.shrink_to_lo(),
+                        &format!(
+                            "consider creating a fresh reborrow of {} here",
+                            self.describe_place(moved_place)
+                                .map(|n| format!("`{}`", n))
+                                .unwrap_or_else(|| "the mutable reference".to_string()),
+                        ),
+                        "&mut *".to_string(),
+                        Applicability::MachineApplicable,
+                    );
+                }
+            }
 
-                let ty = place.ty(self.body, self.infcx.tcx).ty;
+            if needs_note {
                 let opt_name =
                     self.describe_place_with_options(place.as_ref(), IncludingDowncast(true));
                 let note_msg = match opt_name {

src/librustc_mir/borrow_check/diagnostics/explain_borrow.rs

@@ -509,7 +509,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 // Used in a closure.
                 (LaterUseKind::ClosureCapture, var_span)
             }
-            UseSpans::OtherUse(span) => {
+            UseSpans::PatUse(span) | UseSpans::OtherUse(span) => {
                 let block = &self.body.basic_blocks()[location.block];
 
                 let kind = if let Some(&Statement {

src/librustc_mir/borrow_check/diagnostics/mod.rs

@@ -542,20 +542,26 @@ pub(super) enum UseSpans {
         // The span of the first use of the captured variable inside the closure.
         var_span: Span,
     },
-    // This access has a single span associated to it: common case.
+    /// This access is caused by a `match` or `if let` pattern.
+    PatUse(Span),
+    /// This access has a single span associated to it: common case.
     OtherUse(Span),
 }
 
 impl UseSpans {
     pub(super) fn args_or_use(self) -> Span {
         match self {
-            UseSpans::ClosureUse { args_span: span, .. } | UseSpans::OtherUse(span) => span,
+            UseSpans::ClosureUse { args_span: span, .. }
+            | UseSpans::PatUse(span)
+            | UseSpans::OtherUse(span) => span,
         }
     }
 
     pub(super) fn var_or_use(self) -> Span {
         match self {
-            UseSpans::ClosureUse { var_span: span, .. } | UseSpans::OtherUse(span) => span,
+            UseSpans::ClosureUse { var_span: span, .. }
+            | UseSpans::PatUse(span)
+            | UseSpans::OtherUse(span) => span,
         }
     }
 
@@ -624,7 +630,7 @@ impl UseSpans {
     {
         match self {
             closure @ UseSpans::ClosureUse { .. } => closure,
-            UseSpans::OtherUse(_) => if_other(),
+            UseSpans::PatUse(_) | UseSpans::OtherUse(_) => if_other(),
         }
     }
 }
@@ -741,7 +747,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             }
         }
 
-        OtherUse(stmt.source_info.span)
+        if moved_place.projection.iter().any(|p| matches!(p, ProjectionElem::Downcast(..))) {
+            PatUse(stmt.source_info.span)
+        } else {
+            OtherUse(stmt.source_info.span)
+        }
     }
 
     /// Finds the span of arguments of a closure (within `maybe_closure_span`)

src/test/ui/borrowck/issue-41962.stderr

@@ -5,6 +5,10 @@ LL |         if let Some(thing) = maybe {
    |                     ^^^^^ value moved here, in previous iteration of loop
    |
    = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `maybe.0`
+   |
+LL |         if let Some(ref thing) = maybe {
+   |                     ^^^
 
 error: aborting due to previous error
 

src/test/ui/borrowck/move-in-pattern-mut.rs

@@ -0,0 +1,23 @@
+// Issue #63988
+#[derive(Debug)]
+struct S;
+fn foo(_: Option<S>) {}
+
+enum E {
+    V {
+        s: S,
+    }
+}
+fn bar(_: E) {}
+
+fn main() {
+    let s = Some(S);
+    if let Some(mut x) = s {
+        x = S;
+    }
+    foo(s); //~ ERROR use of moved value: `s`
+    let mut e = E::V { s: S };
+    let E::V { s: mut x } = e;
+    x = S;
+    bar(e); //~ ERROR use of moved value: `e`
+}

src/test/ui/borrowck/move-in-pattern-mut.stderr

@@ -0,0 +1,33 @@
+error[E0382]: use of moved value: `s`
+  --> $DIR/move-in-pattern-mut.rs:18:9
+   |
+LL |     if let Some(mut x) = s {
+   |                 ----- value moved here
+...
+LL |     foo(s);
+   |         ^ value used here after partial move
+   |
+   = note: move occurs because value has type `S`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `s.0`
+   |
+LL |     if let Some(ref mut x) = s {
+   |                 ^^^
+
+error[E0382]: use of moved value: `e`
+  --> $DIR/move-in-pattern-mut.rs:22:9
+   |
+LL |     let E::V { s: mut x } = e;
+   |                   ----- value moved here
+LL |     x = S;
+LL |     bar(e);
+   |         ^ value used here after partial move
+   |
+   = note: move occurs because value has type `S`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `e.s`
+   |
+LL |     let E::V { s: ref mut x } = e;
+   |                   ^^^
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0382`.

src/test/ui/borrowck/move-in-pattern.fixed

@@ -0,0 +1,24 @@
+// run-rustfix
+// Issue #63988
+#[derive(Debug)]
+struct S;
+fn foo(_: Option<S>) {}
+
+enum E {
+    V {
+        s: S,
+    }
+}
+fn bar(_: E) {}
+
+fn main() {
+    let s = Some(S);
+    if let Some(ref x) = s {
+        let _ = x;
+    }
+    foo(s); //~ ERROR use of moved value: `s`
+    let e = E::V { s: S };
+    let E::V { s: ref x } = e;
+    let _ = x;
+    bar(e); //~ ERROR use of moved value: `e`
+}

src/test/ui/borrowck/move-in-pattern.rs

@@ -0,0 +1,24 @@
+// run-rustfix
+// Issue #63988
+#[derive(Debug)]
+struct S;
+fn foo(_: Option<S>) {}
+
+enum E {
+    V {
+        s: S,
+    }
+}
+fn bar(_: E) {}
+
+fn main() {
+    let s = Some(S);
+    if let Some(x) = s {
+        let _ = x;
+    }
+    foo(s); //~ ERROR use of moved value: `s`
+    let e = E::V { s: S };
+    let E::V { s: x } = e;
+    let _ = x;
+    bar(e); //~ ERROR use of moved value: `e`
+}

src/test/ui/borrowck/move-in-pattern.stderr

@@ -0,0 +1,33 @@
+error[E0382]: use of moved value: `s`
+  --> $DIR/move-in-pattern.rs:19:9
+   |
+LL |     if let Some(x) = s {
+   |                 - value moved here
+...
+LL |     foo(s);
+   |         ^ value used here after partial move
+   |
+   = note: move occurs because value has type `S`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `s.0`
+   |
+LL |     if let Some(ref x) = s {
+   |                 ^^^
+
+error[E0382]: use of moved value: `e`
+  --> $DIR/move-in-pattern.rs:23:9
+   |
+LL |     let E::V { s: x } = e;
+   |                   - value moved here
+LL |     let _ = x;
+LL |     bar(e);
+   |         ^ value used here after partial move
+   |
+   = note: move occurs because value has type `S`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `e.s`
+   |
+LL |     let E::V { s: ref x } = e;
+   |                   ^^^
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0382`.

src/test/ui/borrowck/mut-borrow-in-loop-2.fixed

@@ -0,0 +1,35 @@
+// 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() {}

src/test/ui/borrowck/mut-borrow-in-loop-2.rs

@@ -0,0 +1,35 @@
+// 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(value); //~ ERROR use of moved value: `value`
+    }
+}
+
+fn main() {}

src/test/ui/borrowck/mut-borrow-in-loop-2.stderr

@@ -0,0 +1,17 @@
+error[E0382]: use of moved value: `value`
+  --> $DIR/mut-borrow-in-loop-2.rs:31: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
+LL |     for _ in 0..3 {
+LL |         Other::handle(value);
+   |                       ^^^^^ value moved here, in previous iteration of loop
+   |
+help: consider creating a fresh reborrow of `value` here
+   |
+LL |         Other::handle(&mut *value);
+   |                       ^^^^^^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0382`.

src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.stderr

@@ -8,6 +8,10 @@ LL |     consume(node) + r
    |             ^^^^ value used here after partial move
    |
    = note: move occurs because value has type `std::boxed::Box<List>`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `node.next.0`
+   |
+LL |         Some(ref right) => consume(right),
+   |              ^^^
 
 error: aborting due to previous error
 

src/test/ui/nll/issue-53807.stderr

@@ -5,6 +5,10 @@ LL |         if let Some(thing) = maybe {
    |                     ^^^^^ value moved here, in previous iteration of loop
    |
    = note: move occurs because value has type `std::vec::Vec<bool>`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `maybe.0`
+   |
+LL |         if let Some(ref thing) = maybe {
+   |                     ^^^
 
 error: aborting due to previous error