Rollup merge of rust-lang#159849 - petrochenkov:estmt, r=JonathanBrouwer

rustc_parse: Stop returning `Option` from statement parsing

`parse_stmt_without_recovery` had one corner case in which it returned `Ok(None)` - when parsing immediately encountered a closing brace `}`, possibly after parsing outer attributes.

It is simpler to never call `parse_stmt_without_recovery` in such contexts than deal with a possibility of no statement being returned without an error.
So this PR changes the function's return type from `PResult<'a, Option<Stmt>>` to `PResult<'a, Stmt>` and adjusts one call site to check for a closing brace.

Author: JonathanBrouwer

compiler/rustc_attr_parsing/src/parser.rs

@@ -660,9 +660,8 @@ impl<'a, 'sess> MetaItemListParserContext<'a, 'sess> {
         // don't `uninterpolate` the token to avoid suggesting anything butchered or questionable
         // when macro metavariables are involved.
         let snapshot = self.parser.create_snapshot_for_diagnostic();
-        let stmt = self.parser.parse_stmt_without_recovery(false, ForceCollect::No, false);
-        match stmt {
-            Ok(Some(stmt)) => {
+        match self.parser.parse_stmt_without_recovery(false, ForceCollect::No, false) {
+            Ok(stmt) => {
                 // The user tried to write something like
                 // `#[deprecated(note = concat!("a", "b"))]`.
                 err.descr = stmt.kind.descr().to_string();
@@ -692,7 +691,6 @@ impl<'a, 'sess> MetaItemListParserContext<'a, 'sess> {
                     });
                 }
             }
-            Ok(None) => {}
             Err(e) => {
                 e.cancel();
                 self.parser.restore_snapshot(snapshot);

compiler/rustc_builtin_macros/src/cfg_eval.rs

@@ -126,9 +126,8 @@ impl CfgEval<'_> {
                     Annotatable::ForeignItem(self.flat_map_foreign_item(item).pop().unwrap())
                 }
                 Annotatable::Stmt(_) => {
-                    let stmt = parser
-                        .parse_stmt_without_recovery(false, ForceCollect::Yes, false)?
-                        .unwrap();
+                    let stmt =
+                        parser.parse_stmt_without_recovery(false, ForceCollect::Yes, false)?;
                     Annotatable::Stmt(Box::new(self.flat_map_stmt(stmt).pop().unwrap()))
                 }
                 Annotatable::Expr(_) => {

compiler/rustc_expand/src/expand.rs

@@ -1120,9 +1120,7 @@ pub fn parse_ast_fragment<'a>(
             let mut stmts = SmallVec::new();
             // Won't make progress on a `}`.
             while this.token != token::Eof && this.token != token::CloseBrace {
-                if let Some(stmt) = this.parse_full_stmt(AttemptLocalParseRecovery::Yes)? {
-                    stmts.push(stmt);
-                }
+                stmts.push(this.parse_full_stmt(AttemptLocalParseRecovery::Yes)?);
             }
             AstFragment::Stmts(stmts)
         }

compiler/rustc_parse/src/parser/expr.rs

@@ -3331,13 +3331,9 @@ impl<'a> Parser<'a> {
             self.restore_snapshot(pre_pat_snapshot);
             match self.parse_stmt_without_recovery(true, ForceCollect::No, false) {
                 // Consume statements for as long as possible.
-                Ok(Some(stmt)) => {
+                Ok(stmt) => {
                     stmts.push(stmt);
                 }
-                Ok(None) => {
-                    self.restore_snapshot(start_snapshot);
-                    break;
-                }
                 // We couldn't parse either yet another statement missing it's
                 // enclosing block nor the next arm's pattern or closing brace.
                 Err(stmt_err) => {

compiler/rustc_parse/src/parser/mod.rs

@@ -987,12 +987,9 @@ impl<'a> Parser<'a> {
         let initial_semicolon = self.token.span;
 
         while self.eat(exp!(Semi)) {
-            let _ = self
-                .parse_stmt_without_recovery(false, ForceCollect::No, false)
-                .unwrap_or_else(|e| {
-                    e.cancel();
-                    None
-                });
+            if let Err(e) = self.parse_stmt_without_recovery(false, ForceCollect::No, false) {
+                e.cancel();
+            }
         }
 
         expect_err

compiler/rustc_parse/src/parser/nonterminal.rs

@@ -139,7 +139,7 @@ impl<'a> Parser<'a> {
                     this.parse_block().map(|block| WithTokens::new(block))
                 })?))
             }
-            NonterminalKind::Stmt => match self.parse_stmt(ForceCollect::Yes)? {
+            NonterminalKind::Stmt => match self.parse_stmt_nonterminal(ForceCollect::Yes) {
                 Some(stmt) => Ok(ParseNtResult::Stmt(Box::new(stmt))),
                 None => {
                     Err(self.dcx().create_err(UnexpectedNonterminal::Statement(self.token.span)))

compiler/rustc_parse/src/parser/stmt.rs

@@ -27,18 +27,22 @@ use crate::diagnostics::{self, MalformedLoopLabel};
 use crate::exp;
 
 impl<'a> Parser<'a> {
-    /// Parses a statement. This stops just before trailing semicolons on everything but items.
+    /// Parses a statement nonterminal, which has a peculiar syntax preserved for backward
+    /// compatibility. The parsing stops just before trailing semicolons on everything but items.
     /// e.g., a `StmtKind::Semi` parses to a `StmtKind::Expr`, leaving the trailing `;` unconsumed.
     ///
     /// If `force_collect` is [`ForceCollect::Yes`], forces collection of tokens regardless of
     /// whether or not we have attributes.
     // Public for rustfmt usage.
-    pub fn parse_stmt(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<Stmt>> {
-        Ok(self.parse_stmt_without_recovery(false, force_collect, false).unwrap_or_else(|e| {
-            e.emit();
-            self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore);
-            None
-        }))
+    pub fn parse_stmt_nonterminal(&mut self, force_collect: ForceCollect) -> Option<Stmt> {
+        match self.parse_stmt_without_recovery(false, force_collect, false) {
+            Ok(stmt) => Some(stmt),
+            Err(e) => {
+                e.emit();
+                self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore);
+                None
+            }
+        }
     }
 
     /// If `force_collect` is [`ForceCollect::Yes`], forces collection of tokens regardless of
@@ -49,19 +53,18 @@ impl<'a> Parser<'a> {
         capture_semi: bool,
         force_collect: ForceCollect,
         force_full_expr: bool,
-    ) -> PResult<'a, Option<Stmt>> {
+    ) -> PResult<'a, Stmt> {
         let pre_attr_pos = self.collect_pos();
         let attrs = self.parse_outer_attributes()?;
         let lo = self.token.span;
 
-        if let Some(stmt) = self.eat_metavar_seq(MetaVarKind::Stmt, |this| {
+        if let Some(mut stmt) = self.eat_metavar_seq(MetaVarKind::Stmt, |this| {
             this.parse_stmt_without_recovery(false, ForceCollect::Yes, false)
         }) {
-            let mut stmt = stmt.expect("an actual statement");
             stmt.visit_attrs(|stmt_attrs| {
                 attrs.prepend_to_nt_inner(stmt_attrs);
             });
-            return Ok(Some(stmt));
+            return Ok(stmt);
         }
 
         if self.token.is_keyword(kw::Mut) && self.is_keyword_ahead(1, &[kw::Let]) {
@@ -161,9 +164,12 @@ impl<'a> Parser<'a> {
             self.mk_stmt(lo.to(item.span), StmtKind::Item(Box::new(item)))
         } else if self.eat(exp!(Semi)) {
             // Do not attempt to parse an expression if we're done here.
-            self.error_outer_attrs(attrs);
+            self.error_outer_attrs(attrs)?;
             self.mk_stmt(lo, StmtKind::Empty)
-        } else if self.token != token::CloseBrace {
+        } else if self.token == token::CloseBrace {
+            self.error_outer_attrs(attrs)?;
+            self.dcx().span_bug(self.token.span, "don't parse a statement if you see `}`");
+        } else {
             // Remainder are line-expr stmts. This is similar to the `parse_stmt_path_start` case
             // above.
             let restrictions =
@@ -185,13 +191,10 @@ impl<'a> Parser<'a> {
                     .emit_err(diagnostics::AssignmentElseNotAllowed { span: e.span.to(bl.span) });
             }
             self.mk_stmt(lo.to(e.span), StmtKind::Expr(e))
-        } else {
-            self.error_outer_attrs(attrs);
-            return Ok(None);
         };
 
         self.maybe_augment_stashed_expr_in_pats_with_suggestions(&stmt);
-        Ok(Some(stmt))
+        Ok(stmt)
     }
 
     fn parse_stmt_path_start(&mut self, lo: Span, attrs: AttrWrapper) -> PResult<'a, Stmt> {
@@ -272,20 +275,21 @@ impl<'a> Parser<'a> {
 
     /// Error on outer attributes in this context.
     /// Also error if the previous token was a doc comment.
-    fn error_outer_attrs(&self, attrs: AttrWrapper) {
-        if !attrs.is_empty()
-            && let attrs @ [.., last] = &*attrs.take_for_recovery(self.psess)
-        {
-            if last.is_doc_comment() {
-                self.dcx().emit_err(diagnostics::DocCommentDoesNotDocumentAnything {
-                    span: last.span,
-                    missing_comma: None,
-                });
-            } else if attrs.iter().any(|a| a.style == AttrStyle::Outer) {
-                self.dcx()
-                    .emit_err(diagnostics::ExpectedStatementAfterOuterAttr { span: last.span });
-            }
+    fn error_outer_attrs(&self, attrs: AttrWrapper) -> PResult<'a, ()> {
+        if attrs.is_empty() {
+            return Ok(());
         }
+        let attrs = attrs.take_for_recovery(self.psess);
+        let last = attrs.last().unwrap();
+        Err(if last.is_doc_comment() {
+            self.dcx().create_err(diagnostics::DocCommentDoesNotDocumentAnything {
+                span: last.span,
+                missing_comma: None,
+            })
+        } else {
+            assert_eq!(last.style, AttrStyle::Outer);
+            self.dcx().create_err(diagnostics::ExpectedStatementAfterOuterAttr { span: last.span })
+        })
     }
 
     fn recover_stmt_local_after_let(
@@ -521,6 +525,10 @@ impl<'a> Parser<'a> {
         let sp = self.token.span;
         let mut err = self.dcx().struct_span_err(sp, msg);
         self.label_expected_raw_ref(&mut err);
+        err.span_label(sp, "expected `{`");
+        if self.token == token::CloseBrace {
+            return err;
+        }
 
         let do_not_suggest_help = self.token.is_keyword(kw::In)
             || self.token == token::Colon
@@ -547,13 +555,13 @@ impl<'a> Parser<'a> {
             // since we want to protect against:
             //     `if 1 1 + 1 {` being suggested as  `if { 1 } 1 + 1 {`
             //                                            +   +
-            Ok(Some(_))
+            Ok(_)
                 if (!self.token.is_keyword(kw::Else)
                     && self.look_ahead(1, |t| t == &token::OpenBrace))
                     || do_not_suggest_help => {}
             // Do not suggest `if foo println!("") {;}` (as would be seen in test for #46836).
-            Ok(Some(Stmt { kind: StmtKind::Empty, .. })) => {}
-            Ok(Some(stmt)) => {
+            Ok(Stmt { kind: StmtKind::Empty, .. }) => {}
+            Ok(stmt) => {
                 let stmt_own_line = self.psess.source_map().is_line_before_span_empty(sp);
                 let stmt_span = if stmt_own_line && self.eat(exp!(Semi)) {
                     // Expand the span to include the semicolon.
@@ -571,9 +579,7 @@ impl<'a> Parser<'a> {
             Err(e) => {
                 e.delay_as_bug();
             }
-            _ => {}
         }
-        err.span_label(sp, "expected `{`");
         err
     }
 
@@ -773,17 +779,12 @@ impl<'a> Parser<'a> {
 
                     let guar = err.emit();
                     self.recover_stmt_(SemiColonMode::Ignore, BlockMode::Ignore);
-                    Some(self.mk_stmt_err(self.token.span, guar))
+                    self.mk_stmt_err(self.token.span, guar)
                 }
                 Ok(stmt) => stmt,
                 Err(err) => return Err(err),
             };
-            if let Some(stmt) = stmt {
-                stmts.push(stmt);
-            } else {
-                // Found only `;` or `}`.
-                continue;
-            };
+            stmts.push(stmt);
         }
         Ok(self.mk_block(stmts, s, lo.to(self.prev_token.span)))
     }
@@ -943,10 +944,7 @@ impl<'a> Parser<'a> {
     }
 
     /// Parses a statement, including the trailing semicolon.
-    pub fn parse_full_stmt(
-        &mut self,
-        recover: AttemptLocalParseRecovery,
-    ) -> PResult<'a, Option<Stmt>> {
+    pub fn parse_full_stmt(&mut self, recover: AttemptLocalParseRecovery) -> PResult<'a, Stmt> {
         // Skip looking for a trailing semicolon when we have a metavar seq.
         if let Some(stmt) = self.eat_metavar_seq(MetaVarKind::Stmt, |this| {
             // Why pass `true` for `force_full_expr`? Statement expressions are less expressive
@@ -959,14 +957,10 @@ impl<'a> Parser<'a> {
             // will reparse successfully.
             this.parse_stmt_without_recovery(false, ForceCollect::No, true)
         }) {
-            let stmt = stmt.expect("an actual statement");
-            return Ok(Some(stmt));
+            return Ok(stmt);
         }
 
-        let Some(mut stmt) = self.parse_stmt_without_recovery(true, ForceCollect::No, false)?
-        else {
-            return Ok(None);
-        };
+        let mut stmt = self.parse_stmt_without_recovery(true, ForceCollect::No, false)?;
 
         let mut eat_semi = true;
         let mut add_semi_to_stmt = false;
@@ -1086,7 +1080,7 @@ impl<'a> Parser<'a> {
             StmtKind::Expr(_) | StmtKind::MacCall(_) => {}
             StmtKind::Let(local) => {
                 if self.try_recover_let_missing_semi(local).is_some() {
-                    return Ok(Some(stmt));
+                    return Ok(stmt);
                 }
                 if let Err(mut e) = self.expect_semi() {
                     // We might be at the `,` in `let x = foo<bar, baz>;`. Try to recover.
@@ -1159,7 +1153,7 @@ impl<'a> Parser<'a> {
         }
 
         stmt.span = stmt.span.to(self.prev_token.span);
-        Ok(Some(stmt))
+        Ok(stmt)
     }
 
     pub(super) fn mk_block(