Rollup merge of #127806 - nnethercote:parser-improvements, r=spastorino

Some parser improvements

I was looking closely at attribute handling in the parser while debugging some issues relating to #124141, and found a few small improvements.

``@spastorino``

compiler/rustc_ast/src/token.rs

@@ -699,8 +699,7 @@ impl Token {
         false
     }
 
-    /// Would `maybe_whole_expr` in `parser.rs` return `Ok(..)`?
-    /// That is, is this a pre-parsed expression dropped into the token stream
+    /// Is this a pre-parsed expression dropped into the token stream
     /// (which happens while parsing the result of macro expansion)?
     pub fn is_whole_expr(&self) -> bool {
         if let Interpolated(nt) = &self.kind

compiler/rustc_parse/src/parser/expr.rs

@@ -785,23 +785,14 @@ impl<'a> Parser<'a> {
             }
         };
 
-        self.parse_and_disallow_postfix_after_cast(cast_expr)
-    }
-
-    /// Parses a postfix operators such as `.`, `?`, or index (`[]`) after a cast,
-    /// then emits an error and returns the newly parsed tree.
-    /// The resulting parse tree for `&x as T[0]` has a precedence of `((&x) as T)[0]`.
-    fn parse_and_disallow_postfix_after_cast(
-        &mut self,
-        cast_expr: P<Expr>,
-    ) -> PResult<'a, P<Expr>> {
-        if let ExprKind::Type(_, _) = cast_expr.kind {
-            panic!("ExprKind::Type must not be parsed");
-        }
+        // Try to parse a postfix operator such as `.`, `?`, or index (`[]`)
+        // after a cast. If one is present, emit an error then return a valid
+        // parse tree; For something like `&x as T[0]` will be as if it was
+        // written `((&x) as T)[0]`.
 
         let span = cast_expr.span;
 
-        let with_postfix = self.parse_expr_dot_or_call_with_(cast_expr, span)?;
+        let with_postfix = self.parse_expr_dot_or_call_with(AttrVec::new(), cast_expr, span)?;
 
         // Check if an illegal postfix operator has been added after the cast.
         // If the resulting expression is not a cast, it is an illegal postfix operator.
@@ -885,23 +876,63 @@ impl<'a> Parser<'a> {
         self.collect_tokens_for_expr(attrs, |this, attrs| {
             let base = this.parse_expr_bottom()?;
             let span = this.interpolated_or_expr_span(&base);
-            this.parse_expr_dot_or_call_with(base, span, attrs)
+            this.parse_expr_dot_or_call_with(attrs, base, span)
         })
     }
 
     pub(super) fn parse_expr_dot_or_call_with(
         &mut self,
-        e0: P<Expr>,
-        lo: Span,
         mut attrs: ast::AttrVec,
+        mut e: P<Expr>,
+        lo: Span,
     ) -> PResult<'a, P<Expr>> {
-        // Stitch the list of outer attributes onto the return value.
-        // A little bit ugly, but the best way given the current code
-        // structure
-        let res = ensure_sufficient_stack(
-            // this expr demonstrates the recursion it guards against
-            || self.parse_expr_dot_or_call_with_(e0, lo),
-        );
+        let res = ensure_sufficient_stack(|| {
+            loop {
+                let has_question =
+                    if self.prev_token.kind == TokenKind::Ident(kw::Return, IdentIsRaw::No) {
+                        // We are using noexpect here because we don't expect a `?` directly after
+                        // a `return` which could be suggested otherwise.
+                        self.eat_noexpect(&token::Question)
+                    } else {
+                        self.eat(&token::Question)
+                    };
+                if has_question {
+                    // `expr?`
+                    e = self.mk_expr(lo.to(self.prev_token.span), ExprKind::Try(e));
+                    continue;
+                }
+                let has_dot =
+                    if self.prev_token.kind == TokenKind::Ident(kw::Return, IdentIsRaw::No) {
+                        // We are using noexpect here because we don't expect a `.` directly after
+                        // a `return` which could be suggested otherwise.
+                        self.eat_noexpect(&token::Dot)
+                    } else if self.token.kind == TokenKind::RArrow && self.may_recover() {
+                        // Recovery for `expr->suffix`.
+                        self.bump();
+                        let span = self.prev_token.span;
+                        self.dcx().emit_err(errors::ExprRArrowCall { span });
+                        true
+                    } else {
+                        self.eat(&token::Dot)
+                    };
+                if has_dot {
+                    // expr.f
+                    e = self.parse_dot_suffix_expr(lo, e)?;
+                    continue;
+                }
+                if self.expr_is_complete(&e) {
+                    return Ok(e);
+                }
+                e = match self.token.kind {
+                    token::OpenDelim(Delimiter::Parenthesis) => self.parse_expr_fn_call(lo, e),
+                    token::OpenDelim(Delimiter::Bracket) => self.parse_expr_index(lo, e)?,
+                    _ => return Ok(e),
+                }
+            }
+        });
+
+        // Stitch the list of outer attributes onto the return value. A little
+        // bit ugly, but the best way given the current code structure.
         if attrs.is_empty() {
             res
         } else {
@@ -915,50 +946,6 @@ impl<'a> Parser<'a> {
         }
     }
 
