about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-09-11 15:02:59 +0000
committerbors <bors@rust-lang.org>2023-09-11 15:02:59 +0000
commit3ebb5629d1a39345d37c4838d31dc7280384ea94 (patch)
tree742f985f86a66379b353a23074a13284b9ecf772
parent68c2f5ba0ffb6f7f0724dd62c7562daa634caaec (diff)
parenta8f3c7684df5ac15c770ef92d58436aa3386cfcf (diff)
downloadrust-3ebb5629d1a39345d37c4838d31dc7280384ea94.tar.gz
rust-3ebb5629d1a39345d37c4838d31dc7280384ea94.zip
Auto merge of #115595 - surechen:114896, r=davidtwco
Fix incorrect mutable suggestion information for binding in ref pattern like:  `let &b = a;`

fixes #114896

I find we have to get pat_span but not local_decl.source_info.span for suggestion. In `let &b = a;`  pat_span is &b. I think check `let &b = a` in hir to make sure it is hir::Node::Local(hir::Local {pat: hir::Pat{kind: hir::PatKind::Ref(.......   can distinguish it from other situation, but I'm not sure.

If my processing method is not accurate, please guide me to modify it, thank you.

r? `@davidtwco`
-rw-r--r--compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs84
-rw-r--r--tests/ui/pattern/issue-114896.rs7
-rw-r--r--tests/ui/pattern/issue-114896.stderr11
3 files changed, 96 insertions, 6 deletions
diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
index 54a557c5145..a0edeec59d0 100644
--- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
+++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
@@ -371,12 +371,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
                     err.span_label(span, format!("cannot {act}"));
                 }
                 if suggest {
-                    err.span_suggestion_verbose(
-                        local_decl.source_info.span.shrink_to_lo(),
-                        "consider changing this to be mutable",
-                        "mut ",
-                        Applicability::MachineApplicable,
-                    );
+                    self.construct_mut_suggestion_for_local_binding_patterns(&mut err, local);
                     let tcx = self.infcx.tcx;
                     if let ty::Closure(id, _) = *the_place_err.ty(self.body, tcx).ty.kind() {
                         self.show_mutating_upvar(tcx, id.expect_local(), the_place_err, &mut err);
@@ -712,6 +707,83 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
         )
     }
 
+    fn construct_mut_suggestion_for_local_binding_patterns(
+        &self,
+        err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>,
+        local: Local,
+    ) {
+        let local_decl = &self.body.local_decls[local];
+        debug!("local_decl: {:?}", local_decl);
+        let pat_span = match *local_decl.local_info() {
+            LocalInfo::User(BindingForm::Var(mir::VarBindingForm {
+                binding_mode: ty::BindingMode::BindByValue(Mutability::Not),
+                opt_ty_info: _,
+                opt_match_place: _,
+                pat_span,
+            })) => pat_span,
+            _ => local_decl.source_info.span,
+        };
+
+        struct BindingFinder {
+            span: Span,
+            hir_id: Option<hir::HirId>,
+        }
+
+        impl<'tcx> Visitor<'tcx> for BindingFinder {
+            fn visit_stmt(&mut self, s: &'tcx hir::Stmt<'tcx>) {
+                if let hir::StmtKind::Local(local) = s.kind {
+                    if local.pat.span == self.span {
+                        self.hir_id = Some(local.hir_id);
+                    }
+                }
+                hir::intravisit::walk_stmt(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 = BindingFinder {
+                span: pat_span,
+                hir_id: None,
+            };
+            v.visit_body(body);
+            v.hir_id
+        } else {
+            None
+        };
+
+        // With ref-binding patterns, the mutability suggestion has to apply to
+        // the binding, not the reference (which would be a type error):
+        //
+        // `let &b = a;` -> `let &(mut b) = a;`
+        if let Some(hir_id) = hir_id
+            && let Some(hir::Node::Local(hir::Local {
+                pat: hir::Pat { kind: hir::PatKind::Ref(_, _), .. },
+                ..
+            })) = hir_map.find(hir_id)
+            && let Ok(name) = self.infcx.tcx.sess.source_map().span_to_snippet(local_decl.source_info.span)
+        {
+            err.span_suggestion(
+                pat_span,
+                "consider changing this to be mutable",
+                format!("&(mut {name})"),
+                Applicability::MachineApplicable,
+            );
+            return;
+        }
+
+        err.span_suggestion_verbose(
+            local_decl.source_info.span.shrink_to_lo(),
+            "consider changing this to be mutable",
+            "mut ",
+            Applicability::MachineApplicable,
+        );
+    }
+
     // point to span of upvar making closure call require mutable borrow
     fn show_mutating_upvar(
         &self,
diff --git a/tests/ui/pattern/issue-114896.rs b/tests/ui/pattern/issue-114896.rs
new file mode 100644
index 00000000000..cde37f658d6
--- /dev/null
+++ b/tests/ui/pattern/issue-114896.rs
@@ -0,0 +1,7 @@
+fn main() {
+    fn x(a: &char) {
+        let &b = a;
+        b.make_ascii_uppercase();
+//~^ cannot borrow `b` as mutable, as it is not declared as mutable
+    }
+}
diff --git a/tests/ui/pattern/issue-114896.stderr b/tests/ui/pattern/issue-114896.stderr
new file mode 100644
index 00000000000..ffeb7bc1365
--- /dev/null
+++ b/tests/ui/pattern/issue-114896.stderr
@@ -0,0 +1,11 @@
+error[E0596]: cannot borrow `b` as mutable, as it is not declared as mutable
+  --> $DIR/issue-114896.rs:4:9
+   |
+LL |         let &b = a;
+   |             -- help: consider changing this to be mutable: `&(mut b)`
+LL |         b.make_ascii_uppercase();
+   |         ^ cannot borrow as mutable
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0596`.