Rollup merge of rust-lang#68728 - Centril:towards-fn-merge, r=petrochenkov

parse: merge `fn` syntax + cleanup item parsing

Here we continue the work in rust-lang#67131 in particular to merge the grammars of `fn` items in various positions.

A list of *language level* changes (as sanctioned by the language team in rust-lang#65041 (comment) and rust-lang#67131):

- `self` parameters are now *syntactically* allowed as the first parameter irrespective of item context (and in function pointers). Instead, semantic validation (`ast_validation`) is used.

- Syntactically, `fn` items in `extern { ... }` blocks can now have bodies (`fn foo() { ... }` as opposed to `fn foo();`). As above, we use semantic restrictions instead.

- Syntactically, `fn` items in free contexts (directly in a file or a module) can now be without bodies (`fn foo();` as opposed to `fn foo() { ... }`. As above, we use semantic restrictions instead, including for non-ident parameter patterns.

- `const extern fn` feature gating is now done post-expansion such that we do not have conditional compatibilities of function qualifiers *in parsing*.

- The `FnFrontMatter` grammar becomes:
   ```rust
   Extern = "extern" StringLit ;
   FnQual = "const"? "async"? "unsafe"? Extern? ;
   FnFrontMatter = FnQual "fn" ;
   ```

   That is, all item contexts now *syntactically* allow `const async unsafe extern "C" fn` and use semantic restrictions to rule out combinations previously prevented syntactically. The semantic restrictions include in particular:

   - `fn`s in `extern { ... }` can have no qualifiers.
   - `const` and `async` cannot be combined.

- To fuse the list-of-items parsing in the 4 contexts that items are allowed, we now must permit inner attributes (`#![attr]`) inside `trait Foo { ... }` definitions. That is, we now allow e.g. `trait Foo { #![attr] }`. This was probably an oversight due to not using a uniform parsing mechanism, which we now do have (`fn parse_item_list`). The semantic support (including e.g. for linting) falls out directly from the attributes infrastructure. To ensure this, we include a test for lints.

Put together, these grammar changes allow us to substantially reduce the complexity of item parsing and its grammar. There are however some other non-language improvements that allow the compression to take place.

A list of *compiler-internal* changes (in particular noting the parser-external data-structure changes):

- We use `enum AllowPlus/RecoverQPath/AllowCVariadic { Yes, No }` in `parser/ty.rs` instead of passing around 3 different `bool`s. I felt this was necessary as it was becoming mentally taxing to track which-is-which.

- `fn visit_trait_item` and `fn visit_impl_item` are merged into `fn visit_assoc_item` which now is passed an `AssocCtxt` to check which one it is.

- We change `FnKind` to:

  ```rust
  pub enum FnKind<'a> {
      Fn(FnCtxt, Ident, &'a FnSig, &'a Visibility, Option<&'a Block>),
      Closure(&'a FnDecl, &'a Expr),
  }
  ```

  with:

  ```rust
  pub enum FnCtxt {
      Free,
      Foreign,
      Assoc(AssocCtxt),
  }
  ```

  This is then taken advantage of in tweaking the various semantic restrictions as well as in pretty printing.

- In `ItemKind::Fn`, we change `P<Block>` to `Option<P<Block>>`.

- In `ForeignItemKind::Fn`, we change `P<FnDecl>` to `FnSig` and `P<Block>` to `Option<P<Block>>`.

- We change `ast::{Unsafety, Spanned<Constness>}>` into `enum ast::{Unsafe, Const} { Yes(Span), No }` respectively. This change in formulation allow us to exclude `Span` in the case of `No`, which facilitates parsing. Moreover, we also add a `Span` to `IsAsync` which is renamed to `Async`. The new `Span`s in `Unsafety` and `Async` are then taken advantage of for better diagnostics. A reason this change was made is to have a more uniform and clear naming scheme.

  The HIR keeps the structures in AST (with those definitions moved into HIR) for now to avoid regressing perf.

- Various cleanups, bug fixes, and diagnostics improvements are made along the way. It is probably best to understand those via the diffs.

I would recommend reviewing this commit-by-commit with whitespace changes hidden.

r? @estebank @petrochenkov

Author: Dylan-DPC

src/librustc_ast_pretty/pprust.rs

@@ -1269,6 +1269,7 @@ impl<'a> State<'a> {
                 self.print_where_clause(&generics.where_clause);
                 self.s.word(" ");
                 self.bopen();
+                self.print_inner_attributes(&item.attrs);
                 for trait_item in trait_items {
                     self.print_assoc_item(trait_item);
                 }

src/librustc_expand/expand.rs

@@ -867,7 +867,7 @@ pub fn parse_ast_fragment<'a>(
         AstFragmentKind::ForeignItems => {
             let mut items = SmallVec::new();
             while this.token != token::Eof {
-                items.push(this.parse_foreign_item()?);
+                items.push(this.parse_foreign_item(&mut false)?);
             }
             AstFragment::ForeignItems(items)
         }

src/librustc_parse/config.rs

@@ -562,14 +562,9 @@ fn is_cfg(attr: &Attribute) -> bool {
 
 /// Process the potential `cfg` attributes on a module.
 /// Also determine if the module should be included in this configuration.
-pub fn process_configure_mod(
-    sess: &ParseSess,
-    cfg_mods: bool,
-    attrs: &[Attribute],
-) -> (bool, Vec<Attribute>) {
+pub fn process_configure_mod(sess: &ParseSess, cfg_mods: bool, attrs: &mut Vec<Attribute>) -> bool {
     // Don't perform gated feature checking.
     let mut strip_unconfigured = StripUnconfigured { sess, features: None };
-    let mut attrs = attrs.to_owned();
-    strip_unconfigured.process_cfg_attrs(&mut attrs);
-    (!cfg_mods || strip_unconfigured.in_cfg(&attrs), attrs)
+    strip_unconfigured.process_cfg_attrs(attrs);
+    !cfg_mods || strip_unconfigured.in_cfg(&attrs)
 }

src/librustc_parse/parser/item.rs

@@ -572,6 +572,11 @@ impl<'a> Parser<'a> {
         if !self.eat_keyword(kw) { self.unexpected() } else { Ok(()) }
     }
 
+    /// Is the given keyword `kw` followed by a non-reserved identifier?
+    fn is_kw_followed_by_ident(&self, kw: Symbol) -> bool {
+        self.token.is_keyword(kw) && self.look_ahead(1, |t| t.is_ident() && !t.is_reserved_ident())
+    }
+
     fn check_or_expected(&mut self, ok: bool, typ: TokenType) -> bool {
         if ok {
             true

src/librustc_parse/parser/mod.rs

@@ -40,36 +40,34 @@ impl<'a> Parser<'a> {
     }
 
     /// Parses a `mod <foo> { ... }` or `mod <foo>;` item.
-    pub(super) fn parse_item_mod(&mut self, outer_attrs: &[Attribute]) -> PResult<'a, ItemInfo> {
-        let (in_cfg, outer_attrs) =
-            crate::config::process_configure_mod(self.sess, self.cfg_mods, outer_attrs);
+    pub(super) fn parse_item_mod(&mut self, attrs: &mut Vec<Attribute>) -> PResult<'a, ItemInfo> {
+        let in_cfg = crate::config::process_configure_mod(self.sess, self.cfg_mods, attrs);
 
         let id_span = self.token.span;
         let id = self.parse_ident()?;
-        if self.eat(&token::Semi) {
+        let (module, mut inner_attrs) = if self.eat(&token::Semi) {
             if in_cfg && self.recurse_into_file_modules {
                 // This mod is in an external file. Let's go get it!
                 let ModulePathSuccess { path, directory_ownership } =
-                    self.submod_path(id, &outer_attrs, id_span)?;
-                let (module, attrs) =
-                    self.eval_src_mod(path, directory_ownership, id.to_string(), id_span)?;
-                Ok((id, ItemKind::Mod(module), Some(attrs)))
+                    self.submod_path(id, &attrs, id_span)?;
+                self.eval_src_mod(path, directory_ownership, id.to_string(), id_span)?
             } else {
-                let placeholder = ast::Mod { inner: DUMMY_SP, items: Vec::new(), inline: false };
-                Ok((id, ItemKind::Mod(placeholder), None))
+                (ast::Mod { inner: DUMMY_SP, items: Vec::new(), inline: false }, Vec::new())
             }
         } else {
             let old_directory = self.directory.clone();
-            self.push_directory(id, &outer_attrs);
+            self.push_directory(id, &attrs);
 
             self.expect(&token::OpenDelim(token::Brace))?;
             let mod_inner_lo = self.token.span;
-            let attrs = self.parse_inner_attributes()?;
+            let inner_attrs = self.parse_inner_attributes()?;
             let module = self.parse_mod_items(&token::CloseDelim(token::Brace), mod_inner_lo)?;
 
             self.directory = old_directory;
-            Ok((id, ItemKind::Mod(module), Some(attrs)))
-        }
+            (module, inner_attrs)
+        };
+        attrs.append(&mut inner_attrs);
+        Ok((id, ItemKind::Mod(module)))
     }
 
     /// Given a termination token, parses all of the items in a module.

src/librustc_parse/parser/module.rs

@@ -7,10 +7,10 @@ use crate::maybe_whole;
 use crate::DirectoryOwnership;
 
 use rustc_errors::{Applicability, PResult};
-use rustc_span::source_map::{respan, BytePos, Span};
-use rustc_span::symbol::{kw, sym, Symbol};
+use rustc_span::source_map::{BytePos, Span};
+use rustc_span::symbol::{kw, sym};
 use syntax::ast;
-use syntax::ast::{AttrStyle, AttrVec, Attribute, Mac, MacStmtStyle, VisibilityKind};
+use syntax::ast::{AttrStyle, AttrVec, Attribute, Mac, MacStmtStyle};
 use syntax::ast::{Block, BlockCheckMode, Expr, ExprKind, Local, Stmt, StmtKind, DUMMY_NODE_ID};
 use syntax::ptr::P;
 use syntax::token::{self, TokenKind};
@@ -55,21 +55,11 @@ impl<'a> Parser<'a> {
             return self.recover_stmt_local(lo, attrs.into(), msg, "let");
         }
 
-        let mac_vis = respan(lo, VisibilityKind::Inherited);
-        if let Some(macro_def) = self.eat_macro_def(&attrs, &mac_vis, lo)? {
-            return Ok(Some(self.mk_stmt(lo.to(self.prev_span), StmtKind::Item(macro_def))));
-        }
-
-        // Starts like a simple path, being careful to avoid contextual keywords
-        // such as a union items, item with `crate` visibility or auto trait items.
-        // Our goal here is to parse an arbitrary path `a::b::c` but not something that starts
-        // like a path (1 token), but it fact not a path.
-        if self.token.is_path_start()
-            && !self.token.is_qpath_start()
-            && !self.is_union_item() // `union::b::c` - path, `union U { ... }` - not a path.
-            && !self.is_crate_vis() // `crate::b::c` - path, `crate struct S;` - not a path.
-            && !self.is_auto_trait_item()
-            && !self.is_async_fn()
+        // Starts like a simple path, being careful to avoid contextual keywords,
+        // e.g., `union`, items with `crate` visibility, or `auto trait` items.
+        // We aim to parse an arbitrary path `a::b` but not something that starts like a path
+        // (1 token), but it fact not a path. Also, we avoid stealing syntax from `parse_item_`.
+        if self.token.is_path_start() && !self.token.is_qpath_start() && !self.is_path_start_item()
         {
             let path = self.parse_path(PathStyle::Expr)?;
 
@@ -199,10 +189,6 @@ impl<'a> Parser<'a> {
         }
     }
 
-    fn is_kw_followed_by_ident(&self, kw: Symbol) -> bool {
-        self.token.is_keyword(kw) && self.look_ahead(1, |t| t.is_ident() && !t.is_reserved_ident())
-    }
-
     fn recover_stmt_local(
         &mut self,
         lo: Span,
@@ -299,16 +285,6 @@ impl<'a> Parser<'a> {
         }
     }
 
-    fn is_auto_trait_item(&self) -> bool {
-        // auto trait
-        (self.token.is_keyword(kw::Auto) &&
-            self.is_keyword_ahead(1, &[kw::Trait]))
-        || // unsafe auto trait
-        (self.token.is_keyword(kw::Unsafe) &&
-         self.is_keyword_ahead(1, &[kw::Auto]) &&
-         self.is_keyword_ahead(2, &[kw::Trait]))
-    }
-
     /// Parses a block. No inner attributes are allowed.
     pub fn parse_block(&mut self) -> PResult<'a, P<Block>> {
         maybe_whole!(self, NtBlock, |x| x);

src/librustc_parse/parser/stmt.rs

@@ -0,0 +1,7 @@
+// pp-exact
+
+trait Foo {
+    #![allow(bar)]
+}
+
+fn main() { }

src/test/pretty/trait-inner-attr.rs

@@ -7,11 +7,11 @@ LL |     fn how_are_you(&self -> Empty {
    |                   |     help: `)` may belong here
    |                   unclosed delimiter
 
-error: expected one of `async`, `const`, `crate`, `default`, `extern`, `fn`, `pub`, `type`, `unsafe`, or `}`, found `)`
+error: expected one of `async`, `const`, `crate`, `default`, `extern`, `fn`, `pub`, `type`, `unsafe`, `}`, or identifier, found `)`
   --> $DIR/issue-58856-2.rs:11:1
    |
 LL |     }
-   |      - expected one of 10 possible tokens
+   |      - expected one of 11 possible tokens
 LL | }
    | ^ unexpected token
 

src/test/ui/issues/issue-58856-2.stderr

@@ -4,7 +4,7 @@ error: expected one of `.`, `;`, `?`, `else`, or an operator, found `}`
 LL |         });
    |          ^ expected one of `.`, `;`, `?`, `else`, or an operator
 
-error: expected one of `async`, `const`, `crate`, `default`, `extern`, `fn`, `pub`, `type`, `unsafe`, or `}`, found `;`
+error: expected one of `async`, `const`, `crate`, `default`, `extern`, `fn`, `pub`, `type`, `unsafe`, `}`, or identifier, found `;`
   --> $DIR/issue-60075.rs:6:11
    |
 LL |     fn qux() -> Option<usize> {

src/test/ui/issues/issue-60075.stderr

@@ -1,6 +1,7 @@
 macro_rules! m {
+    //~^ ERROR missing `fn`, `type`, or `static` for extern-item declaration
     () => {
-        let //~ ERROR expected
+        let
     };
 }
 

src/test/ui/macros/issue-54441.rs

@@ -1,13 +1,11 @@
-error: expected one of `async`, `const`, `crate`, `extern`, `fn`, `pub`, `static`, `type`, or `unsafe`, found keyword `let`
-  --> $DIR/issue-54441.rs:3:9
+error: missing `fn`, `type`, or `static` for extern-item declaration
+  --> $DIR/issue-54441.rs:1:1
    |
-LL |         let
-   |         ^^^ expected one of 9 possible tokens
-...
-LL |     m!();
-   |     ----- in this macro invocation
-   |
-   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
+LL | / macro_rules! m {
+LL | |
+LL | |     () => {
+LL | |         let
+   | |________^ missing `fn`, `type`, or `static`
 
 error: aborting due to previous error
 

src/test/ui/macros/issue-54441.stderr

@@ -1,8 +1,8 @@
 error: expected item after attributes
-  --> $DIR/attr-before-eof.rs:3:16
+  --> $DIR/attr-before-eof.rs:3:1
    |
 LL | #[derive(Debug)]
-   |                ^
+   | ^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/parser/attr-before-eof.stderr

@@ -1,8 +1,8 @@
 error: expected item after attributes
-  --> $DIR/attr-dangling-in-mod.rs:6:14
+  --> $DIR/attr-dangling-in-mod.rs:6:1
    |
 LL | #[foo = "bar"]
-   |              ^
+   | ^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/parser/attr-dangling-in-mod.stderr

@@ -1,13 +1,7 @@
-// Constants (static variables) can be used to match in patterns, but mutable
-// statics cannot. This ensures that there's some form of error if this is
-// attempted.
+// Make sure there's an error when given `extern { ... #[attr] }`.
 
-extern crate libc;
+fn main() {}
 
 extern {
-    static mut rust_dbg_static_mut: libc::c_int;
-    pub fn rust_dbg_static_mut_check_four();
     #[cfg(stage37)] //~ ERROR expected item after attributes
 }
-
-pub fn main() {}

src/test/ui/parser/attrs-after-extern-mod.rs

@@ -1,8 +1,8 @@
 error: expected item after attributes
-  --> $DIR/attrs-after-extern-mod.rs:10:19
+  --> $DIR/attrs-after-extern-mod.rs:6:5
    |
 LL |     #[cfg(stage37)]
-   |                   ^
+   |     ^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/parser/attrs-after-extern-mod.stderr

@@ -19,7 +19,8 @@ impl Foo for u16 {
 }
 
 impl Foo for u32 { //~ ERROR not all trait items implemented, missing: `foo`
-    default pub fn foo<T: Default>() -> T { T::default() } //~ ERROR expected one of
+    default pub fn foo<T: Default>() -> T { T::default() }
+    //~^ ERROR missing `fn`, `type`, or `const` for associated-item declaration
 }
 
 fn main() {}

src/test/ui/parser/default.rs

@@ -1,8 +1,8 @@
-error: expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe`, found keyword `pub`
-  --> $DIR/default.rs:22:13
+error: missing `fn`, `type`, or `const` for associated-item declaration
+  --> $DIR/default.rs:22:12
    |
 LL |     default pub fn foo<T: Default>() -> T { T::default() }
-   |             ^^^ expected one of `async`, `const`, `extern`, `fn`, `type`, or `unsafe`
+   |            ^ missing `fn`, `type`, or `const`
 
 error[E0449]: unnecessary visibility qualifier
   --> $DIR/default.rs:16:5

src/test/ui/parser/default.stderr

@@ -1,8 +1,10 @@
 error: expected item after attributes
-  --> $DIR/doc-before-attr.rs:4:16
+  --> $DIR/doc-before-attr.rs:4:1
    |
+LL | /// hi
+   | ------ other attributes here
 LL | #[derive(Debug)]
-   |                ^
+   | ^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/parser/doc-before-attr.stderr

@@ -1,4 +1,6 @@
+fn main() {}
+
 extern {
     /// hi
-    //~^ ERROR expected item after doc comment
+    //~^ ERROR found a documentation comment that doesn't document anything
 }

src/test/ui/parser/doc-before-extern-rbrace.rs

@@ -1,8 +1,11 @@
-error: expected item after doc comment
-  --> $DIR/doc-before-extern-rbrace.rs:2:5
+error[E0584]: found a documentation comment that doesn't document anything
+  --> $DIR/doc-before-extern-rbrace.rs:4:5
    |
 LL |     /// hi
    |     ^^^^^^ this doc comment doesn't document anything
+   |
+   = help: doc comments must come before what they document, maybe a comment was intended with `//`?
 
 error: aborting due to previous error
 
+For more information about this error, try `rustc --explain E0584`.

src/test/ui/parser/doc-before-extern-rbrace.stderr

@@ -2,7 +2,7 @@ error[E0584]: found a documentation comment that doesn't document anything
   --> $DIR/doc-inside-trait-item.rs:3:5
    |
 LL |     /// empty doc
-   |     ^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^ this doc comment doesn't document anything
    |
    = help: doc comments must come before what they document, maybe a comment was intended with `//`?
 

src/test/ui/parser/doc-inside-trait-item.stderr

@@ -1,4 +1,6 @@
-// error-pattern: expected one of `(`, `async`, `const`, `extern`, `fn`
+fn main() {}
+
 extern {
     pub pub fn foo();
+    //~^ ERROR missing `fn`, `type`, or `static` for extern-item declaration
 }

src/test/ui/parser/duplicate-visibility.rs

@@ -1,8 +1,8 @@
-error: expected one of `(`, `async`, `const`, `extern`, `fn`, `static`, `type`, or `unsafe`, found keyword `pub`
-  --> $DIR/duplicate-visibility.rs:3:9
+error: missing `fn`, `type`, or `static` for extern-item declaration
+  --> $DIR/duplicate-visibility.rs:4:8
    |
 LL |     pub pub fn foo();
-   |         ^^^ expected one of 8 possible tokens
+   |        ^ missing `fn`, `type`, or `static`
 
 error: aborting due to previous error
 

src/test/ui/parser/duplicate-visibility.stderr

@@ -0,0 +1,9 @@
+// check-pass
+
+#![deny(non_camel_case_types)]
+
+fn main() {}
+
+trait foo_bar {
+    #![allow(non_camel_case_types)]
+}

src/test/ui/parser/inner-attr-in-trait-def.rs

@@ -1,5 +1,6 @@
 trait T {
-    extern "Rust" unsafe fn foo(); //~ ERROR expected one of `async`, `const`
+    //~^ ERROR missing `fn`, `type`, or `const` for associated-item declaration
+    extern "Rust" unsafe fn foo();
 }
 
 fn main() {}

src/test/ui/parser/issue-19398.rs

@@ -1,10 +1,11 @@
-error: expected one of `async`, `const`, `crate`, `default`, `extern`, `fn`, `pub`, `type`, `unsafe`, or `}`, found keyword `extern`
-  --> $DIR/issue-19398.rs:2:5
+error: missing `fn`, `type`, or `const` for associated-item declaration
+  --> $DIR/issue-19398.rs:1:10
    |
-LL | trait T {
-   |          - expected one of 10 possible tokens
-LL |     extern "Rust" unsafe fn foo();
-   |     ^^^^^^ unexpected token
+LL |   trait T {
+   |  __________^
+LL | |
+LL | |     extern "Rust" unsafe fn foo();
+   | |____^ missing `fn`, `type`, or `const`
 
 error: aborting due to previous error