diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 9d0d8c2f909af..e063880028fc9 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -899,7 +899,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(), err.cmt.id); db.emit(); } err_out_of_scope(super_scope, sub_scope, cause) => { @@ -1105,7 +1105,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { Origin::Ast) } }; - self.note_immutability_blame(&mut err, blame); + self.note_immutability_blame(&mut err, blame, cmt.id); if is_closure { err.help("closures behind references must be called via `&mut`"); @@ -1184,25 +1184,13 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { fn note_immutability_blame(&self, db: &mut DiagnosticBuilder, - blame: Option) { + blame: Option, + error_node_id: ast::NodeId) { 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 is_implicit_self && snippet != "self" { - // avoid suggesting `mut &self`. - return - } - db.span_label( - let_span, - format!("consider changing this to `mut {}`", snippet) - ); - } - } + self.note_immutable_local(db, error_node_id, node_id) } Some(ImmutabilityBlame::LocalDeref(node_id)) => { let let_span = self.tcx.hir.span(node_id); @@ -1242,6 +1230,46 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { } } + // Suggest a fix when trying to mutably borrow an immutable local + // binding: either to make the binding mutable (if its type is + // not a mutable reference) or to avoid borrowing altogether + fn note_immutable_local(&self, + db: &mut DiagnosticBuilder, + borrowed_node_id: ast::NodeId, + binding_node_id: ast::NodeId) { + let let_span = self.tcx.hir.span(binding_node_id); + if let ty::BindByValue(..) = self.local_binding_mode(binding_node_id) { + if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) { + let (ty, is_implicit_self) = self.local_ty(binding_node_id); + if is_implicit_self && snippet != "self" { + // avoid suggesting `mut &self`. + return + } + if let Some(&hir::TyRptr( + _, + hir::MutTy { + mutbl: hir::MutMutable, + .. + }, + )) = ty.map(|t| &t.node) + { + let borrow_expr_id = self.tcx.hir.get_parent_node(borrowed_node_id); + db.span_suggestion( + self.tcx.hir.span(borrow_expr_id), + "consider removing the `&mut`, as it is an \ + immutable binding to a mutable reference", + snippet + ); + } else { + db.span_label( + let_span, + format!("consider changing this to `mut {}`", snippet), + ); + } + } + } + } + fn report_out_of_scope_escaping_closure_capture(&self, err: &BckError<'a, 'tcx>, capture_span: Span) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 28f93328e9534..95d519eae5851 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5190,36 +5190,36 @@ impl<'a> Parser<'a> { // Only a limited set of initial token sequences is considered self parameters, anything // else is parsed as a normal function parameter list, so some lookahead is required. let eself_lo = self.span; - let (eself, eself_ident) = match self.token { + let (eself, eself_ident, eself_hi) = match self.token { token::BinOp(token::And) => { // &self // &mut self // &'lt self // &'lt mut self // ¬_self - if isolated_self(self, 1) { + (if isolated_self(self, 1) { self.bump(); - (SelfKind::Region(None, Mutability::Immutable), expect_ident(self)) + SelfKind::Region(None, Mutability::Immutable) } else if self.look_ahead(1, |t| t.is_keyword(keywords::Mut)) && isolated_self(self, 2) { self.bump(); self.bump(); - (SelfKind::Region(None, Mutability::Mutable), expect_ident(self)) + SelfKind::Region(None, Mutability::Mutable) } else if self.look_ahead(1, |t| t.is_lifetime()) && isolated_self(self, 2) { self.bump(); let lt = self.expect_lifetime(); - (SelfKind::Region(Some(lt), Mutability::Immutable), expect_ident(self)) + SelfKind::Region(Some(lt), Mutability::Immutable) } else if self.look_ahead(1, |t| t.is_lifetime()) && self.look_ahead(2, |t| t.is_keyword(keywords::Mut)) && isolated_self(self, 3) { self.bump(); let lt = self.expect_lifetime(); self.bump(); - (SelfKind::Region(Some(lt), Mutability::Mutable), expect_ident(self)) + SelfKind::Region(Some(lt), Mutability::Mutable) } else { return Ok(None); - } + }, expect_ident(self), self.prev_span) } token::BinOp(token::Star) => { // *self @@ -5227,43 +5227,45 @@ impl<'a> Parser<'a> { // *mut self // *not_self // Emit special error for `self` cases. - if isolated_self(self, 1) { + (if isolated_self(self, 1) { self.bump(); self.span_err(self.span, "cannot pass `self` by raw pointer"); - (SelfKind::Value(Mutability::Immutable), expect_ident(self)) + SelfKind::Value(Mutability::Immutable) } else if self.look_ahead(1, |t| t.is_mutability()) && isolated_self(self, 2) { self.bump(); self.bump(); self.span_err(self.span, "cannot pass `self` by raw pointer"); - (SelfKind::Value(Mutability::Immutable), expect_ident(self)) + SelfKind::Value(Mutability::Immutable) } else { return Ok(None); - } + }, expect_ident(self), self.prev_span) } token::Ident(..) => { if isolated_self(self, 0) { // self // self: TYPE let eself_ident = expect_ident(self); - if self.eat(&token::Colon) { + let eself_hi = self.prev_span; + (if self.eat(&token::Colon) { let ty = self.parse_ty()?; - (SelfKind::Explicit(ty, Mutability::Immutable), eself_ident) + SelfKind::Explicit(ty, Mutability::Immutable) } else { - (SelfKind::Value(Mutability::Immutable), eself_ident) - } + SelfKind::Value(Mutability::Immutable) + }, eself_ident, eself_hi) } else if self.token.is_keyword(keywords::Mut) && isolated_self(self, 1) { // mut self // mut self: TYPE self.bump(); let eself_ident = expect_ident(self); - if self.eat(&token::Colon) { + let eself_hi = self.prev_span; + (if self.eat(&token::Colon) { let ty = self.parse_ty()?; - (SelfKind::Explicit(ty, Mutability::Mutable), eself_ident) + SelfKind::Explicit(ty, Mutability::Mutable) } else { - (SelfKind::Value(Mutability::Mutable), eself_ident) - } + SelfKind::Value(Mutability::Mutable) + }, eself_ident, eself_hi) } else { return Ok(None); } @@ -5271,7 +5273,7 @@ impl<'a> Parser<'a> { _ => return Ok(None), }; - let eself = codemap::respan(eself_lo.to(self.prev_span), eself); + let eself = codemap::respan(eself_lo.to(eself_hi), eself); Ok(Some(Arg::from_self(eself, eself_ident))) } diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.nll.stderr b/src/test/ui/borrowck/mut-borrow-of-mut-ref.nll.stderr new file mode 100644 index 0000000000000..f8b84bce04ecf --- /dev/null +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.nll.stderr @@ -0,0 +1,9 @@ +error[E0596]: cannot borrow immutable item `b` as mutable + --> $DIR/mut-borrow-of-mut-ref.rs:18:7 + | +LL | g(&mut b) //~ ERROR cannot borrow + | ^^^^^^ cannot borrow as mutable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs b/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs new file mode 100644 index 0000000000000..75b9da5202341 --- /dev/null +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.rs @@ -0,0 +1,21 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Suggest not mutably borrowing a mutable reference + +fn main() { + f(&mut 0) +} + +fn f(b: &mut i32) { + g(&mut b) //~ ERROR cannot borrow +} + +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 new file mode 100644 index 0000000000000..885164cdc0603 --- /dev/null +++ b/src/test/ui/borrowck/mut-borrow-of-mut-ref.stderr @@ -0,0 +1,13 @@ +error[E0596]: cannot borrow immutable argument `b` as mutable + --> $DIR/mut-borrow-of-mut-ref.rs:18:12 + | +LL | g(&mut b) //~ ERROR cannot borrow + | ^ cannot borrow mutably +help: consider removing the `&mut`, as it is an immutable binding to a mutable reference + | +LL | g(b) //~ ERROR cannot borrow + | ^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. diff --git a/src/test/ui/did_you_mean/issue-31424.stderr b/src/test/ui/did_you_mean/issue-31424.stderr index a80593e05f105..9d0ab21ffaf0e 100644 --- a/src/test/ui/did_you_mean/issue-31424.stderr +++ b/src/test/ui/did_you_mean/issue-31424.stderr @@ -10,10 +10,12 @@ LL | (&mut self).bar(); //~ ERROR cannot borrow error[E0596]: cannot borrow immutable argument `self` as mutable --> $DIR/issue-31424.rs:23:15 | -LL | fn bar(self: &mut Self) { - | --------------- consider changing this to `mut self: &mut Self` LL | (&mut self).bar(); //~ ERROR cannot borrow | ^^^^ cannot borrow mutably +help: consider removing the `&mut`, as it is an immutable binding to a mutable reference + | +LL | self.bar(); //~ ERROR cannot borrow + | ^^^^ error: aborting due to 2 previous errors