Rollup merge of #98603 - compiler-errors:minor-borrowck-diagnostic-fi…

…xes, r=davidtwco

Some borrowck diagnostic fixes

1. Remove some redundant `.as_ref` suggestion logic from borrowck, this has the consequence of also not suggesting `.as_ref` after `Option` methods, but (correctly) before.
2. Fix a bug where we were replacing a binding's name with a type. Instead, make it a note.

This is somewhat incomplete. See `src/test/ui/borrowck/suggest-as-ref-on-mut-closure.rs` for more improvements.

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::{sym, Span};
+use rustc_span::Span;
 
 use crate::diagnostics::UseSpans;
 use crate::prefixes::PrefixSet;
@@ -218,29 +218,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
 
     fn report(&mut self, error: GroupedMoveError<'tcx>) {
         let (mut err, err_span) = {
-            let (span, use_spans, original_path, kind, has_complex_bindings): (
-                Span,
-                Option<UseSpans<'tcx>>,
-                Place<'tcx>,
-                &IllegalMoveOriginKind<'_>,
-                bool,
-            ) = match error {
-                GroupedMoveError::MovesFromPlace {
-                    span,
-                    original_path,
-                    ref kind,
-                    ref binds_to,
-                    ..
+            let (span, use_spans, original_path, kind) = match error {
+                GroupedMoveError::MovesFromPlace { span, original_path, ref kind, .. }
+                | GroupedMoveError::MovesFromValue { span, original_path, ref kind, .. } => {
+                    (span, None, original_path, kind)
                 }
-                | GroupedMoveError::MovesFromValue {
-                    span,
-                    original_path,
-                    ref kind,
-                    ref binds_to,
-                    ..
-                } => (span, None, original_path, kind, !binds_to.is_empty()),
                 GroupedMoveError::OtherIllegalMove { use_spans, original_path, ref kind } => {
-                    (use_spans.args_or_use(), Some(use_spans), original_path, kind, false)
+                    (use_spans.args_or_use(), Some(use_spans), original_path, kind)
                 }
             };
             debug!(
@@ -259,7 +243,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                             target_place,
                             span,
                             use_spans,
-                            has_complex_bindings,
                         ),
                     &IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
                         self.cannot_move_out_of_interior_of_drop(span, ty)
@@ -302,7 +285,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         deref_target_place: Place<'tcx>,
         span: Span,
         use_spans: Option<UseSpans<'tcx>>,
-        has_complex_bindings: bool,
     ) -> DiagnosticBuilder<'a, ErrorGuaranteed> {
         // Inspect the type of the content behind the
         // borrow to provide feedback about why this
@@ -399,28 +381,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 }
             }
         };
