From 3f1d055066a85cbed39bbacb4694d726703c88c5 Mon Sep 17 00:00:00 2001 From: Kurtis Nusbaum Date: Sat, 3 Mar 2018 13:16:55 -0800 Subject: [PATCH 1/2] Provide better borrow checker error message --- src/librustc_borrowck/borrowck/mod.rs | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 93d6247eeae47..e4b366f3a69ff 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -896,7 +896,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { }; self.note_and_explain_mutbl_error(&mut db, &err, &error_span); - self.note_immutability_blame(&mut db, err.cmt.immutability_blame()); + self.note_immutability_blame(&mut db, err.cmt.immutability_blame(), error_span); db.emit(); } err_out_of_scope(super_scope, sub_scope, cause) => { @@ -1102,7 +1102,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { Origin::Ast) } }; - self.note_immutability_blame(&mut err, blame); + self.note_immutability_blame(&mut err, blame, span); if is_closure { err.help("closures behind references must be called via `&mut`"); @@ -1181,15 +1181,27 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { fn note_immutability_blame(&self, db: &mut DiagnosticBuilder, - blame: Option) { + blame: Option, + error_span: Span) { match blame { None => {} Some(ImmutabilityBlame::ClosureEnv(_)) => {} Some(ImmutabilityBlame::ImmLocal(node_id)) => { let let_span = self.tcx.hir.span(node_id); - if let ty::BindByValue(..) = self.local_binding_mode(node_id) { - if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) { - let (_, is_implicit_self) = self.local_ty(node_id); + if let ty::BindByValue(bind_value_mut) = self.local_binding_mode(node_id) { + let (local_node_ty, is_implicit_self) = self.local_ty(node_id); + if let Some(local_node_ty) = local_node_ty { + if let hir::Ty_::TyPtr(ref ty_ptr_mutability) = local_node_ty.node { + if ty_ptr_mutability.mutbl == bind_value_mut { + if let Ok(snippet) =self.tcx.sess.codemap().span_to_snippet(let_span) { + db.span_suggestion( + error_span, + "consider removing the `&mut`, as it is an immutable binding to a mutable reference", + snippet); + } + } + } + } else if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) { if is_implicit_self && snippet != "self" { // avoid suggesting `mut &self`. return From 33ff8fcbbdcad4609a6922c1370d5552e9805a3a Mon Sep 17 00:00:00 2001 From: Kurtis Nusbaum Date: Sun, 1 Apr 2018 10:39:26 -0700 Subject: [PATCH 2/2] restructure some code and add first test --- src/librustc_borrowck/borrowck/mod.rs | 9 ++++----- src/test/ui/unnecessary_mut_reference.rs | 18 ++++++++++++++++++ src/test/ui/unnecessary_mut_reference.stderr | 9 +++++++++ 3 files changed, 31 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/unnecessary_mut_reference.rs create mode 100644 src/test/ui/unnecessary_mut_reference.stderr diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index e4b366f3a69ff..c88b5a8f5dccc 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -1191,15 +1191,14 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { if let ty::BindByValue(bind_value_mut) = self.local_binding_mode(node_id) { let (local_node_ty, is_implicit_self) = self.local_ty(node_id); if let Some(local_node_ty) = local_node_ty { - if let hir::Ty_::TyPtr(ref ty_ptr_mutability) = local_node_ty.node { - if ty_ptr_mutability.mutbl == bind_value_mut { - if let Ok(snippet) =self.tcx.sess.codemap().span_to_snippet(let_span) { + match (local_node_ty.node, self.tcx.sess.codemap().span_to_snippet(let_span)) { + (hir::Ty_::TyPtr(ref ty_ptr_mutability), Ok(snippet)) if ty_ptr_mutability.mutbl == bind_value_mut => { db.span_suggestion( error_span, "consider removing the `&mut`, as it is an immutable binding to a mutable reference", snippet); - } - } + }, + _ => {} } } else if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) { if is_implicit_self && snippet != "self" { diff --git a/src/test/ui/unnecessary_mut_reference.rs b/src/test/ui/unnecessary_mut_reference.rs new file mode 100644 index 0000000000000..c6c0409031074 --- /dev/null +++ b/src/test/ui/unnecessary_mut_reference.rs @@ -0,0 +1,18 @@ +struct User { + name: String, +} + +fn main() { + let mut user1 = User{ + name: String::from("Kurtis"), + }; + mutator_part1(&mut user1); +} + +fn mutator_part1(user: &mut User) { + mutator_part2(&mut user); +} + +fn mutator_part2(user: &mut User) { + user.name = String::from("Steve"); +} diff --git a/src/test/ui/unnecessary_mut_reference.stderr b/src/test/ui/unnecessary_mut_reference.stderr new file mode 100644 index 0000000000000..c805d5b4c6fff --- /dev/null +++ b/src/test/ui/unnecessary_mut_reference.stderr @@ -0,0 +1,9 @@ +error[E0596]: cannot borrow immutable argument `user` as mutable + --> unnecessary_mut_reference.rs:13:24 + | +12 | fn mutator_part1(user: &mut User) { +13 | mutator_part2(&mut user); + | ^^^^ cannot borrow mutably + | --------- consider removing the `&mut`, as it is an immutable binding to a mutable reference + +error: aborting due to previous error