Rollup merge of rust-lang#105476 - estebank:moves-n-borrows, r=compiler-errors

Change pattern borrowing suggestions to be verbose and remove invalid suggestion

Synthesize a more accurate span and use verbose suggestion output to
make the message clearer.

Do not suggest borrowing binding in pattern in let else. Fix rust-lang#104838.

Author: matthiaskrgr

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -4,7 +4,7 @@ use rustc_middle::ty;
 use rustc_mir_dataflow::move_paths::{
     IllegalMoveOrigin, IllegalMoveOriginKind, LookupResult, MoveError, MovePathIndex,
 };
-use rustc_span::Span;
+use rustc_span::{BytePos, Span};
 
 use crate::diagnostics::{DescribePlaceOpt, UseSpans};
 use crate::prefixes::PrefixSet;
@@ -148,7 +148,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         match_span: Span,
         statement_span: Span,
     ) {
-        debug!("append_binding_error(match_place={:?}, match_span={:?})", match_place, match_span);
+        debug!(?match_place, ?match_span, "append_binding_error");
 
         let from_simple_let = match_place.is_none();
         let match_place = match_place.unwrap_or(move_from);
@@ -160,7 +160,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     if let GroupedMoveError::MovesFromPlace { span, binds_to, .. } = ge
                         && match_span == *span
                     {
-                        debug!("appending local({:?}) to list", bind_to);
+                        debug!("appending local({bind_to:?}) to list");
                         if !binds_to.is_empty() {
                             binds_to.push(bind_to);
                         }
@@ -198,7 +198,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     } = ge
                     {
                         if match_span == *span && mpi == *other_mpi {
-                            debug!("appending local({:?}) to list", bind_to);
+                            debug!("appending local({bind_to:?}) to list");
                             binds_to.push(bind_to);
                             return;
                         }
@@ -410,15 +410,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
     fn add_move_hints(&self, error: GroupedMoveError<'tcx>, err: &mut Diagnostic, span: Span) {
         match error {
             GroupedMoveError::MovesFromPlace { mut binds_to, move_from, .. } => {
-                if let Ok(snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(span) {
-                    err.span_suggestion(
-                        span,
-                        "consider borrowing here",
-                        format!("&{snippet}"),
-                        Applicability::Unspecified,
-                    );
-                }
-
+                self.add_borrow_suggestions(err, span);
                 if binds_to.is_empty() {
                     let place_ty = move_from.ty(self.body, self.infcx.tcx).ty;
                     let place_desc = match self.describe_place(move_from.as_ref()) {
@@ -461,39 +453,75 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         }
     }
 
+    fn add_borrow_suggestions(&self, err: &mut Diagnostic, span: Span) {
+        match self.infcx.tcx.sess.source_map().span_to_snippet(span) {
+            Ok(snippet) if snippet.starts_with('*') => {
+                err.span_suggestion_verbose(
+                    span.with_hi(span.lo() + BytePos(1)),
+                    "consider removing the dereference here",
+                    String::new(),
+                    Applicability::MaybeIncorrect,
+                );
+            }
+            _ => {
+                err.span_suggestion_verbose(
+                    span.shrink_to_lo(),
+                    "consider borrowing here",
+                    "&".to_string(),
+                    Applicability::MaybeIncorrect,
+                );
+            }
+        }
+    }
+
     fn add_move_error_suggestions(&self, err: &mut Diagnostic, binds_to: &[Local]) {
-        let mut suggestions: Vec<(Span, &str, String)> = Vec::new();
+        let mut suggestions: Vec<(Span, String, String)> = Vec::new();
         for local in binds_to {
             let bind_to = &self.body.local_decls[*local];
             if let Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(
                 VarBindingForm { pat_span, .. },
             )))) = bind_to.local_info
             {
-                if let Ok(pat_snippet) = self.infcx.tcx.sess.source_map().span_to_snippet(pat_span)
+                let Ok(pat_snippet) =
+                    self.infcx.tcx.sess.source_map().span_to_snippet(pat_span) else { continue; };
+                let Some(stripped) = pat_snippet.strip_prefix('&') else {
+                    suggestions.push((
+                        bind_to.source_info.span.shrink_to_lo(),
+                        "consider borrowing the pattern binding".to_string(),
+                        "ref ".to_string(),
+                    ));
+                    continue;
+                };
+                let inner_pat_snippet = stripped.trim_start();
+                let (pat_span, suggestion, to_remove) = if inner_pat_snippet.starts_with("mut")
+                    && inner_pat_snippet["mut".len()..].starts_with(rustc_lexer::is_whitespace)
                 {
-                    if let Some(stripped) = pat_snippet.strip_prefix('&') {
-                        let pat_snippet = stripped.trim_start();
-                        let (suggestion, to_remove) = if pat_snippet.starts_with("mut")
-                            && pat_snippet["mut".len()..].starts_with(rustc_lexer::is_whitespace)
-                        {
-                            (pat_snippet["mut".len()..].trim_start(), "&mut")
-                        } else {
-                            (pat_snippet, "&")
-                        };
-                        suggestions.push((pat_span, to_remove, suggestion.to_owned()));
-                    }
-                }
+                    let inner_pat_snippet = inner_pat_snippet["mut".len()..].trim_start();
+                    let pat_span = pat_span.with_hi(
+                        pat_span.lo()
+                            + BytePos((pat_snippet.len() - inner_pat_snippet.len()) as u32),
+                    );
+                    (pat_span, String::new(), "mutable borrow")
+                } else {
+                    let pat_span = pat_span.with_hi(
+                        pat_span.lo()
+                            + BytePos(
+                                (pat_snippet.len() - inner_pat_snippet.trim_start().len()) as u32,
+                            ),
+                    );
+                    (pat_span, String::new(), "borrow")
+                };
+                suggestions.push((
+                    pat_span,
+                    format!("consider removing the {to_remove}"),
+                    suggestion.to_string(),
+                ));
             }
         }
         suggestions.sort_unstable_by_key(|&(span, _, _)| span);
         suggestions.dedup_by_key(|&mut (span, _, _)| span);
-        for (span, to_remove, suggestion) in suggestions {
-            err.span_suggestion(
-                span,
-                &format!("consider removing the `{to_remove}`"),
-                suggestion,
-                Applicability::MachineApplicable,
-            );
+        for (span, msg, suggestion) in suggestions {
+            err.span_suggestion_verbose(span, &msg, suggestion, Applicability::MachineApplicable);
         }
     }
 
@@ -521,8 +549,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
 
         if binds_to.len() > 1 {
             err.note(
-                "move occurs because these variables have types that \
-                      don't implement the `Copy` trait",
+                "move occurs because these variables have types that don't implement the `Copy` \
+                 trait",
             );
         }
     }

compiler/rustc_mir_build/src/build/block.rs

@@ -231,7 +231,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                                         remainder_span,
                                         pattern,
                                         None,
-                                        Some((None, initializer_span)),
+                                        Some((Some(&destination), initializer_span)),
                                     );
                                     this.visit_primary_bindings(
                                         pattern,

src/test/ui/borrowck/access-mode-in-closures.stderr

@@ -3,10 +3,15 @@ error[E0507]: cannot move out of `s` which is behind a shared reference
    |
 LL |         match *s { S(v) => v }
    |               ^^     -
-   |               |      |
-   |               |      data moved here
-   |               |      move occurs because `v` has type `Vec<isize>`, which does not implement the `Copy` trait
-   |               help: consider borrowing here: `&*s`
+   |                      |
+   |                      data moved here
+   |                      move occurs because `v` has type `Vec<isize>`, which does not implement the `Copy` trait
+   |
+help: consider removing the dereference here
+   |
+LL -         match *s { S(v) => v }
+LL +         match s { S(v) => v }
+   |
 
 error: aborting due to previous error
 

src/test/ui/borrowck/borrowck-for-loop-correct-cmt-for-pattern.stderr

@@ -2,31 +2,46 @@ error[E0507]: cannot move out of a shared reference
   --> $DIR/borrowck-for-loop-correct-cmt-for-pattern.rs:12:15
    |
 LL |     for &a in x.iter() {
-   |         --    ^^^^^^^^
-   |         ||
-   |         |data moved here
-   |         |move occurs because `a` has type `&mut i32`, which does not implement the `Copy` trait
-   |         help: consider removing the `&`: `a`
+   |          -    ^^^^^^^^
+   |          |
+   |          data moved here
+   |          move occurs because `a` has type `&mut i32`, which does not implement the `Copy` trait
+   |
+help: consider removing the borrow
+   |
+LL -     for &a in x.iter() {
+LL +     for a in x.iter() {
+   |
 
 error[E0507]: cannot move out of a shared reference
   --> $DIR/borrowck-for-loop-correct-cmt-for-pattern.rs:18:15
    |
 LL |     for &a in &f.a {
-   |         --    ^^^^
-   |         ||
-   |         |data moved here
-   |         |move occurs because `a` has type `Box<isize>`, which does not implement the `Copy` trait
-   |         help: consider removing the `&`: `a`
+   |          -    ^^^^
+   |          |
+   |          data moved here
+   |          move occurs because `a` has type `Box<isize>`, which does not implement the `Copy` trait
+   |
+help: consider removing the borrow
+   |
+LL -     for &a in &f.a {
+LL +     for a in &f.a {
+   |
 
 error[E0507]: cannot move out of a shared reference
   --> $DIR/borrowck-for-loop-correct-cmt-for-pattern.rs:22:15
    |
 LL |     for &a in x.iter() {
-   |         --    ^^^^^^^^
-   |         ||
-   |         |data moved here
-   |         |move occurs because `a` has type `Box<i32>`, which does not implement the `Copy` trait
-   |         help: consider removing the `&`: `a`
+   |          -    ^^^^^^^^
+   |          |
+   |          data moved here
+   |          move occurs because `a` has type `Box<i32>`, which does not implement the `Copy` trait
+   |
+help: consider removing the borrow
+   |
+LL -     for &a in x.iter() {
+LL +     for a in x.iter() {
+   |
 
 error: aborting due to 3 previous errors
 

src/test/ui/borrowck/borrowck-issue-2657-2.fixed

@@ -0,0 +1,12 @@
+// run-rustfix
+fn main() {
+
+    let x: Option<Box<_>> = Some(Box::new(1));
+
+    match x {
+      Some(ref y) => {
+        let _b = y; //~ ERROR cannot move out
+      }
+      _ => {}
+    }
+}

src/test/ui/borrowck/borrowck-issue-2657-2.rs

@@ -1,3 +1,4 @@
+// run-rustfix
 fn main() {
 
     let x: Option<Box<_>> = Some(Box::new(1));

src/test/ui/borrowck/borrowck-issue-2657-2.stderr

@@ -1,11 +1,14 @@
 error[E0507]: cannot move out of `*y` which is behind a shared reference
-  --> $DIR/borrowck-issue-2657-2.rs:7:18
+  --> $DIR/borrowck-issue-2657-2.rs:8:18
    |
 LL |         let _b = *y;
-   |                  ^^
-   |                  |
-   |                  move occurs because `*y` has type `Box<i32>`, which does not implement the `Copy` trait
-   |                  help: consider borrowing here: `&*y`
+   |                  ^^ move occurs because `*y` has type `Box<i32>`, which does not implement the `Copy` trait
+   |
+help: consider removing the dereference here
+   |
+LL -         let _b = *y;
+LL +         let _b = y;
+   |
 
 error: aborting due to previous error
 

src/test/ui/borrowck/borrowck-move-error-with-note.fixed

@@ -0,0 +1,56 @@
+// run-rustfix
+#![allow(unused)]
+enum Foo {
+    Foo1(Box<u32>, Box<u32>),
+    Foo2(Box<u32>),
+    Foo3,
+}
+
+
+
+fn blah() {
+    let f = &Foo::Foo1(Box::new(1), Box::new(2));
+    match f { //~ ERROR cannot move out of
+        Foo::Foo1(num1,
+                  num2) => (),
+        Foo::Foo2(num) => (),
+        Foo::Foo3 => ()
+    }
+}
+
+struct S {
+    f: String,
+    g: String
+}
+impl Drop for S {
+    fn drop(&mut self) { println!("{}", self.f); }
+}
+
+fn move_in_match() {
+    match (S {f: "foo".to_string(), g: "bar".to_string()}) {
+        //~^ ERROR cannot move out of type `S`, which implements the `Drop` trait
+        S {
+            f: ref _s,
+            g: ref _t
+        } => {}
+    }
+}
+
+// from issue-8064
+struct A {
+    a: Box<isize>,
+}
+
+fn free<T>(_: T) {}
+
+fn blah2() {
+    let a = &A { a: Box::new(1) };
+    match &a.a { //~ ERROR cannot move out of
+        n => {
+            free(n)
+        }
+    }
+    free(a)
+}
+
+fn main() {}

src/test/ui/borrowck/borrowck-move-error-with-note.rs

@@ -1,3 +1,5 @@
+// run-rustfix
+#![allow(unused)]
 enum Foo {
     Foo1(Box<u32>, Box<u32>),
     Foo2(Box<u32>),