Skip to content

Commit

Permalink
fix handling of self
Browse files Browse the repository at this point in the history
  • Loading branch information
arielb1 committed Mar 26, 2017
1 parent 50728e5 commit 39011f8
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 38 deletions.
7 changes: 7 additions & 0 deletions src/librustc/hir/lowering.rs
Expand Up @@ -907,6 +907,13 @@ impl<'a> LoweringContext<'a> {
FunctionRetTy::Default(span) => hir::DefaultReturn(span),
},
variadic: decl.variadic,
has_implicit_self: decl.inputs.get(0).map_or(false, |arg| {
match arg.ty.node {
TyKind::ImplicitSelf => true,
TyKind::Rptr(_, ref mt) => mt.ty.node == TyKind::ImplicitSelf,
_ => false
}
})
})
}

Expand Down
3 changes: 3 additions & 0 deletions src/librustc/hir/mod.rs
Expand Up @@ -1379,6 +1379,9 @@ pub struct FnDecl {
pub inputs: HirVec<P<Ty>>,
pub output: FunctionRetTy,
pub variadic: bool,
/// True if this function has an `self`, `&self` or `&mut self` receiver
/// (but not a `self: Xxx` one).
pub has_implicit_self: bool,
}

#[derive(Copy, Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
Expand Down
60 changes: 31 additions & 29 deletions src/librustc_borrowck/borrowck/mod.rs
Expand Up @@ -42,7 +42,6 @@ use std::fmt;
use std::rc::Rc;
use std::hash::{Hash, Hasher};
use syntax::ast;
use syntax::symbol::keywords;
use syntax_pos::{MultiSpan, Span};
use errors::DiagnosticBuilder;

Expand Down Expand Up @@ -809,32 +808,33 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
}

/// 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> {
fn suggest_mut_for_immutable(&self, pty: &hir::Ty, is_implicit_self: bool) -> Option<String> {
// Check wether the argument is an immutable reference
debug!("suggest_mut_for_immutable({:?}, {:?})", pty, is_implicit_self);
if let hir::TyRptr(lifetime, hir::MutTy {
mutbl: hir::Mutability::MutImmutable,
ref ty
}) = pty.node {
// Account for existing lifetimes when generating the message
if !lifetime.is_elided() {
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 ")));
}
let pointee_snippet = match self.tcx.sess.codemap().span_to_snippet(ty.span) {
Ok(snippet) => snippet,
_ => return None
};

let lifetime_snippet = if !lifetime.is_elided() {
format!("{} ", match self.tcx.sess.codemap().span_to_snippet(lifetime.span) {
Ok(lifetime_snippet) => lifetime_snippet,
_ => return None
})
} else {
bug!("couldn't find a snippet for span: {:?}", pty.span);
}
String::new()
};
Some(format!("use `&{}mut {}` here to make mutable",
lifetime_snippet,
if is_implicit_self { "self" } else { &*pointee_snippet }))
} else {
None
}
None
}

fn local_binding_mode(&self, node_id: ast::NodeId) -> hir::BindingMode {
Expand All @@ -849,24 +849,25 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
}
}

fn local_ty(&self, node_id: ast::NodeId) -> Option<&hir::Ty> {
fn local_ty(&self, node_id: ast::NodeId) -> (Option<&hir::Ty>, bool) {
let parent = self.tcx.hir.get_parent_node(node_id);
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
// Get the position of `node_id` in the arguments list
let arg_pos = fn_body.arguments.iter().position(|arg| arg.pat.id == node_id);
if let Some(i) = arg_pos {
// The argument's `Ty`
Some(&fn_like.decl().inputs[i])
(Some(&fn_like.decl().inputs[i]),
i == 0 && fn_like.decl().has_implicit_self)
} else {
None
(None, false)
}
} else {
None
(None, false)
}
}

