Auto merge of #33505 - petrochenkov:self, r=nrc

Remove ExplicitSelf from HIR

`self` argument is already kept in the argument list and can be retrieved from there if necessary, so there's no need for the duplication.
The same changes can be applied to AST, I'll make them in the next breaking batch.
The first commit also improves parsing of method declarations and fixes https://github.com/rust-lang/rust/issues/33413.

r? @eddyb

3 files changed, 182 insertions, 198 deletions

diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
index e9f3472c4a1..0f3b9be3a7a 100644
--- a/src/libsyntax/ast.rs
+++ b/src/libsyntax/ast.rs
@@ -16,13 +16,12 @@ pub use self::ViewPath_::*;
 pub use self::PathParameters::*;
 
 use attr::ThinAttributes;
-use codemap::{Span, Spanned, DUMMY_SP, ExpnId};
+use codemap::{mk_sp, respan, Span, Spanned, DUMMY_SP, ExpnId};
 use abi::Abi;
 use errors;
 use ext::base;
 use ext::tt::macro_parser;
-use parse::token::InternedString;
-use parse::token;
+use parse::token::{self, keywords, InternedString};
 use parse::lexer;
 use parse::lexer::comments::{doc_comment_style, strip_doc_comment_decoration};
 use print::pprust;
@@ -1674,7 +1673,25 @@ pub struct Arg {
     pub id: NodeId,
 }
 
+/// Represents the kind of 'self' associated with a method.
+/// String representation of `Ident` here is always "self", but hygiene contexts may differ.
+#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
+pub enum SelfKind {
+    /// No self
+    Static,
+    /// `self`, `mut self`
+    Value(Ident),
+    /// `&'lt self`, `&'lt mut self`
+    Region(Option<Lifetime>, Mutability, Ident),
+    /// `self: TYPE`, `mut self: TYPE`
+    Explicit(P<Ty>, Ident),
+}
+
+pub type ExplicitSelf = Spanned<SelfKind>;
+
 impl Arg {
+    #[unstable(feature = "rustc_private", issue = "27812")]
+    #[rustc_deprecated(since = "1.10.0", reason = "use `from_self` instead")]
     pub fn new_self(span: Span, mutability: Mutability, self_ident: Ident) -> Arg {
         let path = Spanned{span:span,node:self_ident};
         Arg {
@@ -1692,6 +1709,51 @@ impl Arg {
             id: DUMMY_NODE_ID
         }
     }
+
+    pub fn to_self(&self) -> Option<ExplicitSelf> {
+        if let PatKind::Ident(_, ident, _) = self.pat.node {
+            if ident.node.name == keywords::SelfValue.name() {
+                return match self.ty.node {
+                    TyKind::Infer => Some(respan(self.pat.span, SelfKind::Value(ident.node))),
+                    TyKind::Rptr(lt, MutTy{ref ty, mutbl}) if ty.node == TyKind::Infer => {
+                        Some(respan(self.pat.span, SelfKind::Region(lt, mutbl, ident.node)))
+                    }
+                    _ => Some(respan(mk_sp(self.pat.span.lo, self.ty.span.hi),
+                                     SelfKind::Explicit(self.ty.clone(), ident.node))),
+                }
+            }
+        }
+        None
+    }
+
+    pub fn from_self(eself: ExplicitSelf, ident_sp: Span, mutbl: Mutability) -> Arg {
+        let pat = |ident, span| P(Pat {
+            id: DUMMY_NODE_ID,
+            node: PatKind::Ident(BindingMode::ByValue(mutbl), respan(ident_sp, ident), None),
+            span: span,
+        });
+        let infer_ty = P(Ty {
+            id: DUMMY_NODE_ID,
+            node: TyKind::Infer,
+            span: DUMMY_SP,
+        });
+        let arg = |ident, ty, span| Arg {
+            pat: pat(ident, span),
+            ty: ty,
+            id: DUMMY_NODE_ID,
+        };
+        match eself.node {
+            SelfKind::Static => panic!("bug: `Arg::from_self` is called \
+                                        with `SelfKind::Static` argument"),
+            SelfKind::Explicit(ty, ident) => arg(ident, ty, mk_sp(eself.span.lo, ident_sp.hi)),
+            SelfKind::Value(ident) => arg(ident, infer_ty, eself.span),
+            SelfKind::Region(lt, mutbl, ident) => arg(ident, P(Ty {
+                id: DUMMY_NODE_ID,
+                node: TyKind::Rptr(lt, MutTy { ty: infer_ty, mutbl: mutbl }),
+                span: DUMMY_SP,
+            }), eself.span),
+        }
+    }
 }
 
 /// Represents the header (not the body) of a function declaration
@@ -1772,21 +1834,6 @@ impl FunctionRetTy {
     }
 }
 
