unify/improve/simplify attribute parsing

src/librustc_parse/parser/attr.rs

@@ -1,4 +1,4 @@
-use super::{Parser, PathStyle, TokenType};
+use super::{Parser, PathStyle};
 use rustc_ast::ast;
 use rustc_ast::attr;
 use rustc_ast::token::{self, Nonterminal};
@@ -10,14 +10,20 @@ use rustc_span::{Span, Symbol};
 use log::debug;
 
 #[derive(Debug)]
-enum InnerAttributeParsePolicy<'a> {
+pub(super) enum InnerAttrPolicy<'a> {
     Permitted,
-    NotPermitted { reason: &'a str, saw_doc_comment: bool, prev_attr_sp: Option<Span> },
+    Forbidden { reason: &'a str, saw_doc_comment: bool, prev_attr_sp: Option<Span> },
 }
 
 const DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG: &str = "an inner attribute is not \
                                                      permitted in this context";
 
+pub(super) const DEFAULT_INNER_ATTR_FORBIDDEN: InnerAttrPolicy<'_> = InnerAttrPolicy::Forbidden {
+    reason: DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG,
+    saw_doc_comment: false,
+    prev_attr_sp: None,
+};
+
 impl<'a> Parser<'a> {
     /// Parses attributes that appear before an item.
     pub(super) fn parse_outer_attributes(&mut self) -> PResult<'a, Vec<ast::Attribute>> {
@@ -25,48 +31,44 @@ impl<'a> Parser<'a> {
         let mut just_parsed_doc_comment = false;
         loop {
             debug!("parse_outer_attributes: self.token={:?}", self.token);
-            match self.token.kind {
+            if self.check(&token::Pound) {
-                token::Pound => {
+                let inner_error_reason = if just_parsed_doc_comment {
-                    let inner_error_reason = if just_parsed_doc_comment {
+                    "an inner attribute is not permitted following an outer doc comment"
-                        "an inner attribute is not permitted following an outer doc comment"
+                } else if !attrs.is_empty() {
-                    } else if !attrs.is_empty() {
+                    "an inner attribute is not permitted following an outer attribute"
-                        "an inner attribute is not permitted following an outer attribute"
+                } else {
-                    } else {
+                    DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG
-                        DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG
+                };
-                    };
+                let inner_parse_policy = InnerAttrPolicy::Forbidden {
-                    let inner_parse_policy = InnerAttributeParsePolicy::NotPermitted {
+                    reason: inner_error_reason,
-                        reason: inner_error_reason,
+                    saw_doc_comment: just_parsed_doc_comment,
-                        saw_doc_comment: just_parsed_doc_comment,
+                    prev_attr_sp: attrs.last().map(|a| a.span),
-                        prev_attr_sp: attrs.last().map(|a| a.span),
+                };
-                    };
+                let attr = self.parse_attribute_with_inner_parse_policy(inner_parse_policy)?;
-                    let attr = self.parse_attribute_with_inner_parse_policy(inner_parse_policy)?;
+                attrs.push(attr);
-                    attrs.push(attr);
+                just_parsed_doc_comment = false;
-                    just_parsed_doc_comment = false;
+            } else if let token::DocComment(s) = self.token.kind {
-                }
+                let attr = self.mk_doc_comment(s);
-                token::DocComment(s) => {
+                if attr.style != ast::AttrStyle::Outer {
-                    let attr = self.mk_doc_comment(s);
+                    self.struct_span_err(self.token.span, "expected outer doc comment")
-                    if attr.style != ast::AttrStyle::Outer {
+                        .note(
-                        let span = self.token.span;
-                        let mut err = self.struct_span_err(span, "expected outer doc comment");
-                        err.note(
                             "inner doc comments like this (starting with \
-                                  `//!` or `/*!`) can only appear before items",
+                              `//!` or `/*!`) can only appear before items",
-                        );
+                        )
-                        return Err(err);
+                        .emit();
-                    }
-                    attrs.push(attr);
-                    self.bump();
-                    just_parsed_doc_comment = true;
                 }
-                _ => break,
+                attrs.push(attr);
+                self.bump();
+                just_parsed_doc_comment = true;
+            } else {
+                break;
             }
         }
         Ok(attrs)
     }
 
     fn mk_doc_comment(&self, s: Symbol) -> ast::Attribute {
-        let style = comments::doc_comment_style(&s.as_str());
+        attr::mk_doc_comment(comments::doc_comment_style(&s.as_str()), s, self.token.span)
-        attr::mk_doc_comment(style, s, self.token.span)
     }
 
     /// Matches `attribute = # ! [ meta_item ]`.
@@ -75,96 +77,68 @@ impl<'a> Parser<'a> {
     /// attribute.
     pub fn parse_attribute(&mut self, permit_inner: bool) -> PResult<'a, ast::Attribute> {
         debug!("parse_attribute: permit_inner={:?} self.token={:?}", permit_inner, self.token);
-        let inner_parse_policy = if permit_inner {
+        let inner_parse_policy =
-            InnerAttributeParsePolicy::Permitted
+            if permit_inner { InnerAttrPolicy::Permitted } else { DEFAULT_INNER_ATTR_FORBIDDEN };
-        } else {
-            InnerAttributeParsePolicy::NotPermitted {
-                reason: DEFAULT_UNEXPECTED_INNER_ATTR_ERR_MSG,
-                saw_doc_comment: false,
-                prev_attr_sp: None,
-            }
-        };
         self.parse_attribute_with_inner_parse_policy(inner_parse_policy)
     }
 
-    /// The same as `parse_attribute`, except it takes in an `InnerAttributeParsePolicy`
+    /// The same as `parse_attribute`, except it takes in an `InnerAttrPolicy`
     /// that prescribes how to handle inner attributes.
     fn parse_attribute_with_inner_parse_policy(
         &mut self,
-        inner_parse_policy: InnerAttributeParsePolicy<'_>,
+        inner_parse_policy: InnerAttrPolicy<'_>,
     ) -> PResult<'a, ast::Attribute> {
         debug!(
             "parse_attribute_with_inner_parse_policy: inner_parse_policy={:?} self.token={:?}",
             inner_parse_policy, self.token
         );
-        let (span, item, style) = match self.token.kind {
+        let lo = self.token.span;
-            token::Pound => {
+        let (span, item, style) = if self.eat(&token::Pound) {
-                let lo = self.token.span;
+            let style =
-                self.bump();
+                if self.eat(&token::Not) { ast::AttrStyle::Inner } else { ast::AttrStyle::Outer };
 
-                if let InnerAttributeParsePolicy::Permitted = inner_parse_policy {
+            self.expect(&token::OpenDelim(token::Bracket))?;
-                    self.expected_tokens.push(TokenType::Token(token::Not));
+            let item = self.parse_attr_item()?;
-                }
+            self.expect(&token::CloseDelim(token::Bracket))?;
+            let attr_sp = lo.to(self.prev_token.span);
 
-                let style = if self.token == token::Not {
+            // Emit error if inner attribute is encountered and forbidden.
-                    self.bump();
+            if style == ast::AttrStyle::Inner {
-                    ast::AttrStyle::Inner
+                self.error_on_forbidden_inner_attr(attr_sp, inner_parse_policy);
-                } else {
-                    ast::AttrStyle::Outer
-                };
-
-                self.expect(&token::OpenDelim(token::Bracket))?;
-                let item = self.parse_attr_item()?;
-                self.expect(&token::CloseDelim(token::Bracket))?;
-                let hi = self.prev_token.span;
-
-                let attr_sp = lo.to(hi);
-
-                // Emit error if inner attribute is encountered and not permitted
-                if style == ast::AttrStyle::Inner {
-                    if let InnerAttributeParsePolicy::NotPermitted {
-                        reason,
-                        saw_doc_comment,
-                        prev_attr_sp,
-                    } = inner_parse_policy
-                    {
-                        let prev_attr_note = if saw_doc_comment {
-                            "previous doc comment"
-                        } else {
-                            "previous outer attribute"
-                        };
-
-                        let mut diagnostic = self.struct_span_err(attr_sp, reason);
-
-                        if let Some(prev_attr_sp) = prev_attr_sp {
-                            diagnostic
-                                .span_label(attr_sp, "not permitted following an outer attribute")
-                                .span_label(prev_attr_sp, prev_attr_note);
-                        }
-
-                        diagnostic
-                            .note(
-                                "inner attributes, like `#![no_std]`, annotate the item \
-                                   enclosing them, and are usually found at the beginning of \
-                                   source files. Outer attributes, like `#[test]`, annotate the \
-                                   item following them.",
-                            )
-                            .emit();
-                    }
-                }
-
-                (attr_sp, item, style)
-            }
-            _ => {
-                let token_str = pprust::token_to_string(&self.token);
-                let msg = &format!("expected `#`, found `{}`", token_str);
-                return Err(self.struct_span_err(self.token.span, msg));
             }
+
+            (attr_sp, item, style)
+        } else {
+            let token_str = pprust::token_to_string(&self.token);
+            let msg = &format!("expected `#`, found `{}`", token_str);
+            return Err(self.struct_span_err(self.token.span, msg));
         };
 
         Ok(attr::mk_attr_from_item(style, item, span))
     }
 
+    pub(super) fn error_on_forbidden_inner_attr(&self, attr_sp: Span, policy: InnerAttrPolicy<'_>) {
+        if let InnerAttrPolicy::Forbidden { reason, saw_doc_comment, prev_attr_sp } = policy {
+            let prev_attr_note =
+                if saw_doc_comment { "previous doc comment" } else { "previous outer attribute" };
+
+            let mut diag = self.struct_span_err(attr_sp, reason);
+
+            if let Some(prev_attr_sp) = prev_attr_sp {
+                diag.span_label(attr_sp, "not permitted following an outer attribute")
+                    .span_label(prev_attr_sp, prev_attr_note);
+            }
+
+            diag.note(
+                "inner attributes, like `#![no_std]`, annotate the item \
+                       enclosing them, and are usually found at the beginning of \
+                       source files. Outer attributes, like `#[test]`, annotate the \
+                       item following them.",
+            )
+            .emit();
+        }
+    }
+
     /// Parses an inner part of an attribute (the path and following tokens).
     /// The tokens must be either a delimited token stream, or empty token stream,
     /// or the "legacy" key-value form.
@@ -200,24 +174,22 @@ impl<'a> Parser<'a> {
     crate fn parse_inner_attributes(&mut self) -> PResult<'a, Vec<ast::Attribute>> {
         let mut attrs: Vec<ast::Attribute> = vec![];
         loop {
-            match self.token.kind {
+            // Only try to parse if it is an inner attribute (has `!`).
-                // Only try to parse if it is an inner attribute (has `!`).
+            if self.check(&token::Pound) && self.look_ahead(1, |t| t == &token::Not) {
-                token::Pound if self.look_ahead(1, |t| t == &token::Not) => {
+                let attr = self.parse_attribute(true)?;
-                    let attr = self.parse_attribute(true)?;
+                assert_eq!(attr.style, ast::AttrStyle::Inner);
-                    assert_eq!(attr.style, ast::AttrStyle::Inner);
+                attrs.push(attr);
+            } else if let token::DocComment(s) = self.token.kind {
+                // We need to get the position of this token before we bump.
+                let attr = self.mk_doc_comment(s);
+                if attr.style == ast::AttrStyle::Inner {
                     attrs.push(attr);
+                    self.bump();
+                } else {
+                    break;
                 }
-                token::DocComment(s) => {
+            } else {
-                    // We need to get the position of this token before we bump.
+                break;
-                    let attr = self.mk_doc_comment(s);
-                    if attr.style == ast::AttrStyle::Inner {
-                        attrs.push(attr);
-                        self.bump();
-                    } else {
-                        break;
-                    }
-                }
-                _ => break,
             }
         }
         Ok(attrs)
@@ -228,8 +200,7 @@ impl<'a> Parser<'a> {
         debug!("checking if {:?} is unusuffixed", lit);
 
         if !lit.kind.is_unsuffixed() {
-            let msg = "suffixed literals are not allowed in attributes";
+            self.struct_span_err(lit.span, "suffixed literals are not allowed in attributes")
-            self.struct_span_err(lit.span, msg)
                 .help(
                     "instead of using a suffixed literal \
                                     (`1u8`, `1.0f32`, etc.), use an unsuffixed version \

src/librustc_parse/parser/stmt.rs

@@ -1,3 +1,4 @@
+use super::attr::DEFAULT_INNER_ATTR_FORBIDDEN;
 use super::diagnostics::Error;
 use super::expr::LhsExpr;
 use super::pat::GateOr;
@@ -238,13 +239,11 @@ impl<'a> Parser<'a> {
 
     /// Parses a block. No inner attributes are allowed.
     pub fn parse_block(&mut self) -> PResult<'a, P<Block>> {
-        maybe_whole!(self, NtBlock, |x| x);
+        let (attrs, block) = self.parse_inner_attrs_and_block()?;
-
+        if let [.., last] = &*attrs {
-        if !self.eat(&token::OpenDelim(token::Brace)) {
+            self.error_on_forbidden_inner_attr(last.span, DEFAULT_INNER_ATTR_FORBIDDEN);
-            return self.error_block_no_opening_brace();
         }
-
+        Ok(block)
-        self.parse_block_tail(self.prev_token.span, BlockCheckMode::Default)
     }
 
     fn error_block_no_opening_brace<T>(&mut self) -> PResult<'a, T> {

src/test/ui/did_you_mean/issue-40006.stderr

@@ -32,11 +32,11 @@ LL |     X() {}
 LL | }
    | - the item list ends here
 
-error: expected `[`, found `#`
+error: expected one of `!` or `[`, found `#`
   --> $DIR/issue-40006.rs:19:17
    |
 LL |     fn xxx() { ### }
-   |                 ^ expected `[`
+   |                 ^ expected one of `!` or `[`
 
 error: expected one of `!` or `::`, found `=`
   --> $DIR/issue-40006.rs:22:7

src/test/ui/generic-associated-types/empty_generics.rs

@@ -3,7 +3,7 @@
 
 trait Foo {
     type Bar<,>;
-    //~^ ERROR expected one of `>`, `const`, identifier, or lifetime, found `,`
+    //~^ ERROR expected one of `#`, `>`, `const`, identifier, or lifetime, found `,`
 }
 
 fn main() {}

src/test/ui/generic-associated-types/empty_generics.stderr

@@ -1,10 +1,10 @@
-error: expected one of `>`, `const`, identifier, or lifetime, found `,`
+error: expected one of `#`, `>`, `const`, identifier, or lifetime, found `,`
   --> $DIR/empty_generics.rs:5:14
    |
 LL | trait Foo {
    |           - while parsing this item list starting here
 LL |     type Bar<,>;
-   |              ^ expected one of `>`, `const`, identifier, or lifetime
+   |              ^ expected one of `#`, `>`, `const`, identifier, or lifetime
 LL |
 LL | }
    | - the item list ends here

src/test/ui/issues/issue-20616-8.rs

@@ -29,7 +29,7 @@ type Type_5_<'a> = Type_1_<'a, ()>;
 
 
 type Type_8<'a,,> = &'a ();
-//~^ error: expected one of `>`, `const`, identifier, or lifetime, found `,`
+//~^ error: expected one of `#`, `>`, `const`, identifier, or lifetime, found `,`
 
 
 //type Type_9<T,,> = Box<T>; // error: expected identifier, found `,`

src/test/ui/issues/issue-20616-8.stderr

@@ -1,8 +1,8 @@
-error: expected one of `>`, `const`, identifier, or lifetime, found `,`
+error: expected one of `#`, `>`, `const`, identifier, or lifetime, found `,`
   --> $DIR/issue-20616-8.rs:31:16
    |
 LL | type Type_8<'a,,> = &'a ();
-   |                ^ expected one of `>`, `const`, identifier, or lifetime
+   |                ^ expected one of `#`, `>`, `const`, identifier, or lifetime
 
 error: aborting due to previous error
 

src/test/ui/issues/issue-20616-9.rs

@@ -32,4 +32,4 @@ type Type_5_<'a> = Type_1_<'a, ()>;
 
 
 type Type_9<T,,> = Box<T>;
-//~^ error: expected one of `>`, `const`, identifier, or lifetime, found `,`
+//~^ error: expected one of `#`, `>`, `const`, identifier, or lifetime, found `,`

src/test/ui/issues/issue-20616-9.stderr

@@ -1,8 +1,8 @@
-error: expected one of `>`, `const`, identifier, or lifetime, found `,`
+error: expected one of `#`, `>`, `const`, identifier, or lifetime, found `,`
   --> $DIR/issue-20616-9.rs:34:15
    |
 LL | type Type_9<T,,> = Box<T>;
-   |               ^ expected one of `>`, `const`, identifier, or lifetime
+   |               ^ expected one of `#`, `>`, `const`, identifier, or lifetime
 
 error: aborting due to previous error
 

src/test/ui/parser/bounds-lifetime.rs

@@ -6,6 +6,6 @@ type A = for<'a: 'b + 'c> fn(); // OK (rejected later by ast_validation)
 type A = for<'a: 'b,> fn(); // OK(rejected later by ast_validation)
 type A = for<'a: 'b +> fn(); // OK (rejected later by ast_validation)
 type A = for<'a, T> fn(); // OK (rejected later by ast_validation)
-type A = for<,> fn(); //~ ERROR expected one of `>`, `const`, identifier, or lifetime, found `,`
+type A = for<,> fn(); //~ ERROR expected one of `#`, `>`, `const`, identifier, or lifetime
 
 fn main() {}

src/test/ui/parser/bounds-lifetime.stderr

@@ -1,8 +1,8 @@
-error: expected one of `>`, `const`, identifier, or lifetime, found `,`
+error: expected one of `#`, `>`, `const`, identifier, or lifetime, found `,`
   --> $DIR/bounds-lifetime.rs:9:14
    |
 LL | type A = for<,> fn();
-   |              ^ expected one of `>`, `const`, identifier, or lifetime
+   |              ^ expected one of `#`, `>`, `const`, identifier, or lifetime
 
 error: aborting due to previous error
 

src/test/ui/parser/column-offset-1-based.rs

@@ -1 +1 @@
-# //~ ERROR expected `[`, found `<eof>`
+# //~ ERROR expected one of `!` or `[`, found `<eof>`

src/test/ui/parser/column-offset-1-based.stderr

@@ -1,8 +1,8 @@
-error: expected `[`, found `<eof>`
+error: expected one of `!` or `[`, found `<eof>`
   --> $DIR/column-offset-1-based.rs:1:1
    |
 LL | #
-   | ^ expected `[`
+   | ^ expected one of `!` or `[`
 
 error: aborting due to previous error
 

src/test/ui/parser/doc-comment-in-if-statement.rs

@@ -1,4 +1,5 @@
 fn main() {
     if true /*!*/ {}
     //~^ ERROR expected `{`, found doc comment `/*!*/`
+    //~| ERROR expected outer doc comment
 }

src/test/ui/parser/doc-comment-in-if-statement.stderr

@@ -1,10 +1,19 @@
+error: expected outer doc comment
+  --> $DIR/doc-comment-in-if-statement.rs:2:13
+   |
+LL |     if true /*!*/ {}
+   |             ^^^^^
+   |
+   = note: inner doc comments like this (starting with `//!` or `/*!`) can only appear before items
+
 error: expected `{`, found doc comment `/*!*/`
   --> $DIR/doc-comment-in-if-statement.rs:2:13
    |
 LL |     if true /*!*/ {}
-   |     --      ^^^^^ expected `{`
+   |     --      ^^^^^ -- help: try placing this code inside a block: `{ {} }`
-   |     |
+   |     |       |
+   |     |       expected `{`
    |     this `if` expression has a condition, but no block
 
-error: aborting due to previous error
+error: aborting due to 2 previous errors
 

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

@@ -1,6 +1,5 @@
-// error-pattern:expected `[`, found `vec`
 mod blade_runner {
-    #vec[doc(
+    #vec[doc( //~ ERROR expected one of `!` or `[`, found `vec`
         brief = "Blade Runner is probably the best movie ever",
         desc = "I like that in the world of Blade Runner it is always
                 raining, and that it's always night time. And Aliens

src/test/ui/parser/issue-1655.stderr

@@ -1,8 +1,8 @@
-error: expected `[`, found `vec`
+error: expected one of `!` or `[`, found `vec`
-  --> $DIR/issue-1655.rs:3:6
+  --> $DIR/issue-1655.rs:2:6
    |
 LL |     #vec[doc(
-   |      ^^^ expected `[`
+   |      ^^^ expected one of `!` or `[`
 
 error: aborting due to previous error
 

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

@@ -1,3 +1,3 @@
-// error-pattern: aborting due to 7 previous errors
+// error-pattern: aborting due to 5 previous errors
 
 fn i(n{...,f #

src/test/ui/parser/issue-63135.stderr

@@ -31,23 +31,11 @@ LL | fn i(n{...,f #
    |        |  expected `}`
    |        `..` must be at the end and cannot have a trailing comma
 
-error: expected `[`, found `}`
+error: expected one of `!` or `[`, found `}`
   --> $DIR/issue-63135.rs:3:16
    |
 LL | fn i(n{...,f #
-   |                ^ expected `[`
+   |                ^ expected one of `!` or `[`
 
-error: expected one of `:` or `|`, found `)`
+error: aborting due to 5 previous errors
-  --> $DIR/issue-63135.rs:3:16
-   |
-LL | fn i(n{...,f #
-   |                ^ expected one of `:` or `|`
-
-error: expected `;` or `{`, found `<eof>`
-  --> $DIR/issue-63135.rs:3:16
-   |
-LL | fn i(n{...,f #
-   |                ^ expected `;` or `{`
-
-error: aborting due to 7 previous errors