Rollup merge of #83251 - estebank:issue-83241, r=oli-obk

Suggestion for call on immutable binding of mutable type

When calling a method requiring a mutable self borrow on an inmutable
to a mutable borrow of the type, suggest making the binding mutable.

Fix #83241.

Author: GuillaumeGomez

compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs

@@ -5,11 +5,14 @@ use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_middle::{
     hir::place::PlaceBase,
-    mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, LocalKind, Location},
+    mir::{
+        self, BindingForm, ClearCrossCrate, ImplicitSelfKind, Local, LocalDecl, LocalInfo,
+        LocalKind, Location,
+    },
 };
 use rustc_span::source_map::DesugaringKind;
 use rustc_span::symbol::{kw, Symbol};
-use rustc_span::Span;
+use rustc_span::{BytePos, Span};
 
 use crate::borrow_check::diagnostics::BorrowedContentSource;
 use crate::borrow_check::MirBorrowckCtxt;
@@ -241,13 +244,74 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     .map(|l| mut_borrow_of_mutable_ref(l, self.local_names[local]))
                     .unwrap_or(false) =>
             {
+                let decl = &self.body.local_decls[local];
                 err.span_label(span, format!("cannot {ACT}", ACT = act));
-                err.span_suggestion(
-                    span,
-                    "try removing `&mut` here",
-                    String::new(),
-                    Applicability::MaybeIncorrect,
-                );
+                if let Some(mir::Statement {
+                    source_info,
+                    kind:
+                        mir::StatementKind::Assign(box (
+                            _,
+                            mir::Rvalue::Ref(
+                                _,
+                                mir::BorrowKind::Mut { allow_two_phase_borrow: false },
+                                _,
+                            ),
+                        )),
+                    ..
+                }) = &self.body[location.block].statements.get(location.statement_index)
+                {
+                    match decl.local_info {
+                        Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::Var(
+                            mir::VarBindingForm {
+                                binding_mode: ty::BindingMode::BindByValue(Mutability::Not),
+                                opt_ty_info: Some(sp),
+                                opt_match_place: _,
+                                pat_span: _,
+                            },
+                        )))) => {
+                            err.span_note(sp, "the binding is already a mutable borrow");
+                        }
+                        _ => {
+                            err.span_note(
+                                decl.source_info.span,
+                                "the binding is already a mutable borrow",
+                            );
+                        }
+                    }
+                    if let Ok(snippet) =
+                        self.infcx.tcx.sess.source_map().span_to_snippet(source_info.span)
+                    {
+                        if snippet.starts_with("&mut ") {
+                            // We don't have access to the HIR to get accurate spans, but we can
+                            // give a best effort structured suggestion.
+                            err.span_suggestion_verbose(
+                                source_info.span.with_hi(source_info.span.lo() + BytePos(5)),
+                                "try removing `&mut` here",
+                                String::new(),
+                                Applicability::MachineApplicable,
+                            );
+                        } else {
+                            // This can occur with things like `(&mut self).foo()`.
+                            err.span_help(source_info.span, "try removing `&mut` here");
+                        }
+                    } else {
+                        err.span_help(source_info.span, "try removing `&mut` here");
+                    }
+                } else if decl.mutability == Mutability::Not
+                    && !matches!(
+                        decl.local_info,
+                        Some(box LocalInfo::User(ClearCrossCrate::Set(BindingForm::ImplicitSelf(
+                            ImplicitSelfKind::MutRef
+                        ))))
+                    )
+                {
+                    err.span_suggestion_verbose(
+                        decl.source_info.span.shrink_to_lo(),
+                        "consider making the binding mutable",
+                        "mut ".to_string(),
+                        Applicability::MachineApplicable,
+                    );
+                }
             }
 
             // We want to suggest users use `let mut` for local (user

src/test/ui/borrowck/mut-borrow-of-mut-ref.rs

