From 6ba494b68bede3bd8d1288f53137c8895260bec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 11 Mar 2017 11:11:50 -0800 Subject: [PATCH 1/3] Point to let when modifying field of immutable variable Point at the immutable local variable when trying to modify one of its fields. Given a file: ```rust struct Foo { pub v: Vec } fn main() { let f = Foo { v: Vec::new() }; f.v.push("cat".to_string()); } ``` present the following output: ``` error: cannot borrow immutable field `f.v` as mutable --> file.rs:7:13 | 6 | let f = Foo { v: Vec::new() }; | - this should be `mut` 7 | f.v.push("cat".to_string()); | ^^^ error: aborting due to previous error ``` --- src/librustc/middle/mem_categorization.rs | 15 +++++++++++++++ src/librustc_borrowck/borrowck/mod.rs | 12 ++++++++++++ src/test/ui/did_you_mean/issue-39544.stderr | 2 ++ 3 files changed, 29 insertions(+) diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index b0c85e2ef4cd4..7db206296933b 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -195,6 +195,21 @@ pub struct cmt_<'tcx> { pub type cmt<'tcx> = Rc>; impl<'tcx> cmt_<'tcx> { + pub fn get_def(&self) -> Option { + match self.cat { + Categorization::Deref(ref cmt, ..) | + Categorization::Interior(ref cmt, _) | + Categorization::Downcast(ref cmt, _) => { + if let Categorization::Local(nid) = cmt.cat { + Some(nid) + } else { + None + } + } + _ => None + } + } + pub fn get_field(&self, name: ast::Name) -> Option { match self.cat { Categorization::Deref(ref cmt, ..) | diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 47b614a81ae25..04036d6c6b9e7 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -659,6 +659,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { pub fn bckerr_to_diag(&self, err: &BckError<'tcx>) -> DiagnosticBuilder<'a> { let span = err.span.clone(); let mut immutable_field = None; + let mut local_def = None; let msg = &match err.code { err_mutbl => { @@ -708,6 +709,14 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { } None }); + local_def = err.cmt.get_def() + .and_then(|nid| { + if !self.tcx.hir.is_argument(nid) { + Some(self.tcx.hir.span(nid)) + } else { + None + } + }); format!("cannot borrow {} as mutable", descr) } @@ -738,6 +747,9 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { if let Some((span, msg)) = immutable_field { db.span_label(span, &msg); } + if let Some(span) = local_def { + db.span_label(span, &"this should be `mut`"); + } db } diff --git a/src/test/ui/did_you_mean/issue-39544.stderr b/src/test/ui/did_you_mean/issue-39544.stderr index c0088f39ad3e1..1cd322efab57d 100644 --- a/src/test/ui/did_you_mean/issue-39544.stderr +++ b/src/test/ui/did_you_mean/issue-39544.stderr @@ -1,6 +1,8 @@ error: cannot borrow immutable field `z.x` as mutable --> $DIR/issue-39544.rs:21:18 | +20 | let z = Z { x: X::Y }; + | - this should be `mut` 21 | let _ = &mut z.x; | ^^^ From 38b5b29c57fc86aae5a1bc8d1319cc08907d9ee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 12 Mar 2017 13:49:28 -0700 Subject: [PATCH 2/3] Change label to "consider changing this to `mut f`" Change the wording of mutable borrow on immutable binding from "this should be `mut`" to "consider changing this to `mut f`". --- src/librustc_borrowck/borrowck/mod.rs | 4 +++- src/test/ui/did_you_mean/issue-39544.stderr | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 04036d6c6b9e7..073ebe24de924 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -748,7 +748,9 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { db.span_label(span, &msg); } if let Some(span) = local_def { - db.span_label(span, &"this should be `mut`"); + if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(span) { + db.span_label(span, &format!("consider changing this to `mut {}`", snippet)); + } } db } diff --git a/src/test/ui/did_you_mean/issue-39544.stderr b/src/test/ui/did_you_mean/issue-39544.stderr index 1cd322efab57d..3eb3e9a4c3b0a 100644 --- a/src/test/ui/did_you_mean/issue-39544.stderr +++ b/src/test/ui/did_you_mean/issue-39544.stderr @@ -2,7 +2,7 @@ error: cannot borrow immutable field `z.x` as mutable --> $DIR/issue-39544.rs:21:18 | 20 | let z = Z { x: X::Y }; - | - this should be `mut` + | - consider changing this to `mut z` 21 | let _ = &mut z.x; | ^^^ From 9ac628d5e84bd3664e41d53bafa3d66b303b19c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 12 Mar 2017 21:28:49 -0700 Subject: [PATCH 3/3] Add label to primary span for mutable access of immutable struct error --- src/librustc_borrowck/borrowck/mod.rs | 11 ++++++++--- src/test/ui/did_you_mean/issue-39544.stderr | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 073ebe24de924..1b44ba1ec617e 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -747,9 +747,9 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { if let Some((span, msg)) = immutable_field { db.span_label(span, &msg); } - if let Some(span) = local_def { - if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(span) { - db.span_label(span, &format!("consider changing this to `mut {}`", snippet)); + if let Some(let_span) = local_def { + if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) { + db.span_label(let_span, &format!("consider changing this to `mut {}`", snippet)); } } db @@ -1120,6 +1120,11 @@ before rustc 1.16, this temporary lived longer - see issue #39283 \ } else { db.span_label(*error_span, &format!("cannot borrow mutably")); } + } else if let Categorization::Interior(ref cmt, _) = err.cmt.cat { + if let mc::MutabilityCategory::McImmutable = cmt.mutbl { + db.span_label(*error_span, + &"cannot mutably borrow immutable field"); + } } } } diff --git a/src/test/ui/did_you_mean/issue-39544.stderr b/src/test/ui/did_you_mean/issue-39544.stderr index 3eb3e9a4c3b0a..7f124e6d34d35 100644 --- a/src/test/ui/did_you_mean/issue-39544.stderr +++ b/src/test/ui/did_you_mean/issue-39544.stderr @@ -4,7 +4,7 @@ error: cannot borrow immutable field `z.x` as mutable 20 | let z = Z { x: X::Y }; | - consider changing this to `mut z` 21 | let _ = &mut z.x; - | ^^^ + | ^^^ cannot mutably borrow immutable field error: aborting due to previous error