-/// Represents the kind of 'self' associated with a method
-#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
-pub enum SelfKind {
-    /// No self
-    Static,
-    /// `self`
-    Value(Ident),
-    /// `&'lt self`, `&'lt mut self`
-    Region(Option<Lifetime>, Mutability, Ident),
-    /// `self: TYPE`
-    Explicit(P<Ty>, Ident),
-}
-
-pub type ExplicitSelf = Spanned<SelfKind>;
-
 #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
 pub struct Mod {
     /// A span from the first token past `{` to the last token until `}`.
diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 48368cc025c..fc62cee92fd 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -2036,7 +2036,7 @@ impl<'a> Parser<'a> {
         }
     }
 
-    /// Parse mutability declaration (mut/const/imm)
+    /// Parse mutability (`mut` or nothing).
     pub fn parse_mutability(&mut self) -> PResult<'a, Mutability> {
         if self.eat_keyword(keywords::Mut) {
             Ok(Mutability::Mutable)
@@ -4616,184 +4616,142 @@ impl<'a> Parser<'a> {
         }))
     }
 
-    fn expect_self_ident(&mut self) -> PResult<'a, ast::Ident> {
-        match self.token {
-            token::Ident(id) if id.name == keywords::SelfValue.name() => {
-                self.bump();
-                // The hygiene context of `id` needs to be preserved here,
-                // so we can't just return `SelfValue.ident()`.
-                Ok(id)
-            },
-            _ => {
-                let token_str = self.this_token_to_string();
-                return Err(self.fatal(&format!("expected `self`, found `{}`",
-                                   token_str)))
-            }
-        }
-    }
-
-    /// Parse the argument list and result type of a function
-    /// that may have a self type.
+    /// Parse the parameter list and result type of a function that may have a `self` parameter.
     fn parse_fn_decl_with_self<F>(&mut self,
-                                  parse_arg_fn: F) -> PResult<'a, (ExplicitSelf, P<FnDecl>)> where
-        F: FnMut(&mut Parser<'a>) -> PResult<'a,  Arg>,
+                                  parse_arg_fn: F)
+                                  -> PResult<'a, (ExplicitSelf, P<FnDecl>)>
+        where F: FnMut(&mut Parser<'a>) -> PResult<'a,  Arg>,
     {
-        fn maybe_parse_borrowed_explicit_self<'b>(this: &mut Parser<'b>)
-                                                  -> PResult<'b, ast::SelfKind> {
-            // The following things are possible to see here:
-            //
-            //     fn(&mut self)
-            //     fn(&mut self)
-            //     fn(&'lt self)
-            //     fn(&'lt mut self)
-            //
-            // We already know that the current token is `&`.
-
-            if this.look_ahead(1, |t| t.is_keyword(keywords::SelfValue)) {
-                this.bump();
-                Ok(SelfKind::Region(None, Mutability::Immutable, this.expect_self_ident()?))
-            } else if this.look_ahead(1, |t| t.is_mutability()) &&
-                      this.look_ahead(2, |t| t.is_keyword(keywords::SelfValue)) {
-                this.bump();
-                let mutability = this.parse_mutability()?;
-                Ok(SelfKind::Region(None, mutability, this.expect_self_ident()?))
-            } else if this.look_ahead(1, |t| t.is_lifetime()) &&
-                      this.look_ahead(2, |t| t.is_keyword(keywords::SelfValue)) {
-                this.bump();
-                let lifetime = this.parse_lifetime()?;
-                let ident = this.expect_self_ident()?;
-                Ok(SelfKind::Region(Some(lifetime), Mutability::Immutable, ident))
-            } else if this.look_ahead(1, |t| t.is_lifetime()) &&
-                      this.look_ahead(2, |t| t.is_mutability()) &&
-                      this.look_ahead(3, |t| t.is_keyword(keywords::SelfValue)) {
-                this.bump();
-                let lifetime = this.parse_lifetime()?;
-                let mutability = this.parse_mutability()?;
-                Ok(SelfKind::Region(Some(lifetime), mutability, this.expect_self_ident()?))
-            } else {
-                Ok(SelfKind::Static)
-            }
-        }
+        let expect_ident = |this: &mut Self| match this.token {
+            token::Ident(ident) => { this.bump(); ident } // Preserve hygienic context.
+            _ => unreachable!()
+        };
 
         self.expect(&token::OpenDelim(token::Paren))?;
 