@@ -2,12 +2,36 @@
 #![crate_type = "rlib"]
 
 pub fn f(b: &mut i32) {
-    g(&mut b);
+    //~^ NOTE the binding is already a mutable borrow
+    //~| NOTE the binding is already a mutable borrow
+    h(&mut b);
     //~^ ERROR cannot borrow
+    //~| NOTE cannot borrow as mutable
     //~| HELP try removing `&mut` here
     g(&mut &mut b);
     //~^ ERROR cannot borrow
+    //~| NOTE cannot borrow as mutable
     //~| HELP try removing `&mut` here
 }
 
-pub fn g(_: &mut i32) {}
+pub fn g(b: &mut i32) { //~ NOTE the binding is already a mutable borrow
+    h(&mut &mut b);
+    //~^ ERROR cannot borrow
+    //~| NOTE cannot borrow as mutable
+    //~| HELP try removing `&mut` here
+}
+
+pub fn h(_: &mut i32) {}
+
+trait Foo {
+    fn bar(&mut self);
+}
+
+impl Foo for &mut String {
+    fn bar(&mut self) {}
+}
+
+pub fn baz(f: &mut String) { //~ HELP consider making the binding mutable
+    f.bar(); //~ ERROR cannot borrow `f` as mutable, as it is not declared as mutable
+    //~^ NOTE cannot borrow as mutable
+}

src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr

@@ -1,21 +1,65 @@
 error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable
-  --> $DIR/mut-borrow-of-mut-ref.rs:5:7
+  --> $DIR/mut-borrow-of-mut-ref.rs:7:7
    |
