Auto merge of #39139 - estebank:issue-38147, r=nikomatsakis

Point to immutable arg/fields when trying to use as &mut

Present the following output when trying to access an immutable borrow's
field as mutable:

```
error[E0389]: cannot borrow data mutably in a `&` reference
  --> $DIR/issue-38147-1.rs:27:9
   |
26 | fn f(&self) {
   |      -----  use `&mut self` here to make mutable
27 |     f.s.push('x');
   |     ^^^ assignment into an immutable reference
```

And the following when trying to access an immutable struct field as mutable:

```
error: cannot borrow immutable borrowed content `*self.s` as mutable
  --> $DIR/issue-38147-3.rs:17:9
   |
12 |     s: &'a String
   |     ------------- use `&'a mut String` here to make mutable
...|
16 |     fn f(&self) {
   |          -----  use `&mut self` here to make mutable
17 |         self.s.push('x');
   |         ^^^^^^ cannot borrow as mutable
```

Fixes #38147.
This commit is contained in:
bors 2017-01-27 04:57:12 +00:00
commit 025fb7de09
21 changed files with 310 additions and 67 deletions

View File

@ -38,7 +38,7 @@ use syntax_pos::Span;
/// - The default implementation for a trait method.
///
/// To construct one, use the `Code::from_node` function.
#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug)]
pub struct FnLikeNode<'a> { node: map::Node<'a> }
/// MaybeFnLike wraps a method that indicates if an object

View File

@ -194,6 +194,63 @@ pub struct cmt_<'tcx> {
pub type cmt<'tcx> = Rc<cmt_<'tcx>>;
impl<'tcx> cmt_<'tcx> {
pub fn get_field(&self, name: ast::Name) -> Option<DefId> {
match self.cat {
Categorization::Deref(ref cmt, ..) |
Categorization::Interior(ref cmt, _) |
Categorization::Downcast(ref cmt, _) => {
if let Categorization::Local(_) = cmt.cat {
if let ty::TyAdt(def, _) = self.ty.sty {
return def.struct_variant().find_field_named(name).map(|x| x.did);
}
None
} else {
cmt.get_field(name)
}
}
_ => None
}
}
pub fn get_field_name(&self) -> Option<ast::Name> {
match self.cat {
Categorization::Interior(_, ref ik) => {
if let InteriorKind::InteriorField(FieldName::NamedField(name)) = *ik {
Some(name)
} else {
None
}
}
Categorization::Deref(ref cmt, ..) |
Categorization::Downcast(ref cmt, _) => {
cmt.get_field_name()
}
_ => None,
}
}
pub fn get_arg_if_immutable(&self, map: &hir_map::Map) -> Option<ast::NodeId> {
match self.cat {
Categorization::Deref(ref cmt, ..) |
Categorization::Interior(ref cmt, _) |
Categorization::Downcast(ref cmt, _) => {
if let Categorization::Local(nid) = cmt.cat {
if let ty::TyAdt(_, _) = self.ty.sty {
if let ty::TyRef(_, ty::TypeAndMut{mutbl: MutImmutable, ..}) = cmt.ty.sty {
return Some(nid);
}
}
None
} else {
cmt.get_arg_if_immutable(map)
}
}
_ => None
}
}
}
pub trait ast_node {
fn id(&self) -> ast::NodeId;
fn span(&self) -> Span;

View File

@ -1325,6 +1325,7 @@ bitflags! {
}
}
#[derive(Debug)]
pub struct VariantDef {
/// The variant's DefId. If this is a tuple-like struct,
/// this is the DefId of the struct's ctor.
@ -1335,6 +1336,7 @@ pub struct VariantDef {
pub ctor_kind: CtorKind,
}
#[derive(Debug)]
pub struct FieldDef {
pub did: DefId,
pub name: Name,

View File

@ -134,7 +134,7 @@ pub enum TypeVariants<'tcx> {
TyRawPtr(TypeAndMut<'tcx>),
/// A reference; a pointer with an associated lifetime. Written as
/// `&a mut T` or `&'a T`.
/// `&'a mut T` or `&'a T`.
TyRef(&'tcx Region, TypeAndMut<'tcx>),
/// The anonymous type of a function declaration/definition. Each

View File

@ -195,7 +195,8 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
bccx.report_aliasability_violation(
borrow_span,
loan_cause,
mc::AliasableReason::UnaliasableImmutable);
mc::AliasableReason::UnaliasableImmutable,
cmt);
Err(())
}
(mc::Aliasability::FreelyAliasable(alias_cause), ty::UniqueImmBorrow) |
@ -203,7 +204,8 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
bccx.report_aliasability_violation(
borrow_span,
loan_cause,
alias_cause);
alias_cause,
cmt);
Err(())
}
(..) => {

View File

@ -540,7 +540,7 @@ pub fn opt_loan_path<'tcx>(cmt: &mc::cmt<'tcx>) -> Option<Rc<LoanPath<'tcx>>> {
// Errors
// Errors that can occur
#[derive(PartialEq)]
#[derive(Debug, PartialEq)]
pub enum bckerr_code<'tcx> {
err_mutbl,
/// superscope, subscope, loan cause
@ -550,7 +550,7 @@ pub enum bckerr_code<'tcx> {
// Combination of an error code and the categorization of the expression
// that caused it
#[derive(PartialEq)]
#[derive(Debug, PartialEq)]
pub struct BckError<'tcx> {
span: Span,
cause: AliasableViolationKind,
@ -601,12 +601,8 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
_ => { }
}
// General fallback.
let span = err.span.clone();
let mut db = self.struct_span_err(
err.span,
&self.bckerr_to_string(&err));
self.note_and_explain_bckerr(&mut db, err, span);
let mut db = self.bckerr_to_diag(&err);
self.note_and_explain_bckerr(&mut db, err);
db.emit();
}
@ -771,8 +767,11 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
self.tcx.sess.span_err_with_code(s, msg, code);
}
pub fn bckerr_to_string(&self, err: &BckError<'tcx>) -> String {
match err.code {
pub fn bckerr_to_diag(&self, err: &BckError<'tcx>) -> DiagnosticBuilder<'a> {
let span = err.span.clone();
let mut immutable_field = None;
let msg = &match err.code {
err_mutbl => {
let descr = match err.cmt.note {
mc::NoteClosureEnv(_) | mc::NoteUpvarRef(_) => {
@ -783,6 +782,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
format!("{} {}",
err.cmt.mutbl.to_user_str(),
self.cmt_to_string(&err.cmt))
}
Some(lp) => {
format!("{} {} `{}`",
@ -807,6 +807,19 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
BorrowViolation(euv::AutoUnsafe) |
BorrowViolation(euv::ForLoop) |
BorrowViolation(euv::MatchDiscriminant) => {
// Check for this field's definition to see if it is an immutable reference
// and suggest making it mutable if that is the case.
immutable_field = err.cmt.get_field_name()
.and_then(|name| err.cmt.get_field(name))
.and_then(|did| self.tcx.hir.as_local_node_id(did))
.and_then(|nid| {
if let hir_map::Node::NodeField(ref field) = self.tcx.hir.get(nid) {
return self.suggest_mut_for_immutable(&field.ty)
.map(|msg| (self.tcx.hir.span(nid), msg));
}
None
});
format!("cannot borrow {} as mutable", descr)
}
BorrowViolation(euv::ClosureInvocation) => {
@ -830,13 +843,20 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
its contents can be safely reborrowed",
descr)
}
};
let mut db = self.struct_span_err(span, msg);
if let Some((span, msg)) = immutable_field {
db.span_label(span, &msg);
}
db
}
pub fn report_aliasability_violation(&self,
span: Span,
kind: AliasableViolationKind,
cause: mc::AliasableReason) {
cause: mc::AliasableReason,
cmt: mc::cmt<'tcx>) {
let mut is_closure = false;
let prefix = match kind {
MutabilityViolation => {
@ -903,6 +923,9 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
self.tcx.sess, span, E0389,
"{} in a `&` reference", prefix);
e.span_label(span, &"assignment into an immutable reference");
if let Some(nid) = cmt.get_arg_if_immutable(&self.tcx.hir) {
self.immutable_argument_should_be_mut(nid, &mut e);
}
e
}
};
@ -913,6 +936,55 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
err.emit();
}
/// Given a type, if it is an immutable reference, return a suggestion to make it mutable
fn suggest_mut_for_immutable(&self, pty: &hir::Ty) -> Option<String> {
// Check wether the argument is an immutable reference
if let hir::TyRptr(opt_lifetime, hir::MutTy {
mutbl: hir::Mutability::MutImmutable,
ref ty
}) = pty.node {
// Account for existing lifetimes when generating the message
if let Some(lifetime) = opt_lifetime {
if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(ty.span) {
if let Ok(lifetime_snippet) = self.tcx.sess.codemap()
.span_to_snippet(lifetime.span) {
return Some(format!("use `&{} mut {}` here to make mutable",
lifetime_snippet,
snippet));
}
}
} else if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(pty.span) {
if snippet.starts_with("&") {
return Some(format!("use `{}` here to make mutable",
snippet.replace("&", "&mut ")));
}
} else {
bug!("couldn't find a snippet for span: {:?}", pty.span);
}
}
None
}
fn immutable_argument_should_be_mut(&self, nid: ast::NodeId, db: &mut DiagnosticBuilder) {
let parent = self.tcx.hir.get_parent_node(nid);
let parent_node = self.tcx.hir.get(parent);
// The parent node is like a fn
if let Some(fn_like) = FnLikeNode::from_node(parent_node) {
// `nid`'s parent's `Body`
let fn_body = self.tcx.hir.body(fn_like.body());
// Get the position of `nid` in the arguments list
let arg_pos = fn_body.arguments.iter().position(|arg| arg.pat.id == nid);
if let Some(i) = arg_pos {
// The argument's `Ty`
let arg_ty = &fn_like.decl().inputs[i];
if let Some(msg) = self.suggest_mut_for_immutable(&arg_ty) {
db.span_label(arg_ty.span, &msg);
}
}
}
}
fn report_out_of_scope_escaping_closure_capture(&self,
err: &BckError<'tcx>,
capture_span: Span)
@ -961,8 +1033,8 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
}
}
pub fn note_and_explain_bckerr(&self, db: &mut DiagnosticBuilder, err: BckError<'tcx>,
error_span: Span) {
pub fn note_and_explain_bckerr(&self, db: &mut DiagnosticBuilder, err: BckError<'tcx>) {
let error_span = err.span.clone();
match err.code {
err_mutbl => self.note_and_explain_mutbl_error(db, &err, &error_span),
err_out_of_scope(super_scope, sub_scope, cause) => {
@ -1114,41 +1186,13 @@ before rustc 1.16, this temporary lived longer - see issue #39283 \
}
}
_ => {
if let Categorization::Deref(ref inner_cmt, ..) = err.cmt.cat {
if let Categorization::Local(local_id) = inner_cmt.cat {
let parent = self.tcx.hir.get_parent_node(local_id);
if let Some(fn_like) = FnLikeNode::from_node(self.tcx.hir.get(parent)) {
if let Some(i) = self.tcx.hir.body(fn_like.body()).arguments.iter()
.position(|arg| arg.pat.id == local_id) {
let arg_ty = &fn_like.decl().inputs[i];
if let hir::TyRptr(
opt_lifetime,
hir::MutTy{mutbl: hir::Mutability::MutImmutable, ref ty}) =
arg_ty.node {
if let Some(lifetime) = opt_lifetime {
if let Ok(snippet) = self.tcx.sess.codemap()
.span_to_snippet(ty.span) {
if let Ok(lifetime_snippet) = self.tcx.sess.codemap()
.span_to_snippet(lifetime.span) {
db.span_label(arg_ty.span,
&format!("use `&{} mut {}` \
here to make mutable",
lifetime_snippet,
snippet));
}
}
}
else if let Ok(snippet) = self.tcx.sess.codemap()
.span_to_snippet(arg_ty.span) {
if snippet.starts_with("&") {
db.span_label(arg_ty.span,
&format!("use `{}` here to make mutable",
snippet.replace("&", "&mut ")));
}
}
}
}
if let Categorization::Deref(..) = err.cmt.cat {
db.span_label(*error_span, &"cannot borrow as mutable");
if let Some(local_id) = err.cmt.get_arg_if_immutable(&self.tcx.hir) {
self.immutable_argument_should_be_mut(local_id, db);
} else if let Categorization::Deref(ref inner_cmt, ..) = err.cmt.cat {
if let Categorization::Local(local_id) = inner_cmt.cat {
self.immutable_argument_should_be_mut(local_id, db);
}
}
} else if let Categorization::Local(local_id) = err.cmt.cat {

View File

@ -0,0 +1,31 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
struct Pass<'a> {
s: &'a mut String
}
impl<'a> Pass<'a> {
fn f(&mut self) {
self.s.push('x');
}
}
struct Foo<'a> {
s: &'a mut String
}
impl<'a> Foo<'a> {
fn f(&self) {
self.s.push('x');
}
}
fn main() {}

View File

@ -0,0 +1,10 @@
error[E0389]: cannot borrow data mutably in a `&` reference
--> $DIR/issue-38147-1.rs:27:9
|
26 | fn f(&self) {
| ----- use `&mut self` here to make mutable
27 | self.s.push('x');
| ^^^^^^ assignment into an immutable reference
error: aborting due to previous error

View File

@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
struct Bar<'a> {
s: &'a String
}
impl<'a> Bar<'a> {
fn f(&mut self) {
self.s.push('x');
}
}
fn main() {}

View File

@ -0,0 +1,11 @@
error: cannot borrow immutable borrowed content `*self.s` as mutable
--> $DIR/issue-38147-2.rs:17:9
|
12 | s: &'a String
| ------------- use `&'a mut String` here to make mutable
...
17 | self.s.push('x');
| ^^^^^^ cannot borrow as mutable
error: aborting due to previous error

View File

@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
struct Qux<'a> {
s: &'a String
}
impl<'a> Qux<'a> {
fn f(&self) {
self.s.push('x');
}
}
fn main() {}

View File

@ -0,0 +1,13 @@
error: cannot borrow immutable borrowed content `*self.s` as mutable
--> $DIR/issue-38147-3.rs:17:9
|
12 | s: &'a String
| ------------- use `&'a mut String` here to make mutable
...
16 | fn f(&self) {
| ----- use `&mut self` here to make mutable
17 | self.s.push('x');
| ^^^^^^ cannot borrow as mutable
error: aborting due to previous error

View File

@ -0,0 +1,19 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
struct Foo<'a> {
s: &'a mut String
}
fn f(x: usize, f: &Foo) {
f.s.push('x');
}
fn main() {}

View File

@ -0,0 +1,10 @@
error[E0389]: cannot borrow data mutably in a `&` reference
--> $DIR/issue-38147-4.rs:16:5
|
15 | fn f(x: usize, f: &Foo) {
| ---- use `&mut Foo` here to make mutable
16 | f.s.push('x');
| ^^^ assignment into an immutable reference
error: aborting due to previous error

View File

@ -12,7 +12,7 @@ error: cannot borrow immutable borrowed content `*x` as mutable
74 | fn deref_extend_mut_field1(x: &Own<Point>) -> &mut isize {
| ----------- use `&mut Own<Point>` here to make mutable
75 | &mut x.y //~ ERROR cannot borrow
| ^
| ^ cannot borrow as mutable
error[E0499]: cannot borrow `*x` as mutable more than once at a time
--> $DIR/borrowck-borrow-overloaded-auto-deref-mut.rs:88:19
@ -38,7 +38,7 @@ error: cannot borrow immutable borrowed content `*x` as mutable
101 | fn assign_field2<'a>(x: &'a Own<Point>) {
| -------------- use `&'a mut Own<Point>` here to make mutable
102 | x.y = 3; //~ ERROR cannot borrow
| ^
| ^ cannot borrow as mutable
error[E0499]: cannot borrow `*x` as mutable more than once at a time
--> $DIR/borrowck-borrow-overloaded-auto-deref-mut.rs:111:5
@ -64,7 +64,7 @@ error: cannot borrow immutable borrowed content `*x` as mutable
130 | fn deref_extend_mut_method1(x: &Own<Point>) -> &mut isize {
| ----------- use `&mut Own<Point>` here to make mutable
131 | x.y_mut() //~ ERROR cannot borrow
| ^
| ^ cannot borrow as mutable
error: cannot borrow immutable argument `x` as mutable
--> $DIR/borrowck-borrow-overloaded-auto-deref-mut.rs:139:6
@ -80,7 +80,7 @@ error: cannot borrow immutable borrowed content `*x` as mutable
142 | fn assign_method2<'a>(x: &'a Own<Point>) {
| -------------- use `&'a mut Own<Point>` here to make mutable
143 | *x.y_mut() = 3; //~ ERROR cannot borrow
| ^
| ^ cannot borrow as mutable
error: aborting due to 10 previous errors

View File

@ -12,7 +12,7 @@ error: cannot borrow immutable borrowed content `*x` as mutable
50 | fn deref_extend_mut1<'a>(x: &'a Own<isize>) -> &'a mut isize {
| -------------- use `&'a mut Own<isize>` here to make mutable
51 | &mut **x //~ ERROR cannot borrow
| ^^
| ^^ cannot borrow as mutable
error: cannot borrow immutable argument `x` as mutable
--> $DIR/borrowck-borrow-overloaded-deref-mut.rs:59:6
@ -28,7 +28,7 @@ error: cannot borrow immutable borrowed content `*x` as mutable
62 | fn assign2<'a>(x: &'a Own<isize>) {
| -------------- use `&'a mut Own<isize>` here to make mutable
63 | **x = 3; //~ ERROR cannot borrow
| ^^
| ^^ cannot borrow as mutable
error: aborting due to 4 previous errors

View File

@ -17,13 +17,15 @@ error: cannot borrow immutable borrowed content `*f` as mutable
35 | fn test2<F>(f: &F) where F: FnMut() {
| -- use `&mut F` here to make mutable
36 | (*f)(); //~ ERROR: cannot borrow immutable borrowed content `*f` as mutable
| ^^^^
| ^^^^ cannot borrow as mutable
error: cannot borrow immutable `Box` content `*f.f` as mutable
--> $DIR/borrowck-call-is-borrow-issue-12224.rs:44:5
|
43 | fn test4(f: &Test) {
| ----- use `&mut Test` here to make mutable
44 | f.f.call_mut(()) //~ ERROR: cannot borrow immutable `Box` content `*f.f` as mutable
| ^^^
| ^^^ cannot borrow as mutable
error[E0504]: cannot move `f` into closure because it is borrowed
--> $DIR/borrowck-call-is-borrow-issue-12224.rs:63:13

View File

@ -5,7 +5,7 @@ error: cannot borrow immutable borrowed content `*x` as mutable
| ---- use `&mut Foo` here to make mutable
26 | x.f();
27 | x.h(); //~ ERROR cannot borrow
| ^
| ^ cannot borrow as mutable
error: aborting due to previous error

View File

@ -4,7 +4,7 @@ error: cannot borrow immutable borrowed content `*x` as mutable
16 | fn broken(x: &Vec<String>) {
| ------------ use `&mut Vec<String>` here to make mutable
17 | x.push(format!("this is broken"));
| ^
| ^ cannot borrow as mutable
error: aborting due to previous error

View File

@ -5,13 +5,13 @@ error: cannot borrow immutable borrowed content `*x` as mutable
| ---- use `&mut Foo` here to make mutable
18 | x.borrowed();
19 | x.borrowed_mut(); //~ ERROR cannot borrow
| ^
| ^ cannot borrow as mutable
error: cannot borrow immutable `Box` content `*x` as mutable
--> $DIR/borrowck-object-mutability.rs:29:5
|
29 | x.borrowed_mut(); //~ ERROR cannot borrow
| ^
| ^ cannot borrow as mutable
error: aborting due to 2 previous errors

View File

@ -4,7 +4,7 @@ error: cannot borrow immutable borrowed content `*a` as mutable
17 | pub fn foo<'a>(mut a: &'a String) {
| ---------- use `&'a mut String` here to make mutable
18 | a.push_str("foo");
| ^
| ^ cannot borrow as mutable
error: cannot borrow immutable borrowed content `*a` as mutable
--> $DIR/mut-arg-hint.rs:13:9
@ -12,7 +12,7 @@ error: cannot borrow immutable borrowed content `*a` as mutable
12 | fn foo(mut a: &String) {
| ------- use `&mut String` here to make mutable
13 | a.push_str("bar");
| ^
| ^ cannot borrow as mutable
error: cannot borrow immutable borrowed content `*a` as mutable
--> $DIR/mut-arg-hint.rs:25:9
@ -20,7 +20,7 @@ error: cannot borrow immutable borrowed content `*a` as mutable
24 | pub fn foo(mut a: &String) {
| ------- use `&mut String` here to make mutable
25 | a.push_str("foo");
| ^
| ^ cannot borrow as mutable
error: aborting due to 3 previous errors