fix handling of self

arielb1 · arielb1 · commit 39011f8590b6 · 2017-03-27T01:37:46.000+03:00
diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs
@@ -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
+                }
+            })
         })
     }
 
diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs
@@ -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)]
diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs
@@ -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;
 
@@ -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 {
@@ -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)
         }
     }
 
@@ -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
                         }
@@ -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);
                             }
                         }
@@ -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);
                     }
                 }
diff --git a/src/test/ui/did_you_mean/issue-39544.rs b/src/test/ui/did_you_mean/issue-39544.rs
@@ -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;
diff --git a/src/test/ui/did_you_mean/issue-39544.stderr b/src/test/ui/did_you_mean/issue-39544.stderr
@@ -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
 

Original file line number	Diff line number	Diff line change
`@@ -1379,6 +1379,9 @@ pub struct FnDecl {`
`1379`	`1379`	`pub inputs: HirVec<P<Ty>>,`
`1380`	`1380`	`pub output: FunctionRetTy,`
`1381`	`1381`	`pub variadic: bool,`
	`1382`	+ /// True if this function has an `self`, `&self` or `&mut self` receiver
	`1383`	+ /// (but not a `self: Xxx` one).
	`1384`	`+ pub has_implicit_self: bool,`
`1382`	`1385`	`}`
`1383`	`1386`
`1384`	`1387`	`#[derive(Copy, Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]`