Suggest removing &mut from borrow of &mut

Fix a typo: minding -> binding
Add test for &mut &mut

4 files changed, 60 insertions, 46 deletions

diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs
index da7677f802b..92a2baa30ee 100644
--- a/compiler/rustc_middle/src/mir/mod.rs
+++ b/compiler/rustc_middle/src/mir/mod.rs
@@ -813,7 +813,7 @@ pub struct BlockTailInfo {
 /// argument, or the return place.
 #[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
 pub struct LocalDecl<'tcx> {
-    /// Whether this is a mutable minding (i.e., `let x` or `let mut x`).
+    /// Whether this is a mutable binding (i.e., `let x` or `let mut x`).
     ///
     /// Temporaries and the return place are always mutable.
     pub mutability: Mutability,
diff --git a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs
index 9728662c984..e1af6fc07cf 100644
--- a/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs
+++ b/compiler/rustc_mir/src/borrow_check/diagnostics/mutability_errors.rs
@@ -1,11 +1,11 @@
 use rustc_hir as hir;
 use rustc_hir::Node;
 use rustc_index::vec::Idx;
-use rustc_middle::mir::{self, ClearCrossCrate, Local, LocalInfo, Location};
+use rustc_middle::mir::{self, ClearCrossCrate, Local, LocalDecl, LocalInfo, Location};
 use rustc_middle::mir::{Mutability, Place, PlaceRef, ProjectionElem};
 use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_span::source_map::DesugaringKind;
-use rustc_span::symbol::kw;
+use rustc_span::symbol::{kw, Symbol};
 use rustc_span::Span;
 
 use crate::borrow_check::diagnostics::BorrowedContentSource;
@@ -211,36 +211,12 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
 
             // Suggest removing a `&mut` from the use of a mutable reference.
             PlaceRef { local, projection: [] }
-                if {
-                    self.body
-                        .local_decls
-                        .get(local)
-                        .map(|local_decl| {
-                            if let Some(box LocalInfo::User(ClearCrossCrate::Set(
-                                mir::BindingForm::ImplicitSelf(kind),
-                            ))) = local_decl.local_info
-                            {
-                                // Check if the user variable is a `&mut self` and we can therefore
-                                // suggest removing the `&mut`.
-                                //
-                                // Deliberately fall into this case for all implicit self types,
-                                // so that we don't fall in to the next case with them.
-                                kind == mir::ImplicitSelfKind::MutRef
-                            } else if Some(kw::SelfLower) == self.local_names[local] {
-                                // Otherwise, check if the name is the self kewyord - in which case
-                                // we have an explicit self. Do the same thing in this case and check
-                                // for a `self: &mut Self` to suggest removing the `&mut`.
-                                if let ty::Ref(_, _, hir::Mutability::Mut) = local_decl.ty.kind() {
-                                    true
-                                } else {
-                                    false
-                                }
-                            } else {
-                                false
-                            }
-                        })
-                        .unwrap_or(false)
-                } =>
+                if self
+                    .body
+                    .local_decls
+                    .get(local)
+                    .map(|l| mut_borrow_of_mutable_ref(l, self.local_names[local]))
+                    .unwrap_or(false) =>
             {
                 err.span_label(span, format!("cannot {ACT}", ACT = act));
                 err.span_label(span, "try removing `&mut` here");
@@ -581,6 +557,34 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
     }
 }
 
+fn mut_borrow_of_mutable_ref(local_decl: &LocalDecl<'_>, local_name: Option<Symbol>) -> bool {
+    debug!("local_info: {:?}, ty.kind(): {:?}", local_decl.local_info, local_decl.ty.kind());
+
+    match local_decl.local_info.as_deref() {
+        // Check if mutably borrowing a mutable reference.
+        Some(LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::Var(
+            mir::VarBindingForm {
+                binding_mode: ty::BindingMode::BindByValue(Mutability::Not), ..
+            },
+        )))) => matches!(local_decl.ty.kind(), ty::Ref(_, _, hir::Mutability::Mut)),
+        Some(LocalInfo::User(ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf(kind)))) => {
+            // Check if the user variable is a `&mut self` and we can therefore
+            // suggest removing the `&mut`.
+            //
+            // Deliberately fall into this case for all implicit self types,
+            // so that we don't fall in to the next case with them.
+            *kind == mir::ImplicitSelfKind::MutRef
+        }
+        _ if Some(kw::SelfLower) == local_name => {
+            // Otherwise, check if the name is the `self` keyword - in which case
+            // we have an explicit self. Do the same thing in this case and check
+            // for a `self: &mut Self` to suggest removing the `&mut`.
+            matches!(local_decl.ty.kind(), ty::Ref(_, _, hir::Mutability::Mut))
+        }
+        _ => false,
+    }
+}
+
 fn suggest_ampmut_self<'tcx>(
     tcx: TyCtxt<'tcx>,
     local_decl: &mir::LocalDecl<'tcx>,
diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs b/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs
index 212f706953e..59b541a24d1 100644
--- a/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs
+++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs
@@ -1,11 +1,11 @@
 // Suggest not mutably borrowing a mutable reference
+#![crate_type = "rlib"]
 
-fn main() {
-    f(&mut 0)
+pub fn f(b: &mut i32) {
+    g(&mut b);
+    //~^ ERROR cannot borrow
+    g(&mut &mut b);
+    //~^ ERROR cannot borrow
 }
 
-fn f(b: &mut i32) {
-    g(&mut b) //~ ERROR cannot borrow
-}
-
-fn g(_: &mut i32) {}
+pub fn g(_: &mut i32) {}
diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr
index 09dabbc89b4..8710f204698 100644
--- a/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr
+++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr
@@ -1,11 +1,21 @@
 error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable
-  --> $DIR/mut-borrow-of-mut-ref.rs:8:7
+  --> $DIR/mut-borrow-of-mut-ref.rs:5:7
    |
-LL | fn f(b: &mut i32) {
-   |      - help: consider changing this to be mutable: `mut b`
-LL |     g(&mut b)
-   |       ^^^^^^ cannot borrow as mutable
+LL |     g(&mut b);
+   |       ^^^^^^
+   |       |
+   |       cannot borrow as mutable
+   |       try removing `&mut` here
 
-error: aborting due to previous error
+error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable
+  --> $DIR/mut-borrow-of-mut-ref.rs:7:12
+   |
+LL |     g(&mut &mut b);
+   |            ^^^^^^
+   |            |
+   |            cannot borrow as mutable
+   |            try removing `&mut` here
+
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0596`.