-        let ty = move_place.ty(self.body, self.infcx.tcx).ty;
-        let def_id = match *ty.kind() {
-            ty::Adt(self_def, _) => self_def.did(),
-            ty::Foreign(def_id)
-            | ty::FnDef(def_id, _)
-            | ty::Closure(def_id, _)
-            | ty::Generator(def_id, ..)
-            | ty::Opaque(def_id, _) => def_id,
-            _ => return err,
-        };
-        let diag_name = self.infcx.tcx.get_diagnostic_name(def_id);
-        if matches!(diag_name, Some(sym::Option | sym::Result))
-            && use_spans.map_or(true, |v| !v.for_closure())
-            && !has_complex_bindings
-        {
-            err.span_suggestion_verbose(
-                span.shrink_to_hi(),
-                &format!("consider borrowing the `{}`'s content", diag_name.unwrap()),
-                ".as_ref()",
-                Applicability::MaybeIncorrect,
-            );
-        } else if let Some(use_spans) = use_spans {
+        if let Some(use_spans) = use_spans {
             self.explain_captures(
                 &mut err, span, span, use_spans, move_place, None, "", "", "", false, true,
             );

compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

@@ -434,16 +434,16 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
 
                 match self.local_names[local] {
                     Some(name) if !local_decl.from_compiler_desugaring() => {
-                        let label = match local_decl.local_info.as_ref().unwrap() {
-                            box LocalInfo::User(ClearCrossCrate::Set(
+                        let label = match local_decl.local_info.as_deref().unwrap() {
+                            LocalInfo::User(ClearCrossCrate::Set(
                                 mir::BindingForm::ImplicitSelf(_),
                             )) => {
                                 let (span, suggestion) =
                                     suggest_ampmut_self(self.infcx.tcx, local_decl);
                                 Some((true, span, suggestion))
                             }
 
-                            box LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
+                            LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
                                 mir::VarBindingForm {
                                     binding_mode: ty::BindingMode::BindByValue(_),
                                     opt_ty_info,
@@ -473,20 +473,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                     // on for loops, RHS points to the iterator part
                                     Some(DesugaringKind::ForLoop) => {
                                         self.suggest_similar_mut_method_for_for_loop(&mut err);
-                                        Some((
-                                            false,
-                                            opt_assignment_rhs_span.unwrap(),
-                                            format!(
-                                                "this iterator yields `{SIGIL}` {DESC}s",
-                                                SIGIL = pointer_sigil,
-                                                DESC = pointer_desc
-                                            ),
-                                        ))
+                                        err.span_label(opt_assignment_rhs_span.unwrap(), format!(
+                                            "this iterator yields `{pointer_sigil}` {pointer_desc}s",
+                                        ));
+                                        None
                                     }
                                     // don't create labels for compiler-generated spans
                                     Some(_) => None,
                                     None => {
-                                        let (span, suggestion) = if name != kw::SelfLower {
+                                        let label = if name != kw::SelfLower {
                                             suggest_ampmut(
                                                 self.infcx.tcx,
                                                 local_decl,
@@ -501,7 +496,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                                         ..
                                                     }),
                                                 ))) => {
-                                                    suggest_ampmut_self(self.infcx.tcx, local_decl)
+                                                    let (span, sugg) = suggest_ampmut_self(
+                                                        self.infcx.tcx,
+                                                        local_decl,
+                                                    );
+                                                    (true, span, sugg)
                                                 }
                                                 // explicit self (eg `self: &'a Self`)
                                                 _ => suggest_ampmut(
@@ -512,12 +511,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                                 ),
                                             }
                                         };
-                                        Some((true, span, suggestion))
+                                        Some(label)
                                     }
                                 }
                             }
 
-                            box LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
+                            LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
                                 mir::VarBindingForm {
                                     binding_mode: ty::BindingMode::BindByReference(_),
                                     ..
@@ -528,7 +527,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                     .map(|replacement| (true, pattern_span, replacement))
                             }
 
-                            box LocalInfo::User(ClearCrossCrate::Clear) => {
+                            LocalInfo::User(ClearCrossCrate::Clear) => {
                                 bug!("saw cleared local state")
                             }
 
@@ -559,7 +558,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                                 }
                             }
                             Some((false, err_label_span, message)) => {
-                                err.span_label(err_label_span, &message);
+                                err.span_label(
+                                    err_label_span,
+                                    &format!(
+                                        "consider changing this binding's type to be: `{message}`"
+                                    ),
+                                );
                             }
                             None => {}
                         }
@@ -1004,7 +1008,7 @@ fn suggest_ampmut<'tcx>(
     local_decl: &mir::LocalDecl<'tcx>,
     opt_assignment_rhs_span: Option<Span>,
     opt_ty_info: Option<Span>,
-) -> (Span, String) {
+) -> (bool, Span, String) {
     if let Some(assignment_rhs_span) = opt_assignment_rhs_span
         && let Ok(src) = tcx.sess.source_map().span_to_snippet(assignment_rhs_span)
     {
@@ -1028,37 +1032,38 @@ fn suggest_ampmut<'tcx>(
             let lt_name = &src[1..ws_pos];
             let ty = src[ws_pos..].trim_start();
             if !is_mutbl(ty) {
-                return (assignment_rhs_span, format!("&{lt_name} mut {ty}"));
+                return (true, assignment_rhs_span, format!("&{lt_name} mut {ty}"));
             }
         } else if let Some(stripped) = src.strip_prefix('&') {
             let stripped = stripped.trim_start();
             if !is_mutbl(stripped) {
-                return (assignment_rhs_span, format!("&mut {stripped}"));
+                return (true, assignment_rhs_span, format!("&mut {stripped}"));
             }
         }
     }
 