Expand All @@ -880,8 +881,8 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
let let_span = self.tcx.hir.span(node_id);
if let hir::BindingMode::BindByValue(..) = self.local_binding_mode(node_id) {
if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(let_span) {
if self.tcx.hir.name(node_id) == keywords::SelfValue.name() &&
snippet != "self" {
let (_, is_implicit_self) = self.local_ty(node_id);
if is_implicit_self && snippet != "self" {
// avoid suggesting `mut &self`.
return
}
Expand All @@ -906,8 +907,9 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
}
}
hir::BindingMode::BindByValue(..) => {
if let Some(local_ty) = self.local_ty(node_id) {
if let Some(msg) = self.suggest_mut_for_immutable(local_ty) {
if let (Some(local_ty), is_implicit_self) = self.local_ty(node_id) {
if let Some(msg) =
self.suggest_mut_for_immutable(local_ty, is_implicit_self) {
db.span_label(local_ty.span, &msg);
}
}
Expand All @@ -921,7 +923,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
};

if let hir_map::Node::NodeField(ref field) = self.tcx.hir.get(node_id) {
if let Some(msg) = self.suggest_mut_for_immutable(&field.ty) {
if let Some(msg) = self.suggest_mut_for_immutable(&field.ty, false) {
db.span_label(field.ty.span, &msg);
}
}
Expand Down
28 changes: 27 additions & 1 deletion src/test/ui/did_you_mean/issue-39544.rs
Expand Up @@ -16,11 +16,37 @@ pub struct Z {
x: X
}

pub fn main() {
fn main() {
let z = Z { x: X::Y };
let _ = &mut z.x;
}

impl Z {
fn foo<'z>(&'z self) {
let _ = &mut self.x;
}

fn foo1(&self, other: &Z) {
let _ = &mut self.x;
let _ = &mut other.x;
}

fn foo2<'a>(&'a self, other: &Z) {
let _ = &mut self.x;
let _ = &mut other.x;
}

fn foo3<'a>(self: &'a Self, other: &Z) {
let _ = &mut self.x;
let _ = &mut other.x;
}

fn foo4(other: &Z) {
let _ = &mut other.x;
}

}

pub fn with_arg(z: Z, w: &Z) {
let _ = &mut z.x;
let _ = &mut w.x;
Expand Down
83 changes: 75 additions & 8 deletions src/test/ui/did_you_mean/issue-39544.stderr
Expand Up @@ -6,22 +6,89 @@ error: cannot borrow immutable field `z.x` as mutable
21 | let _ = &mut z.x;
| ^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `self.x` as mutable
--> $DIR/issue-39544.rs:26:22
|
25 | fn foo<'z>(&'z self) {
| -------- use `&'z mut self` here to make mutable
26 | let _ = &mut self.x;
| ^^^^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `self.x` as mutable
--> $DIR/issue-39544.rs:30:22
|
29 | fn foo1(&self, other: &Z) {
| ----- use `&mut self` here to make mutable
30 | let _ = &mut self.x;
| ^^^^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `other.x` as mutable
--> $DIR/issue-39544.rs:31:22
|
29 | fn foo1(&self, other: &Z) {
| -- use `&mut Z` here to make mutable
30 | let _ = &mut self.x;
31 | let _ = &mut other.x;
| ^^^^^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `self.x` as mutable
--> $DIR/issue-39544.rs:35:22
|
34 | fn foo2<'a>(&'a self, other: &Z) {
| -------- use `&'a mut self` here to make mutable
35 | let _ = &mut self.x;
| ^^^^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `other.x` as mutable
--> $DIR/issue-39544.rs:36:22
|
34 | fn foo2<'a>(&'a self, other: &Z) {
| -- use `&mut Z` here to make mutable
35 | let _ = &mut self.x;
36 | let _ = &mut other.x;
| ^^^^^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `self.x` as mutable
--> $DIR/issue-39544.rs:40:22
|
39 | fn foo3<'a>(self: &'a Self, other: &Z) {
| -------- use `&'a mut Self` here to make mutable
40 | let _ = &mut self.x;
| ^^^^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `other.x` as mutable
--> $DIR/issue-39544.rs:41:22
|
39 | fn foo3<'a>(self: &'a Self, other: &Z) {
| -- use `&mut Z` here to make mutable
40 | let _ = &mut self.x;
41 | let _ = &mut other.x;
| ^^^^^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `other.x` as mutable
--> $DIR/issue-39544.rs:45:22
|
44 | fn foo4(other: &Z) {
| -- use `&mut Z` here to make mutable
45 | let _ = &mut other.x;
| ^^^^^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `z.x` as mutable
--> $DIR/issue-39544.rs:25:18
--> $DIR/issue-39544.rs:51:18
|
24 | pub fn with_arg(z: Z, w: &Z) {
50 | pub fn with_arg(z: Z, w: &Z) {
| - consider changing this to `mut z`
25 | let _ = &mut z.x;
51 | let _ = &mut z.x;
| ^^^ cannot mutably borrow immutable field

error: cannot borrow immutable field `w.x` as mutable
--> $DIR/issue-39544.rs:26:18
--> $DIR/issue-39544.rs:52:18
|
24 | pub fn with_arg(z: Z, w: &Z) {
50 | pub fn with_arg(z: Z, w: &Z) {
| -- use `&mut Z` here to make mutable
25 | let _ = &mut z.x;
26 | let _ = &mut w.x;
51 | let _ = &mut z.x;
52 | let _ = &mut w.x;
| ^^^ cannot mutably borrow immutable field

error: aborting due to 3 previous errors
error: aborting due to 11 previous errors

0 comments on commit 39011f8

Please sign in to comment.