-        // A bit of complexity and lookahead is needed here in order to be
-        // backwards compatible.
-        let lo = self.span.lo;
-        let mut self_ident_lo = self.span.lo;
-        let mut self_ident_hi = self.span.hi;
-
-        let mut mutbl_self = Mutability::Immutable;
-        let explicit_self = match self.token {
+        // Parse optional self parameter of a method.
+        // 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.lo;
+        let mut eself_mutbl = Mutability::Immutable;
+        let (eself, eself_ident_sp) = match self.token {
             token::BinOp(token::And) => {
-                let eself = maybe_parse_borrowed_explicit_self(self)?;
-                self_ident_lo = self.last_span.lo;
-                self_ident_hi = self.last_span.hi;
-                eself
+                // &self
+                // &mut self
+                // &'lt self
+                // &'lt mut self
+                // &not_self
+                if self.look_ahead(1, |t| t.is_keyword(keywords::SelfValue)) {
+                    self.bump();
+                    (SelfKind::Region(None, Mutability::Immutable, expect_ident(self)),
+                        self.last_span)
+                } else if self.look_ahead(1, |t| t.is_keyword(keywords::Mut)) &&
+                          self.look_ahead(2, |t| t.is_keyword(keywords::SelfValue)) {
+                    self.bump();
+                    self.bump();
+                    (SelfKind::Region(None, Mutability::Mutable, expect_ident(self)),
+                        self.last_span)
+                } else if self.look_ahead(1, |t| t.is_lifetime()) &&
+                          self.look_ahead(2, |t| t.is_keyword(keywords::SelfValue)) {
+                    self.bump();
+                    let lt = self.parse_lifetime()?;
+                    (SelfKind::Region(Some(lt), Mutability::Immutable, expect_ident(self)),
+                        self.last_span)
+                } else if self.look_ahead(1, |t| t.is_lifetime()) &&
+                          self.look_ahead(2, |t| t.is_keyword(keywords::Mut)) &&
+                          self.look_ahead(3, |t| t.is_keyword(keywords::SelfValue)) {
+                    self.bump();
+                    let lt = self.parse_lifetime()?;
+                    self.bump();
+                    (SelfKind::Region(Some(lt), Mutability::Mutable, expect_ident(self)),
+                        self.last_span)
+                } else {
+                    (SelfKind::Static, codemap::DUMMY_SP)
+                }
             }
             token::BinOp(token::Star) => {
-                // Possibly "*self" or "*mut self" -- not supported. Try to avoid
-                // emitting cryptic "unexpected token" errors.
-                self.bump();
-                let _mutability = if self.token.is_mutability() {
-                    self.parse_mutability()?
-                } else {
-                    Mutability::Immutable
-                };
-                if self.token.is_keyword(keywords::SelfValue) {
-                    let span = self.span;
-                    self.span_err(span, "cannot pass self by raw pointer");
+                // *self
+                // *const self
+                // *mut self
+                // *not_self
+                // Emit special error for `self` cases.
+                if self.look_ahead(1, |t| t.is_keyword(keywords::SelfValue)) {
                     self.bump();
+                    self.span_err(self.span, "cannot pass `self` by raw pointer");
+                    (SelfKind::Value(expect_ident(self)), self.last_span)
+                } else if self.look_ahead(1, |t| t.is_mutability()) &&
+                          self.look_ahead(2, |t| t.is_keyword(keywords::SelfValue)) {
+                    self.bump();
+                    self.bump();
+                    self.span_err(self.span, "cannot pass `self` by raw pointer");
+                    (SelfKind::Value(expect_ident(self)), self.last_span)
+                } else {
+                    (SelfKind::Static, codemap::DUMMY_SP)
                 }
-                // error case, making bogus self ident:
-                SelfKind::Value(keywords::SelfValue.ident())
             }
             token::Ident(..) => {
                 if self.token.is_keyword(keywords::SelfValue) {
-                    let self_ident = self.expect_self_ident()?;
-
-                    // Determine whether this is the fully explicit form, `self:
-                    // TYPE`.
+                    // self
+                    // self: TYPE
+                    let eself_ident = expect_ident(self);
+                    let eself_ident_sp = self.last_span;
                     if self.eat(&token::Colon) {
-                        SelfKind::Explicit(self.parse_ty_sum()?, self_ident)
+                        (SelfKind::Explicit(self.parse_ty_sum()?, eself_ident), eself_ident_sp)
                     } else {
-                        SelfKind::Value(self_ident)
+                        (SelfKind::Value(eself_ident), eself_ident_sp)
                     }
-                } else if self.token.is_mutability() &&
+                } else if self.token.is_keyword(keywords::Mut) &&
                         self.look_ahead(1, |t| t.is_keyword(keywords::SelfValue)) {
-                    mutbl_self = self.parse_mutability()?;
-                    let self_ident = self.expect_self_ident()?;
-
-                    // Determine whether this is the fully explicit form,
-                    // `self: TYPE`.
+                    // mut self
+                    // mut self: TYPE
+                    eself_mutbl = Mutability::Mutable;
+                    self.bump();
+                    let eself_ident = expect_ident(self);
+                    let eself_ident_sp = self.last_span;
                     if self.eat(&token::Colon) {
-                        SelfKind::Explicit(self.parse_ty_sum()?, self_ident)
+                        (SelfKind::Explicit(self.parse_ty_sum()?, eself_ident), eself_ident_sp)
                     } else {
-                        SelfKind::Value(self_ident)
+                        (SelfKind::Value(eself_ident), eself_ident_sp)
                     }
                 } else {
-                    SelfKind::Static
+                    (SelfKind::Static, codemap::DUMMY_SP)
                 }
             }
-            _ => SelfKind::Static,
+            _ => (SelfKind::Static, codemap::DUMMY_SP)
         };
+        let mut eself = codemap::respan(mk_sp(eself_lo, self.last_span.hi), eself);
 
-        let explicit_self_sp = mk_sp(self_ident_lo, self_ident_hi);
-
-        // shared fall-through for the three cases below. borrowing prevents simply
-        // writing this as a closure
-        macro_rules! parse_remaining_arguments {
-            ($self_id:ident) =>
-            {
-            // If we parsed a self type, expect a comma before the argument list.
-            match self.token {
-                token::Comma => {
+        // Parse the rest of the function parameter list.
+        let sep = SeqSep::trailing_allowed(token::Comma);
+        let fn_inputs = match eself.node {
+            SelfKind::Static => {
+                eself.span = codemap::DUMMY_SP;
+                self.parse_seq_to_before_end(&token::CloseDelim(token::Paren), sep, parse_arg_fn)
+            }
+            SelfKind::Value(..) | SelfKind::Region(..) | SelfKind::Explicit(..) => {
+                if self.check(&token::CloseDelim(token::Paren)) {
+                    vec![Arg::from_self(eself.clone(), eself_ident_sp, eself_mutbl)]
+                } else if self.check(&token::Comma) {
                     self.bump();
-                    let sep = SeqSep::trailing_allowed(token::Comma);
-                    let mut fn_inputs = self.parse_seq_to_before_end(
-                        &token::CloseDelim(token::Paren),
-                        sep,
-                        parse_arg_fn
+                    let mut fn_inputs = vec![Arg::from_self(eself.clone(), eself_ident_sp,
+                                                            eself_mutbl)];
+                    fn_inputs.append(&mut self.parse_seq_to_before_end(
+                        &token::CloseDelim(token::Paren), sep, parse_arg_fn)
                     );
-                    fn_inputs.insert(0, Arg::new_self(explicit_self_sp, mutbl_self, $self_id));
                     fn_inputs
-                }
-                token::CloseDelim(token::Paren) => {
-                    vec!(Arg::new_self(explicit_self_sp, mutbl_self, $self_id))
-                }
-                _ => {
-                    let token_str = self.this_token_to_string();
-                    return Err(self.fatal(&format!("expected `,` or `)`, found `{}`",
-                                       token_str)))
+                } else {
+                    return self.unexpected();
                 }
             }
-            }
-        }
-
-        let fn_inputs = match explicit_self {
-            SelfKind::Static =>  {
-                let sep = SeqSep::trailing_allowed(token::Comma);
-                self.parse_seq_to_before_end(&token::CloseDelim(token::Paren), sep, parse_arg_fn)
-            }
-            SelfKind::Value(id) => parse_remaining_arguments!(id),
-            SelfKind::Region(_,_,id) => parse_remaining_arguments!(id),
-            SelfKind::Explicit(_,id) => parse_remaining_arguments!(id),
         };
 
-
+        // Parse closing paren and return type.
         self.expect(&token::CloseDelim(token::Paren))?;
-
-        let hi = self.span.hi;
-
-        let ret_ty = self.parse_ret_ty()?;
-
-        let fn_decl = P(FnDecl {
+        Ok((eself, P(FnDecl {
             inputs: fn_inputs,
-            output: ret_ty,
+            output: self.parse_ret_ty()?,
             variadic: false
-        });
-
-        Ok((spanned(lo, hi, explicit_self), fn_decl))
+        })))
     }
 
     // parse the |arg, arg| header on a lambda
diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs
index 7bfc58e85c2..ebb4927d69c 100644
--- a/src/libsyntax/print/pprust.rs
+++ b/src/libsyntax/print/pprust.rs
@@ -1522,7 +1522,7 @@ impl<'a> State<'a> {
                       m.abi,
                       Some(ident),
                       &m.generics,
-                      Some(&m.explicit_self.node),
+                      None,
                       vis)
     }
 
@@ -2656,36 +2656,9 @@ impl<'a> State<'a> {
     }
 
     pub fn print_fn_args(&mut self, decl: &ast::FnDecl,
-                         opt_explicit_self: Option<&ast::SelfKind>,
+                         _: Option<&ast::SelfKind>,
                          is_closure: bool) -> io::Result<()> {
-        // It is unfortunate to duplicate the commasep logic, but we want the
-        // self type and the args all in the same box.
-        self.rbox(0, Inconsistent)?;
-        let mut first = true;
-        if let Some(explicit_self) = opt_explicit_self {
-            let m = match *explicit_self {
-                ast::SelfKind::Static => ast::Mutability::Immutable,
-                _ => match decl.inputs[0].pat.node {
-                    PatKind::Ident(ast::BindingMode::ByValue(m), _, _) => m,
-                    _ => ast::Mutability::Immutable
-                }
-            };
-            first = !self.print_explicit_self(explicit_self, m)?;
-        }
-
-        // HACK(eddyb) ignore the separately printed self argument.
-        let args = if first {
-            &decl.inputs[..]
-        } else {
-            &decl.inputs[1..]
-        };
-
-        for arg in args {
-            if first { first = false; } else { self.word_space(",")?; }
-            self.print_arg(arg, is_closure)?;
-        }
-
-        self.end()
+        self.commasep(Inconsistent, &decl.inputs, |s, arg| s.print_arg(arg, is_closure))
     }
 
     pub fn print_fn_args_and_ret(&mut self, decl: &ast::FnDecl,
@@ -2956,18 +2929,24 @@ impl<'a> State<'a> {
         match input.ty.node {
             ast::TyKind::Infer if is_closure => self.print_pat(&input.pat)?,
             _ => {
-                match input.pat.node {
-                    PatKind::Ident(_, ref path1, _)
-                            if path1.node.name == keywords::Invalid.name() => {
-                        // Do nothing.
+                let (mutbl, invalid) = match input.pat.node {
+                    PatKind::Ident(ast::BindingMode::ByValue(mutbl), ident, _) |
+                    PatKind::Ident(ast::BindingMode::ByRef(mutbl), ident, _) => {
+                        (mutbl, ident.node.name == keywords::Invalid.name())
                     }
-                    _ => {
+                    _ => (ast::Mutability::Immutable, false)
+                };
+
+                if let Some(eself) = input.to_self() {
+                    self.print_explicit_self(&eself.node, mutbl)?;
+                } else {
+                    if !invalid {
                         self.print_pat(&input.pat)?;
                         word(&mut self.s, ":")?;
                         space(&mut self.s)?;
                     }
+                    self.print_type(&input.ty)?;
                 }
-                self.print_type(&input.ty)?;
             }
         }
         self.end()