-    fn parse_expr_dot_or_call_with_(&mut self, mut e: P<Expr>, lo: Span) -> PResult<'a, P<Expr>> {
-        loop {
-            let has_question =
-                if self.prev_token.kind == TokenKind::Ident(kw::Return, IdentIsRaw::No) {
-                    // we are using noexpect here because we don't expect a `?` directly after a `return`
-                    // which could be suggested otherwise
-                    self.eat_noexpect(&token::Question)
-                } else {
-                    self.eat(&token::Question)
-                };
-            if has_question {
-                // `expr?`
-                e = self.mk_expr(lo.to(self.prev_token.span), ExprKind::Try(e));
-                continue;
-            }
-            let has_dot = if self.prev_token.kind == TokenKind::Ident(kw::Return, IdentIsRaw::No) {
-                // we are using noexpect here because we don't expect a `.` directly after a `return`
-                // which could be suggested otherwise
-                self.eat_noexpect(&token::Dot)
-            } else if self.token.kind == TokenKind::RArrow && self.may_recover() {
-                // Recovery for `expr->suffix`.
-                self.bump();
-                let span = self.prev_token.span;
-                self.dcx().emit_err(errors::ExprRArrowCall { span });
-                true
-            } else {
-                self.eat(&token::Dot)
-            };
-            if has_dot {
-                // expr.f
-                e = self.parse_dot_suffix_expr(lo, e)?;
-                continue;
-            }
-            if self.expr_is_complete(&e) {
-                return Ok(e);
-            }
-            e = match self.token.kind {
-                token::OpenDelim(Delimiter::Parenthesis) => self.parse_expr_fn_call(lo, e),
-                token::OpenDelim(Delimiter::Bracket) => self.parse_expr_index(lo, e)?,
-                _ => return Ok(e),
-            }
-        }
-    }
-
     pub(super) fn parse_dot_suffix_expr(
         &mut self,
         lo: Span,
@@ -1388,7 +1375,7 @@ impl<'a> Parser<'a> {
     /// Parses things like parenthesized exprs, macros, `return`, etc.
     ///
     /// N.B., this does not parse outer attributes, and is private because it only works
-    /// correctly if called from `parse_dot_or_call_expr()`.
+    /// correctly if called from `parse_expr_dot_or_call`.
     fn parse_expr_bottom(&mut self) -> PResult<'a, P<Expr>> {
         maybe_recover_from_interpolated_ty_qpath!(self, true);
 

compiler/rustc_parse/src/parser/item.rs

@@ -128,56 +128,41 @@ impl<'a> Parser<'a> {
             Some(item.into_inner())
         });
 
-        let item =
-            self.collect_tokens_trailing_token(attrs, force_collect, |this: &mut Self, attrs| {
-                let item =
-                    this.parse_item_common_(attrs, mac_allowed, attrs_allowed, fn_parse_mode);
-                Ok((item?, TrailingToken::None))
-            })?;
+        self.collect_tokens_trailing_token(attrs, force_collect, |this, mut attrs| {
+            let lo = this.token.span;
+            let vis = this.parse_visibility(FollowedByType::No)?;
+            let mut def = this.parse_defaultness();
+            let kind = this.parse_item_kind(
+                &mut attrs,
+                mac_allowed,
+                lo,
+                &vis,
+                &mut def,
+                fn_parse_mode,
+                Case::Sensitive,
+            )?;
+            if let Some((ident, kind)) = kind {
+                this.error_on_unconsumed_default(def, &kind);
+                let span = lo.to(this.prev_token.span);
+                let id = DUMMY_NODE_ID;
+                let item = Item { ident, attrs, id, kind, vis, span, tokens: None };
+                return Ok((Some(item), TrailingToken::None));
+            }
 
-        Ok(item)
-    }
+            // At this point, we have failed to parse an item.
+            if !matches!(vis.kind, VisibilityKind::Inherited) {
+                this.dcx().emit_err(errors::VisibilityNotFollowedByItem { span: vis.span, vis });
+            }
 
-    fn parse_item_common_(
-        &mut self,
-        mut attrs: AttrVec,
-        mac_allowed: bool,
-        attrs_allowed: bool,
-        fn_parse_mode: FnParseMode,
-    ) -> PResult<'a, Option<Item>> {
-        let lo = self.token.span;
-        let vis = self.parse_visibility(FollowedByType::No)?;
-        let mut def = self.parse_defaultness();
-        let kind = self.parse_item_kind(
-            &mut attrs,
-            mac_allowed,
-            lo,
-            &vis,
-            &mut def,
-            fn_parse_mode,
-            Case::Sensitive,
-        )?;
-        if let Some((ident, kind)) = kind {
-            self.error_on_unconsumed_default(def, &kind);
-            let span = lo.to(self.prev_token.span);
-            let id = DUMMY_NODE_ID;
-            let item = Item { ident, attrs, id, kind, vis, span, tokens: None };
-            return Ok(Some(item));
-        }
+            if let Defaultness::Default(span) = def {
+                this.dcx().emit_err(errors::DefaultNotFollowedByItem { span });
+            }
 
-        // At this point, we have failed to parse an item.
-        if !matches!(vis.kind, VisibilityKind::Inherited) {
-            self.dcx().emit_err(errors::VisibilityNotFollowedByItem { span: vis.span, vis });
-        }
-
-        if let Defaultness::Default(span) = def {
-            self.dcx().emit_err(errors::DefaultNotFollowedByItem { span });
-        }
-
-        if !attrs_allowed {
-            self.recover_attrs_no_item(&attrs)?;
-        }
-        Ok(None)
+            if !attrs_allowed {
+                this.recover_attrs_no_item(&attrs)?;
+            }
+            Ok((None, TrailingToken::None))
+        })
     }
 
     /// Error in-case `default` was parsed in an in-appropriate context.