-LL |     g(&mut b);
-   |       ^^^^^^
-   |       |
-   |       cannot borrow as mutable
-   |       help: try removing `&mut` here
+LL |     h(&mut b);
+   |       ^^^^^^ cannot borrow as mutable
+   |
+note: the binding is already a mutable borrow
+  --> $DIR/mut-borrow-of-mut-ref.rs:4:13
+   |
+LL | pub fn f(b: &mut i32) {
+   |             ^^^^^^^^
+help: try removing `&mut` here
+   |
+LL -     h(&mut b);
+LL +     h(b);
+   | 
 
 error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable
-  --> $DIR/mut-borrow-of-mut-ref.rs:8:12
+  --> $DIR/mut-borrow-of-mut-ref.rs:11:12
    |
 LL |     g(&mut &mut b);
-   |            ^^^^^^
-   |            |
-   |            cannot borrow as mutable
-   |            help: try removing `&mut` here
+   |            ^^^^^^ cannot borrow as mutable
+   |
+note: the binding is already a mutable borrow
+  --> $DIR/mut-borrow-of-mut-ref.rs:4:13
+   |
+LL | pub fn f(b: &mut i32) {
+   |             ^^^^^^^^
+help: try removing `&mut` here
+   |
+LL -     g(&mut &mut b);
+LL +     g(&mut b);
+   | 
+
+error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable
+  --> $DIR/mut-borrow-of-mut-ref.rs:18:12
+   |
+LL |     h(&mut &mut b);
+   |            ^^^^^^ cannot borrow as mutable
+   |
+note: the binding is already a mutable borrow
+  --> $DIR/mut-borrow-of-mut-ref.rs:17:13
+   |
+LL | pub fn g(b: &mut i32) {
+   |             ^^^^^^^^
+help: try removing `&mut` here
+   |
+LL -     h(&mut &mut b);
+LL +     h(&mut b);
+   | 
+
+error[E0596]: cannot borrow `f` as mutable, as it is not declared as mutable
+  --> $DIR/mut-borrow-of-mut-ref.rs:35:5
+   |
+LL |     f.bar();
+   |     ^ cannot borrow as mutable
+   |
+help: consider making the binding mutable
+   |
+LL | pub fn baz(mut f: &mut String) {
+   |            +++
 
-error: aborting due to 2 previous errors
+error: aborting due to 4 previous errors
 
 For more information about this error, try `rustc --explain E0596`.

src/test/ui/did_you_mean/issue-31424.stderr

@@ -1,11 +1,19 @@
 error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
   --> $DIR/issue-31424.rs:7:9
    |
+LL |         (&mut self).bar();
+   |         ^^^^^^^^^^^ cannot borrow as mutable
+   |
+note: the binding is already a mutable borrow
+  --> $DIR/issue-31424.rs:6:12
+   |
+LL |     fn foo(&mut self) {
+   |            ^^^^^^^^^
+help: try removing `&mut` here
+  --> $DIR/issue-31424.rs:7:9
+   |
 LL |         (&mut self).bar();
    |         ^^^^^^^^^^^
-   |         |
-   |         cannot borrow as mutable
-   |         help: try removing `&mut` here
 
 warning: function cannot return without recursing
   --> $DIR/issue-31424.rs:13:5
@@ -22,11 +30,19 @@ LL |         (&mut self).bar();
 error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
   --> $DIR/issue-31424.rs:16:9
    |
+LL |         (&mut self).bar();
+   |         ^^^^^^^^^^^ cannot borrow as mutable
+   |
+note: the binding is already a mutable borrow
+  --> $DIR/issue-31424.rs:13:18
+   |
+LL |     fn bar(self: &mut Self) {
+   |                  ^^^^^^^^^
+help: try removing `&mut` here
+  --> $DIR/issue-31424.rs:16:9
+   |
 LL |         (&mut self).bar();
    |         ^^^^^^^^^^^
-   |         |
-   |         cannot borrow as mutable
-   |         help: try removing `&mut` here
 
 error: aborting due to 2 previous errors; 1 warning emitted
 

src/test/ui/did_you_mean/issue-34126.stderr

@@ -2,10 +2,18 @@ error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
   --> $DIR/issue-34126.rs:6:18
    |
 LL |         self.run(&mut self);
-   |                  ^^^^^^^^^
-   |                  |
-   |                  cannot borrow as mutable
-   |                  help: try removing `&mut` here
+   |                  ^^^^^^^^^ cannot borrow as mutable
+   |
+note: the binding is already a mutable borrow
+  --> $DIR/issue-34126.rs:5:14
+   |
+LL |     fn start(&mut self) {
+   |              ^^^^^^^^^
+help: try removing `&mut` here
+   |
+LL -         self.run(&mut self);
+LL +         self.run(self);
+   | 
 
 error[E0502]: cannot borrow `self` as mutable because it is also borrowed as immutable
   --> $DIR/issue-34126.rs:6:18

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

@@ -13,11 +13,19 @@ LL |         (&mut self).bar();
 error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
   --> $DIR/issue-51191.rs:7:9
    |
+LL |         (&mut self).bar();
+   |         ^^^^^^^^^^^ cannot borrow as mutable
+   |
+note: the binding is already a mutable borrow
+  --> $DIR/issue-51191.rs:4:18
+   |
+LL |     fn bar(self: &mut Self) {
+   |                  ^^^^^^^^^
+help: try removing `&mut` here
+  --> $DIR/issue-51191.rs:7:9
+   |
 LL |         (&mut self).bar();
    |         ^^^^^^^^^^^
-   |         |
-   |         cannot borrow as mutable
-   |         help: try removing `&mut` here
 
 error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
   --> $DIR/issue-51191.rs:13:9
@@ -42,11 +50,19 @@ LL |         (&mut self).bar();
 error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
   --> $DIR/issue-51191.rs:28:9
    |
+LL |         (&mut self).bar();
+   |         ^^^^^^^^^^^ cannot borrow as mutable
+   |
+note: the binding is already a mutable borrow
+  --> $DIR/issue-51191.rs:27:16
+   |
+LL |     fn mtblref(&mut self) {
+   |                ^^^^^^^^^
+help: try removing `&mut` here
+  --> $DIR/issue-51191.rs:28:9
+   |
 LL |         (&mut self).bar();
    |         ^^^^^^^^^^^
-   |         |
-   |         cannot borrow as mutable
-   |         help: try removing `&mut` here
 
 error: aborting due to 5 previous errors; 1 warning emitted