-    let highlight_span = match opt_ty_info {
+    let (suggestability, highlight_span) = match opt_ty_info {
         // if this is a variable binding with an explicit type,
         // try to highlight that for the suggestion.
-        Some(ty_span) => ty_span,
+        Some(ty_span) => (true, ty_span),
 
         // otherwise, just highlight the span associated with
         // the (MIR) LocalDecl.
-        None => local_decl.source_info.span,
+        None => (false, local_decl.source_info.span),
     };
 
     if let Ok(src) = tcx.sess.source_map().span_to_snippet(highlight_span)
         && let (true, Some(ws_pos)) = (src.starts_with("&'"), src.find(char::is_whitespace))
     {
         let lt_name = &src[1..ws_pos];
         let ty = &src[ws_pos..];
-        return (highlight_span, format!("&{} mut{}", lt_name, ty));
+        return (true, highlight_span, format!("&{} mut{}", lt_name, ty));
     }
 
     let ty_mut = local_decl.ty.builtin_deref(true).unwrap();
     assert_eq!(ty_mut.mutbl, hir::Mutability::Not);
     (
+        suggestability,
         highlight_span,
         if local_decl.ty.is_region_ptr() {
             format!("&mut {}", ty_mut.ty)

compiler/rustc_mir_build/src/build/mod.rs

@@ -162,7 +162,11 @@ fn mir_build(tcx: TyCtxt<'_>, def: ty::WithOptConstParam<LocalDefId>) -> Body<'_
                 let opt_ty_info;
                 let self_arg;
                 if let Some(ref fn_decl) = tcx.hir().fn_decl_by_hir_id(owner_id) {
-                    opt_ty_info = fn_decl.inputs.get(index).map(|ty| ty.span);
+                    opt_ty_info = fn_decl
+                        .inputs
+                        .get(index)
+                        // Make sure that inferred closure args have no type span
+                        .and_then(|ty| if arg.pat.span != ty.span { Some(ty.span) } else { None });
                     self_arg = if index == 0 && fn_decl.implicit_self.has_implicit_self() {
                         match fn_decl.implicit_self {
                             hir::ImplicitSelfKind::Imm => Some(ImplicitSelfKind::Imm),

src/test/ui/binop/binop-move-semantics.stderr

@@ -63,8 +63,20 @@ LL |     use_mut(n); use_imm(m);
 error[E0507]: cannot move out of `*m` which is behind a mutable reference
   --> $DIR/binop-move-semantics.rs:30:5
    |
-LL |     *m
-   |     ^^ move occurs because `*m` has type `T`, which does not implement the `Copy` trait
+LL |       *m
+   |       -^
+   |       |
+   |  _____move occurs because `*m` has type `T`, which does not implement the `Copy` trait
+   | |
+LL | |     +
+LL | |     *n;
+   | |______- `*m` moved due to usage in operator
+   |
+note: calling this operator moves the left-hand side
+  --> $SRC_DIR/core/src/ops/arith.rs:LL:COL
+   |
+LL |     fn add(self, rhs: Rhs) -> Self::Output;
+   |            ^^^^
 
 error[E0507]: cannot move out of `*n` which is behind a shared reference
   --> $DIR/binop-move-semantics.rs:32:5

src/test/ui/borrowck/borrowck-borrow-mut-base-ptr-in-aliasable-loc.stderr

@@ -2,7 +2,7 @@ error[E0594]: cannot assign to `**t1`, which is behind a `&` reference
   --> $DIR/borrowck-borrow-mut-base-ptr-in-aliasable-loc.rs:9:5
    |
 LL |     let t1 = t0;
-   |         -- help: consider changing this to be a mutable reference: `&mut &mut isize`
+   |         -- consider changing this binding's type to be: `&mut &mut isize`
 LL |     let p: &isize = &**t0;
 LL |     **t1 = 22;
    |     ^^^^^^^^^ `t1` is a `&` reference, so the data it refers to cannot be written

src/test/ui/borrowck/issue-85765.rs

@@ -1,7 +1,7 @@
 fn main() {
     let mut test = Vec::new();
     let rofl: &Vec<Vec<i32>> = &mut test;
-    //~^ HELP consider changing this to be a mutable reference
+    //~^ NOTE consider changing this binding's type to be
     rofl.push(Vec::new());
     //~^ ERROR cannot borrow `*rofl` as mutable, as it is behind a `&` reference
     //~| NOTE `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
@@ -15,14 +15,14 @@ fn main() {
 
     #[rustfmt::skip]
     let x: &usize = &mut{0};
-    //~^ HELP consider changing this to be a mutable reference
+    //~^ NOTE consider changing this binding's type to be
     *x = 1;
     //~^ ERROR cannot assign to `*x`, which is behind a `&` reference
     //~| NOTE `x` is a `&` reference, so the data it refers to cannot be written
 
     #[rustfmt::skip]
     let y: &usize = &mut(0);
-    //~^ HELP consider changing this to be a mutable reference
+    //~^ NOTE consider changing this binding's type to be
     *y = 1;
     //~^ ERROR cannot assign to `*y`, which is behind a `&` reference
     //~| NOTE `y` is a `&` reference, so the data it refers to cannot be written

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

@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `*rofl` as mutable, as it is behind a `&` reference
   --> $DIR/issue-85765.rs:5:5
    |
 LL |     let rofl: &Vec<Vec<i32>> = &mut test;
-   |         ---- help: consider changing this to be a mutable reference: `&mut Vec<Vec<i32>>`
+   |         ---- consider changing this binding's type to be: `&mut Vec<Vec<i32>>`
 LL |
 LL |     rofl.push(Vec::new());
    |     ^^^^^^^^^^^^^^^^^^^^^ `rofl` is a `&` reference, so the data it refers to cannot be borrowed as mutable
@@ -20,7 +20,7 @@ error[E0594]: cannot assign to `*x`, which is behind a `&` reference
   --> $DIR/issue-85765.rs:19:5
    |
 LL |     let x: &usize = &mut{0};
-   |         - help: consider changing this to be a mutable reference: `&mut usize`
+   |         - consider changing this binding's type to be: `&mut usize`
 LL |
 LL |     *x = 1;
    |     ^^^^^^ `x` is a `&` reference, so the data it refers to cannot be written
@@ -29,7 +29,7 @@ error[E0594]: cannot assign to `*y`, which is behind a `&` reference
   --> $DIR/issue-85765.rs:26:5
    |
 LL |     let y: &usize = &mut(0);
-   |         - help: consider changing this to be a mutable reference: `&mut usize`
+   |         - consider changing this binding's type to be: `&mut usize`
 LL |
 LL |     *y = 1;
    |     ^^^^^^ `y` is a `&` reference, so the data it refers to cannot be written

src/test/ui/borrowck/issue-91206.rs

@@ -9,7 +9,8 @@ impl TestClient {
 fn main() {
     let client = TestClient;
     let inner = client.get_inner_ref();
-    //~^ HELP consider changing this to be a mutable reference
+    //~^ NOTE consider changing this binding's type to be
     inner.clear();
     //~^ ERROR cannot borrow `*inner` as mutable, as it is behind a `&` reference [E0596]
+    //~| NOTE `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable
 }

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

@@ -2,7 +2,7 @@ error[E0596]: cannot borrow `*inner` as mutable, as it is behind a `&` reference
   --> $DIR/issue-91206.rs:13:5
    |
 LL |     let inner = client.get_inner_ref();
-   |         ----- help: consider changing this to be a mutable reference: `&mut Vec<usize>`
+   |         ----- consider changing this binding's type to be: `&mut Vec<usize>`
 LL |
 LL |     inner.clear();
    |     ^^^^^^^^^^^^^ `inner` is a `&` reference, so the data it refers to cannot be borrowed as mutable

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

@@ -2,7 +2,7 @@ error[E0594]: cannot assign to `*foo`, which is behind a `&` reference
   --> $DIR/issue-92015.rs:6:5
    |
 LL |     let foo = Some(&0).unwrap();
-   |         --- help: consider changing this to be a mutable reference: `&mut i32`
+   |         --- consider changing this binding's type to be: `&mut i32`
 LL |     *foo = 1;
    |     ^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written
 

src/test/ui/borrowck/suggest-as-ref-on-mut-closure.rs

@@ -0,0 +1,16 @@
+// This is not exactly right, yet.
+
+// Ideally we should be suggesting `as_mut` for the first case,
+// and suggesting to change `as_ref` to `as_mut` in the second.
+
+fn x(cb: &mut Option<&mut dyn FnMut()>) {
+    cb.map(|cb| cb());
+    //~^ ERROR cannot move out of `*cb` which is behind a mutable reference
+}
+
+fn x2(cb: &mut Option<&mut dyn FnMut()>) {
+    cb.as_ref().map(|cb| cb());
+    //~^ ERROR cannot borrow `*cb` as mutable, as it is behind a `&` reference
+}
+
+fn main() {}