compiler/rustc_parse/src/parser/mod.rs

@@ -101,7 +101,6 @@ pub enum TrailingToken {
     MaybeComma,
 }
 
-/// Like `maybe_whole_expr`, but for things other than expressions.
 #[macro_export]
 macro_rules! maybe_whole {
     ($p:expr, $constructor:ident, |$x:ident| $e:expr) => {

compiler/rustc_parse/src/parser/pat.rs

@@ -392,9 +392,9 @@ impl<'a> Parser<'a> {
         // Parse `?`, `.f`, `(arg0, arg1, ...)` or `[expr]` until they've all been eaten.
         if let Ok(expr) = snapshot
             .parse_expr_dot_or_call_with(
+                AttrVec::new(),
                 self.mk_expr(pat_span, ExprKind::Dummy), // equivalent to transforming the parsed pattern into an `Expr`
                 pat_span,
-                AttrVec::new(),
             )
             .map_err(|err| err.cancel())
         {

compiler/rustc_parse/src/parser/stmt.rs

@@ -164,7 +164,7 @@ impl<'a> Parser<'a> {
             };
 
             let expr = this.with_res(Restrictions::STMT_EXPR, |this| {
-                this.parse_expr_dot_or_call_with(expr, lo, attrs)
+                this.parse_expr_dot_or_call_with(attrs, expr, lo)
             })?;
             // `DUMMY_SP` will get overwritten later in this function
             Ok((this.mk_stmt(rustc_span::DUMMY_SP, StmtKind::Expr(expr)), TrailingToken::None))
@@ -206,7 +206,7 @@ impl<'a> Parser<'a> {
             // Since none of the above applied, this is an expression statement macro.
             let e = self.mk_expr(lo.to(hi), ExprKind::MacCall(mac));
             let e = self.maybe_recover_from_bad_qpath(e)?;
-            let e = self.parse_expr_dot_or_call_with(e, lo, attrs)?;
+            let e = self.parse_expr_dot_or_call_with(attrs, e, lo)?;
             let e = self
                 .parse_expr_assoc_with(0, LhsExpr::Parsed { expr: e, starts_statement: false })?;
             StmtKind::Expr(e)