parser: address review comments re. self.

rust-lang · Feb 2, 2020 · 71a6f58 · 71a6f58
1 parent 8674efd
commit 71a6f58
Show file tree

Hide file tree

Showing 19 changed files with 124 additions and 128 deletions.
diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs
@@ -372,10 +372,10 @@ impl<'a> AstValidator<'a> {
                 self.err_handler()
                     .struct_span_err(
                         param.span,
-                        "`self` parameter only allowed in associated `fn`s",
+                        "`self` parameter is only allowed in associated functions",
                     )
                     .span_label(param.span, "not semantically valid as function parameter")
-                    .note("associated `fn`s are those in `impl` or `trait` definitions")
+                    .note("associated functions are those in `impl` or `trait` definitions")
                     .emit();
             }
         }

diff --git a/src/librustc_parse/parser/diagnostics.rs b/src/librustc_parse/parser/diagnostics.rs
@@ -1336,8 +1336,7 @@ impl<'a> Parser<'a> {
         err: &mut DiagnosticBuilder<'_>,
         pat: P<ast::Pat>,
         require_name: bool,
-        is_self_semantic: bool,
-        in_assoc_item: bool,
+        first_param: bool,
     ) -> Option<Ident> {
         // If we find a pattern followed by an identifier, it could be an (incorrect)
         // C-style parameter declaration.
@@ -1357,13 +1356,12 @@ impl<'a> Parser<'a> {
             return Some(ident);
         } else if let PatKind::Ident(_, ident, _) = pat.kind {
             if require_name
-                && (in_assoc_item
-                    || self.token == token::Comma
+                && (self.token == token::Comma
                     || self.token == token::Lt
                     || self.token == token::CloseDelim(token::Paren))
             {
                 // `fn foo(a, b) {}`, `fn foo(a<x>, b<y>) {}` or `fn foo(usize, usize) {}`
-                if is_self_semantic {
+                if first_param {
                     err.span_suggestion(
                         pat.span,
                         "if this is a `self` type, give it a parameter name",
@@ -1420,21 +1418,12 @@ impl<'a> Parser<'a> {
         Ok((pat, ty))
     }
 
-    pub(super) fn recover_bad_self_param(
-        &mut self,
-        mut param: ast::Param,
-        in_assoc_item: bool,
-    ) -> PResult<'a, ast::Param> {
+    pub(super) fn recover_bad_self_param(&mut self, mut param: Param) -> PResult<'a, Param> {
         let sp = param.pat.span;
         param.ty.kind = TyKind::Err;
-        let mut err = self.struct_span_err(sp, "unexpected `self` parameter in function");
-        if in_assoc_item {
-            err.span_label(sp, "must be the first associated function parameter");
-        } else {
-            err.span_label(sp, "not valid as function parameter");
-            err.note("`self` is only valid as the first parameter of an associated function");
-        }
-        err.emit();
+        self.struct_span_err(sp, "unexpected `self` parameter in function")
+            .span_label(sp, "must be the first parameter of an associated function")
+            .emit();
         Ok(param)
     }
 

diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs
@@ -1715,9 +1715,6 @@ impl<'a> Parser<'a> {
 
 /// The parsing configuration used to parse a parameter list (see `parse_fn_params`).
 pub(super) struct ParamCfg {
-    /// Is `self` is *semantically* allowed as the first parameter?
-    /// This is only used for diagnostics.
-    pub in_assoc_item: bool,
     /// `is_name_required` decides if, per-parameter,
     /// the parameter must have a pattern or just a type.
     pub is_name_required: fn(&token::Token) -> bool,
@@ -1733,7 +1730,7 @@ impl<'a> Parser<'a> {
         attrs: Vec<Attribute>,
         header: FnHeader,
     ) -> PResult<'a, Option<P<Item>>> {
-        let cfg = ParamCfg { in_assoc_item: false, is_name_required: |_| true };
+        let cfg = ParamCfg { is_name_required: |_| true };
         let (ident, decl, generics) = self.parse_fn_sig(&cfg)?;
         let (inner_attrs, body) = self.parse_inner_attrs_and_block()?;
         let kind = ItemKind::Fn(FnSig { decl, header }, generics, body);
@@ -1748,7 +1745,7 @@ impl<'a> Parser<'a> {
         attrs: Vec<Attribute>,
         extern_sp: Span,
     ) -> PResult<'a, P<ForeignItem>> {
-        let cfg = ParamCfg { in_assoc_item: false, is_name_required: |_| true };
+        let cfg = ParamCfg { is_name_required: |_| true };
         self.expect_keyword(kw::Fn)?;
         let (ident, decl, generics) = self.parse_fn_sig(&cfg)?;
         let span = lo.to(self.token.span);
@@ -1763,9 +1760,8 @@ impl<'a> Parser<'a> {
         attrs: &mut Vec<Attribute>,
         is_name_required: fn(&token::Token) -> bool,
     ) -> PResult<'a, (Ident, AssocItemKind, Generics)> {
-        let cfg = ParamCfg { in_assoc_item: true, is_name_required };
         let header = self.parse_fn_front_matter()?;
-        let (ident, decl, generics) = self.parse_fn_sig(&cfg)?;
+        let (ident, decl, generics) = self.parse_fn_sig(&ParamCfg { is_name_required })?;
         let sig = FnSig { header, decl };
         let body = self.parse_assoc_fn_body(at_end, attrs)?;
         Ok((ident, AssocItemKind::Fn(sig, body), generics))
@@ -1893,11 +1889,7 @@ impl<'a> Parser<'a> {
         // Possibly parse `self`. Recover if we parsed it and it wasn't allowed here.
         if let Some(mut param) = self.parse_self_param()? {
             param.attrs = attrs.into();
-            return if first_param {
-                Ok(param)
-            } else {
-                self.recover_bad_self_param(param, cfg.in_assoc_item)
-            };
+            return if first_param { Ok(param) } else { self.recover_bad_self_param(param) };
         }
 
         let is_name_required = match self.token.kind {
@@ -1909,13 +1901,9 @@ impl<'a> Parser<'a> {
 
             let pat = self.parse_fn_param_pat()?;
             if let Err(mut err) = self.expect(&token::Colon) {
-                return if let Some(ident) = self.parameter_without_type(
-                    &mut err,
-                    pat,
-                    is_name_required,
-                    first_param && cfg.in_assoc_item,
-                    cfg.in_assoc_item,
-                ) {
+                return if let Some(ident) =
+                    self.parameter_without_type(&mut err, pat, is_name_required, first_param)
+                {
                     err.emit();
                     Ok(dummy_arg(ident))
                 } else {

diff --git a/src/librustc_parse/parser/ty.rs b/src/librustc_parse/parser/ty.rs
@@ -288,8 +288,7 @@ impl<'a> Parser<'a> {
         let unsafety = self.parse_unsafety();
         let ext = self.parse_extern()?;
         self.expect_keyword(kw::Fn)?;
-        let cfg = ParamCfg { in_assoc_item: false, is_name_required: |_| false };
-        let decl = self.parse_fn_decl(&cfg, false)?;
+        let decl = self.parse_fn_decl(&ParamCfg { is_name_required: |_| false }, false)?;
         Ok(TyKind::BareFn(P(BareFnTy { ext, unsafety, generic_params, decl })))
     }
 

diff --git a/src/test/ui/invalid-self-argument/bare-fn-start.rs b/src/test/ui/invalid-self-argument/bare-fn-start.rs
@@ -1,6 +1,6 @@
 fn a(&self) { }
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 //~| NOTE not semantically valid as function parameter
-//~| NOTE associated `fn`s are those in `impl` or `trait` definitions
+//~| NOTE associated functions are those in `impl` or `trait` definitions
 
 fn main() { }
diff --git a/src/test/ui/invalid-self-argument/bare-fn-start.stderr b/src/test/ui/invalid-self-argument/bare-fn-start.stderr
@@ -1,10 +1,10 @@
-error: `self` parameter only allowed in associated `fn`s
+error: `self` parameter is only allowed in associated functions
   --> $DIR/bare-fn-start.rs:1:6
    |
 LL | fn a(&self) { }
    |      ^^^^^ not semantically valid as function parameter
    |
-   = note: associated `fn`s are those in `impl` or `trait` definitions
+   = note: associated functions are those in `impl` or `trait` definitions
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/invalid-self-argument/bare-fn.rs b/src/test/ui/invalid-self-argument/bare-fn.rs
@@ -1,6 +1,5 @@
 fn b(foo: u32, &mut self) { }
 //~^ ERROR unexpected `self` parameter in function
-//~| NOTE not valid as function parameter
-//~| NOTE `self` is only valid as the first parameter of an associated function
+//~| NOTE must be the first parameter of an associated function
 
 fn main() { }
diff --git a/src/test/ui/invalid-self-argument/bare-fn.stderr b/src/test/ui/invalid-self-argument/bare-fn.stderr
@@ -2,9 +2,7 @@ error: unexpected `self` parameter in function
   --> $DIR/bare-fn.rs:1:16
    |
 LL | fn b(foo: u32, &mut self) { }
-   |                ^^^^^^^^^ not valid as function parameter
-   |
-   = note: `self` is only valid as the first parameter of an associated function
+   |                ^^^^^^^^^ must be the first parameter of an associated function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/invalid-self-argument/trait-fn.rs b/src/test/ui/invalid-self-argument/trait-fn.rs
@@ -3,7 +3,7 @@ struct Foo {}
 impl Foo {
     fn c(foo: u32, self) {}
     //~^ ERROR unexpected `self` parameter in function
-    //~| NOTE must be the first associated function parameter
+    //~| NOTE must be the first parameter of an associated function
 
     fn good(&mut self, foo: u32) {}
 }

diff --git a/src/test/ui/invalid-self-argument/trait-fn.stderr b/src/test/ui/invalid-self-argument/trait-fn.stderr
@@ -2,7 +2,7 @@ error: unexpected `self` parameter in function
   --> $DIR/trait-fn.rs:4:20
    |
 LL |     fn c(foo: u32, self) {}
-   |                    ^^^^ must be the first associated function parameter
+   |                    ^^^^ must be the first parameter of an associated function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/inverted-parameters.rs b/src/test/ui/parser/inverted-parameters.rs
@@ -21,6 +21,7 @@ fn pattern((i32, i32) (a, b)) {}
 fn fizz(i32) {}
 //~^ ERROR expected one of `:`, `@`
 //~| HELP if this was a parameter name, give it a type
+//~| HELP if this is a `self` type, give it a parameter name
 //~| HELP if this is a type, explicitly ignore the parameter name
 
 fn missing_colon(quux S) {}

diff --git a/src/test/ui/parser/inverted-parameters.stderr b/src/test/ui/parser/inverted-parameters.stderr
@@ -35,6 +35,10 @@ LL | fn fizz(i32) {}
    |            ^ expected one of `:`, `@`, or `|`
    |
    = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: if this is a `self` type, give it a parameter name
+   |
+LL | fn fizz(self: i32) {}
+   |         ^^^^^^^^^
 help: if this was a parameter name, give it a type
    |
 LL | fn fizz(i32: TypeName) {}
@@ -45,7 +49,7 @@ LL | fn fizz(_: i32) {}
    |         ^^^^^^
 
 error: expected one of `:`, `@`, or `|`, found `S`
-  --> $DIR/inverted-parameters.rs:26:23
+  --> $DIR/inverted-parameters.rs:27:23
    |
 LL | fn missing_colon(quux S) {}
    |                  -----^

diff --git a/src/test/ui/parser/omitted-arg-in-item-fn.stderr b/src/test/ui/parser/omitted-arg-in-item-fn.stderr
@@ -5,6 +5,10 @@ LL | fn foo(x) {
    |         ^ expected one of `:`, `@`, or `|`
    |
    = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: if this is a `self` type, give it a parameter name
+   |
+LL | fn foo(self: x) {
+   |        ^^^^^^^
 help: if this was a parameter name, give it a type
    |
 LL | fn foo(x: TypeName) {

diff --git a/src/test/ui/parser/pat-lt-bracket-2.stderr b/src/test/ui/parser/pat-lt-bracket-2.stderr
@@ -5,6 +5,10 @@ LL | fn a(B<) {}
    |       ^ expected one of `:`, `@`, or `|`
    |
    = note: anonymous parameters are removed in the 2018 edition (see RFC 1685)
+help: if this is a `self` type, give it a parameter name
+   |
+LL | fn a(self: B<) {}
+   |      ^^^^^^^
 help: if this is a type, explicitly ignore the parameter name
    |
 LL | fn a(_: B<) {}

diff --git a/src/test/ui/parser/self-in-function-arg.stderr b/src/test/ui/parser/self-in-function-arg.stderr
@@ -2,9 +2,7 @@ error: unexpected `self` parameter in function
   --> $DIR/self-in-function-arg.rs:1:15
    |
 LL | fn foo(x:i32, self: i32) -> i32 { self }
-   |               ^^^^ not valid as function parameter
-   |
-   = note: `self` is only valid as the first parameter of an associated function
+   |               ^^^^ must be the first parameter of an associated function
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/parser/self-param-semantic-fail.rs b/src/test/ui/parser/self-param-semantic-fail.rs
@@ -6,59 +6,59 @@ fn main() {}
 
 fn free() {
     fn f1(self) {}
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f2(mut self) {}
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f3(&self) {}
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f4(&mut self) {}
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f5<'a>(&'a self) {}
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f6<'a>(&'a mut self) {}
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f7(self: u8) {}
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f8(mut self: u8) {}
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
 }
 
 extern {
     fn f1(self);
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f2(mut self);
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     //~| ERROR patterns aren't allowed in
     fn f3(&self);
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f4(&mut self);
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f5<'a>(&'a self);
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f6<'a>(&'a mut self);
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f7(self: u8);
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     fn f8(mut self: u8);
-    //~^ ERROR `self` parameter only allowed in associated `fn`s
+    //~^ ERROR `self` parameter is only allowed in associated functions
     //~| ERROR patterns aren't allowed in
 }
 
 type X1 = fn(self);
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 type X2 = fn(mut self);
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 //~| ERROR patterns aren't allowed in
 type X3 = fn(&self);
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 type X4 = fn(&mut self);
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 type X5 = for<'a> fn(&'a self);
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 type X6 = for<'a> fn(&'a mut self);
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 type X7 = fn(self: u8);
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 type X8 = fn(mut self: u8);
-//~^ ERROR `self` parameter only allowed in associated `fn`s
+//~^ ERROR `self` parameter is only allowed in associated functions
 //~| ERROR patterns aren't allowed in