From 2f305ff460862e18b67db2faa11b85ab223d4e60 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 16 Jul 2024 15:02:19 +1000 Subject: [PATCH 1/7] Remove an unnecessary `?`. --- compiler/rustc_parse/src/parser/item.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index d2fea5583b8..994e7c5d2db 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -128,14 +128,10 @@ pub(super) fn parse_item_common( 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)) - })?; - - Ok(item) + self.collect_tokens_trailing_token(attrs, force_collect, |this, attrs| { + let item = this.parse_item_common_(attrs, mac_allowed, attrs_allowed, fn_parse_mode)?; + Ok((item, TrailingToken::None)) + }) } fn parse_item_common_( From 48cdfc388d67a6bde420d8d44eddf8ef1e5b7eb2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 16 Jul 2024 15:07:42 +1000 Subject: [PATCH 2/7] Inline `Parser::parse_item_common_`. It has a single call site, it isn't that big, and its name is confusingly similar to `Parser::parse_item_common`. --- compiler/rustc_parse/src/parser/item.rs | 79 +++++++++++-------------- 1 file changed, 34 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 994e7c5d2db..2d5781c3bbf 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -128,54 +128,43 @@ pub(super) fn parse_item_common( Some(item.into_inner()) }); - self.collect_tokens_trailing_token(attrs, force_collect, |this, 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)); + } + + // 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 }); + } + + if let Defaultness::Default(span) = def { + this.dcx().emit_err(errors::DefaultNotFollowedByItem { span }); + } + + if !attrs_allowed { + this.recover_attrs_no_item(&attrs)?; + } + Ok((None, TrailingToken::None)) }) } - fn parse_item_common_( - &mut self, - mut attrs: AttrVec, - mac_allowed: bool, - attrs_allowed: bool, - fn_parse_mode: FnParseMode, - ) -> PResult<'a, Option> { - 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)); - } - - // 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) - } - /// Error in-case `default` was parsed in an in-appropriate context. fn error_on_unconsumed_default(&self, def: Defaultness, kind: &ItemKind) { if let Defaultness::Default(span) = def { From d247489ac280147b48c72ed739ea9056e0ca6ff2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 16 Jul 2024 15:54:34 +1000 Subject: [PATCH 3/7] Reorder `Parser::parse_expr_dot_or_call_with` arguments. Put `attrs` before `e0` because that matches the order in the source code, where outer attributes appear before expressions. --- compiler/rustc_parse/src/parser/expr.rs | 4 ++-- compiler/rustc_parse/src/parser/pat.rs | 2 +- compiler/rustc_parse/src/parser/stmt.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index b2df9a14eb0..a1bb047e464 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -885,15 +885,15 @@ fn parse_expr_dot_or_call(&mut self, attrs: AttrWrapper) -> PResult<'a, P> 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, + mut attrs: ast::AttrVec, e0: P, lo: Span, - mut attrs: ast::AttrVec, ) -> PResult<'a, P> { // Stitch the list of outer attributes onto the return value. // A little bit ugly, but the best way given the current code diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index 6f2b7177159..7ef6474c29e 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -388,9 +388,9 @@ fn maybe_recover_trailing_expr( // 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()) { diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index d65f6ff68ee..1259d223af6 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -164,7 +164,7 @@ fn parse_stmt_path_start(&mut self, lo: Span, attrs: AttrWrapper) -> PResult<'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 @@ fn parse_stmt_mac(&mut self, lo: Span, attrs: AttrVec, path: ast::Path) -> PResu // 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) From 96cc9c99b29c373db41093dee3170f36dc15543b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 16 Jul 2024 16:02:45 +1000 Subject: [PATCH 4/7] Inline and remove `Parser::parse_and_disallow_postfix_after_cast`. It has a single call site. Removing it removes the need for an `ExprKind` check. The commit also clarifies the relevant comment. --- compiler/rustc_parse/src/parser/expr.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index a1bb047e464..49a71b97be2 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -785,19 +785,10 @@ fn parse_assoc_op_cast( } }; - 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, - ) -> PResult<'a, P> { - 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; From 96b39f1204992368f059b44b56362a91e1e27ef9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 16 Jul 2024 16:12:58 +1000 Subject: [PATCH 5/7] Inline and remove `Parser::parse_expr_dot_or_call_with_`. It only has two call sites, and it extremely similar to `Parser::parse_expr_dot_or_call_with`, in both name and behaviour. The only difference is the latter has an `attrs` argument and an `ensure_sufficient_stack` call. We can pass in an empty `attrs` as necessary, as is already done at some `parse_expr_dot_or_call_with` call sites. --- compiler/rustc_parse/src/parser/expr.rs | 102 ++++++++++++------------ 1 file changed, 49 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 49a71b97be2..896f84d3a36 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -792,7 +792,7 @@ fn parse_assoc_op_cast( 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. @@ -883,16 +883,56 @@ fn parse_expr_dot_or_call(&mut self, attrs: AttrWrapper) -> PResult<'a, P> pub(super) fn parse_expr_dot_or_call_with( &mut self, mut attrs: ast::AttrVec, - e0: P, + mut e: P, lo: Span, ) -> PResult<'a, P> { - // 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 { @@ -906,50 +946,6 @@ pub(super) fn parse_expr_dot_or_call_with( } } - fn parse_expr_dot_or_call_with_(&mut self, mut e: P, lo: Span) -> PResult<'a, P> { - 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, From 8cb6bc3b5a33cbd7d9b131fad2f63cc243b26ea0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 16 Jul 2024 16:39:04 +1000 Subject: [PATCH 6/7] Fix a comment. --- compiler/rustc_parse/src/parser/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 896f84d3a36..796a279188d 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1375,7 +1375,7 @@ fn parse_dot_suffix(&mut self, self_arg: P, lo: Span) -> PResult<'a, P PResult<'a, P> { maybe_recover_from_interpolated_ty_qpath!(self, true); From 9c4f3dbd066fd8e35f188388f66493522f7d4476 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 16 Jul 2024 16:40:35 +1000 Subject: [PATCH 7/7] Remove references to `maybe_whole_expr`. It was removed in #126571. --- compiler/rustc_ast/src/token.rs | 3 +-- compiler/rustc_parse/src/parser/mod.rs | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_ast/src/token.rs b/compiler/rustc_ast/src/token.rs index efe19566152..9478da236c3 100644 --- a/compiler/rustc_ast/src/token.rs +++ b/compiler/rustc_ast/src/token.rs @@ -699,8 +699,7 @@ fn is_whole_path(&self) -> bool { 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 diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 958458eda9a..0117f993bcb 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/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) => {