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

@@ -212,6 +212,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                             &mut err,
                             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 let Some(DesugaringKind::ForLoop(_)) = move_span.desugaring_kind() {
@@ -256,11 +269,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,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
                 // Used in a closure.
                 (LaterUseKind::ClosureCapture, var_span)
             }
-            UseSpans::OtherUse(span) | UseSpans::FnSelfUse { var_span: span, .. } => {
+            UseSpans::PatUse(span)
+            | UseSpans::OtherUse(span)
+            | UseSpans::FnSelfUse { var_span: span, .. } => {
                 let block = &self.body.basic_blocks()[location.block];
 
                 let kind = if let Some(&Statement {

src/librustc_mir/borrow_check/diagnostics/mod.rs

@@ -558,7 +558,9 @@ pub(super) enum UseSpans {
         fn_span: Span,
         kind: FnSelfUseKind,
     },
-    // 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),
 }
 
@@ -577,6 +579,7 @@ impl UseSpans {
         match self {
             UseSpans::ClosureUse { args_span: span, .. }
             | UseSpans::FnSelfUse { var_span: span, .. }
+            | UseSpans::PatUse(span)
             | UseSpans::OtherUse(span) => span,
         }
     }
@@ -585,6 +588,7 @@ impl UseSpans {
         match self {
             UseSpans::ClosureUse { var_span: span, .. }
             | UseSpans::FnSelfUse { var_span: span, .. }
+            | UseSpans::PatUse(span)
             | UseSpans::OtherUse(span) => span,
         }
     }
@@ -655,7 +659,7 @@ impl UseSpans {
         match self {
             closure @ UseSpans::ClosureUse { .. } => closure,
             fn_self @ UseSpans::FnSelfUse { .. } => fn_self,
-            UseSpans::OtherUse(_) => if_other(),
+            UseSpans::PatUse(_) | UseSpans::OtherUse(_) => if_other(),
         }
     }
 }
@@ -772,7 +776,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             }
         }
 
-        let normal_ret = OtherUse(stmt.source_info.span);
+        let normal_ret =
+            if moved_place.projection.iter().any(|p| matches!(p, ProjectionElem::Downcast(..))) {
+                PatUse(stmt.source_info.span)
+            } else {
+                OtherUse(stmt.source_info.span)
+            };
 
         // We are trying to find MIR of the form:
         // ```
@@ -792,6 +801,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             _ => return normal_ret,
         };
 
+        debug!("move_spans: normal_ret = {:?}", normal_ret);
         debug!("move_spans: target_temp = {:?}", target_temp);
 
         if let Some(Terminator { kind: TerminatorKind::Call { func, args, fn_span, .. }, .. }) =

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/pattern/bindings-after-at/bind-by-move-neither-can-live-while-the-other-survives-1.stderr

@@ -46,6 +46,10 @@ LL |         Some(_z @ ref _y) => {}
    |              value moved here
    |
    = note: move occurs because value has type `X`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `x.0`
+   |
+LL |         Some(ref _z @ ref _y) => {}
+   |              ^^^
 
 error[E0382]: borrow of moved value
   --> $DIR/bind-by-move-neither-can-live-while-the-other-survives-1.rs:35:19
@@ -57,6 +61,10 @@ LL |         Some(_z @ ref mut _y) => {}
    |              value moved here
    |
    = note: move occurs because value has type `X`, which does not implement the `Copy` trait
+help: borrow this field in the pattern to avoid moving `x.0`
+   |
+LL |         Some(ref _z @ ref mut _y) => {}
+   |              ^^^
 
 error: aborting due to 6 previous errors