From fc0abdeb5529f3bc942d090f50895cae88731b12 Mon Sep 17 00:00:00 2001
From: yukang <moorekang@gmail.com>
Date: Fri, 21 Jul 2023 23:57:28 +0800
Subject: [PATCH] More precisely point out what is immutable for E0596

---
 .../src/diagnostics/mutability_errors.rs      | 52 +++++++++++++++++--
 tests/ui/borrowck/issue-111554.stderr         | 10 +++-
 tests/ui/borrowck/issue_113842.rs             | 22 ++++++++
 tests/ui/borrowck/issue_113842.stderr         | 31 +++++++++++
 ...et-else-binding-explicit-mut-borrow.stderr | 10 +++-
 tests/ui/nll/issue-51191.rs                   |  1 +
 tests/ui/nll/issue-51191.stderr               | 16 ++++--
 7 files changed, 133 insertions(+), 9 deletions(-)
 create mode 100644 tests/ui/borrowck/issue_113842.rs
 create mode 100644 tests/ui/borrowck/issue_113842.stderr

diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
index e6bde6a8c54dc..efe6f9cd5ce5f 100644
--- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
@@ -506,9 +506,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                 self.expected_fn_found_fn_mut_call(&mut err, span, act);
             }
 
-            PlaceRef { local: _, projection: [.., ProjectionElem::Deref] } => {
-                err.span_label(span, format!("cannot {act}"));
-
+            PlaceRef { local: _, projection: [proj_base @ .., ProjectionElem::Deref] } => {
+                let mut span = span;
                 match opt_source {
                     Some(BorrowedContentSource::OverloadedDeref(ty)) => {
                         err.help(format!(
@@ -523,8 +522,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                         ));
                         self.suggest_map_index_mut_alternatives(ty, &mut err, span);
                     }
+                    Some(BorrowedContentSource::DerefSharedRef) => {
+                        let place_ref =
+                            PlaceRef { local: the_place_err.local, projection: proj_base };
+                        let ty = place_ref.ty(self.body, self.infcx.tcx).ty;
+                        self.suggest_detailed_hint_for_ref(ty, &mut err, &mut span);
+                    }
                     _ => (),
                 }
+                err.span_label(span, format!("cannot {act}"));
             }
 
             _ => {
@@ -539,6 +545,46 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         }
     }
 
+    fn suggest_detailed_hint_for_ref(&self, ty: Ty<'_>, err: &mut Diagnostic, span: &mut Span) {
+        struct ExprFinder {
+            span: Span,
+            hir_id: Option<hir::HirId>,
+        }
+
+        impl<'tcx> Visitor<'tcx> for ExprFinder {
+            fn visit_expr(&mut self, s: &'tcx hir::Expr<'tcx>) {
+                if s.span.contains(self.span) {
+                    self.hir_id = Some(s.hir_id);
+                }
+                hir::intravisit::walk_expr(self, s);
+            }
+        }
+        let hir_map = self.infcx.tcx.hir();
+        let def_id = self.body.source.def_id();
+        let hir_id = if let Some(local_def_id) = def_id.as_local()
+            && let Some(body_id) = hir_map.maybe_body_owned_by(local_def_id) {
+            let body = hir_map.body(body_id);
+            let mut v = ExprFinder {
+                span: *span,
+                hir_id: None,
+            };
+            v.visit_body(body);
+            v.hir_id
+        } else {
+            None
+        };
+        let expr = if let Some(hir_id) = hir_id {
+            hir_map.expect_expr(hir_id)
+        } else {
+            return;
+        };
+        if let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, Mutability::Mut, ex) = expr.kind
+            && !matches!(ex.kind, hir::ExprKind::Unary(hir::UnOp::Deref, _) | hir::ExprKind::Field(_, _) | hir::ExprKind::Lit(_)) {
+                *span = ex.span;
+                err.span_help(ex.span, format!("this expression is of type `{}`, which is an immutable reference", ty));
+        }
+    }
+
     fn suggest_map_index_mut_alternatives(&self, ty: Ty<'tcx>, err: &mut Diagnostic, span: Span) {
         let Some(adt) = ty.ty_adt_def() else { return };
         let did = adt.did();
diff --git a/tests/ui/borrowck/issue-111554.stderr b/tests/ui/borrowck/issue-111554.stderr
index 6b7a42e495999..4779b573719c7 100644
--- a/tests/ui/borrowck/issue-111554.stderr
+++ b/tests/ui/borrowck/issue-111554.stderr
@@ -14,7 +14,15 @@ error[E0596]: cannot borrow data in a `&` reference as mutable
   --> $DIR/issue-111554.rs:10:16
    |
 LL |         || bar(&mut self);
-   |                ^^^^^^^^^ cannot borrow as mutable
+   |                ^^^^^----
+   |                     |
+   |                     cannot borrow as mutable
+   |
+help: this expression is of type `&Foo`, which is an immutable reference
+  --> $DIR/issue-111554.rs:10:21
+   |
+LL |         || bar(&mut self);
+   |                     ^^^^
 
 error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
   --> $DIR/issue-111554.rs:21:16
diff --git a/tests/ui/borrowck/issue_113842.rs b/tests/ui/borrowck/issue_113842.rs
new file mode 100644
index 0000000000000..adb7e40b267b1
--- /dev/null
+++ b/tests/ui/borrowck/issue_113842.rs
@@ -0,0 +1,22 @@
+mod some_library {
+    pub fn foo(_: &mut [i32]) {}
+    pub fn bar<'a>() -> &'a [i32] {
+        &[]
+    }
+    pub fn bar_mut<'a>() -> &'a mut [i32] {
+        &mut []
+    }
+}
+
+struct Foo {
+    pub x: i32,
+}
+
+fn foo() {
+    let foo = Foo { x: 0 };
+    let _y: &mut Foo = &mut &foo; //~ ERROR cannot borrow data in a `&` reference as mutable
+}
+
+fn main() {
+    some_library::foo(&mut some_library::bar()); //~ ERROR cannot borrow data in a `&` reference as mutable
+}
diff --git a/tests/ui/borrowck/issue_113842.stderr b/tests/ui/borrowck/issue_113842.stderr
new file mode 100644
index 0000000000000..164574b77f5bb
--- /dev/null
+++ b/tests/ui/borrowck/issue_113842.stderr
@@ -0,0 +1,31 @@
+error[E0596]: cannot borrow data in a `&` reference as mutable
+  --> $DIR/issue_113842.rs:17:24
+   |
+LL |     let _y: &mut Foo = &mut &foo;
+   |                        ^^^^^----
+   |                             |
+   |                             cannot borrow as mutable
+   |
+help: this expression is of type `&Foo`, which is an immutable reference
+  --> $DIR/issue_113842.rs:17:29
+   |
+LL |     let _y: &mut Foo = &mut &foo;
+   |                             ^^^^
+
+error[E0596]: cannot borrow data in a `&` reference as mutable
+  --> $DIR/issue_113842.rs:21:23
+   |
+LL |     some_library::foo(&mut some_library::bar());
+   |                       ^^^^^-------------------
+   |                            |
+   |                            cannot borrow as mutable
+   |
+help: this expression is of type `&[i32]`, which is an immutable reference
+  --> $DIR/issue_113842.rs:21:28
+   |
+LL |     some_library::foo(&mut some_library::bar());
+   |                            ^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0596`.
diff --git a/tests/ui/let-else/let-else-binding-explicit-mut-borrow.stderr b/tests/ui/let-else/let-else-binding-explicit-mut-borrow.stderr
index 023fab8fe4a3d..46e60d662de67 100644
--- a/tests/ui/let-else/let-else-binding-explicit-mut-borrow.stderr
+++ b/tests/ui/let-else/let-else-binding-explicit-mut-borrow.stderr
@@ -2,7 +2,15 @@ error[E0596]: cannot borrow data in a `&` reference as mutable
   --> $DIR/let-else-binding-explicit-mut-borrow.rs:7:37
    |
 LL |     let Some(n): &mut Option<i32> = &mut &Some(5i32) else {
-   |                                     ^^^^^^^^^^^^^^^^ cannot borrow as mutable
+   |                                     ^^^^^-----------
+   |                                          |
+   |                                          cannot borrow as mutable
+   |
+help: this expression is of type `&Option<i32>`, which is an immutable reference
+  --> $DIR/let-else-binding-explicit-mut-borrow.rs:7:42
+   |
+LL |     let Some(n): &mut Option<i32> = &mut &Some(5i32) else {
+   |                                          ^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/tests/ui/nll/issue-51191.rs b/tests/ui/nll/issue-51191.rs
index 836587d93b84b..a2076b0db6188 100644
--- a/tests/ui/nll/issue-51191.rs
+++ b/tests/ui/nll/issue-51191.rs
@@ -22,6 +22,7 @@ impl Struct {
         (&mut self).bar();
         //~^ ERROR cannot borrow `self` as mutable, as it is not declared as mutable [E0596]
         //~^^ ERROR cannot borrow data in a `&` reference as mutable [E0596]
+        //~^^^ HELP this expression is of type `&Struct`, which is an immutable reference
     }
 
     fn mtblref(&mut self) {
diff --git a/tests/ui/nll/issue-51191.stderr b/tests/ui/nll/issue-51191.stderr
index c14056c3a834b..5da92b2ad6e8f 100644
--- a/tests/ui/nll/issue-51191.stderr
+++ b/tests/ui/nll/issue-51191.stderr
@@ -48,21 +48,29 @@ error[E0596]: cannot borrow data in a `&` reference as mutable
   --> $DIR/issue-51191.rs:22:9
    |
 LL |         (&mut self).bar();
-   |         ^^^^^^^^^^^ cannot borrow as mutable
+   |         ^^^^^^----^
+   |               |
+   |               cannot borrow as mutable
+   |
+help: this expression is of type `&Struct`, which is an immutable reference
+  --> $DIR/issue-51191.rs:22:15
+   |
+LL |         (&mut self).bar();
+   |               ^^^^
 
 error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
-  --> $DIR/issue-51191.rs:28:9
+  --> $DIR/issue-51191.rs:29:9
    |
 LL |         (&mut self).bar();
    |         ^^^^^^^^^^^ cannot borrow as mutable
    |
 note: the binding is already a mutable borrow
-  --> $DIR/issue-51191.rs:27:16
+  --> $DIR/issue-51191.rs:28:16
    |
 LL |     fn mtblref(&mut self) {
    |                ^^^^^^^^^
 help: try removing `&mut` here
-  --> $DIR/issue-51191.rs:28:9
+  --> $DIR/issue-51191.rs:29:9
    |
 LL |         (&mut self).bar();
    |         ^^^^^^^^^^^