From 219389360c0b20cd6b980072e08723b91fea2c9b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Jun 2024 21:32:50 +1000 Subject: [PATCH 01/11] Add a comment. Something that was non-obvious to me. --- compiler/rustc_ast/src/ast.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 9cb193b4a6781..ca7f0363fa7ce 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -972,7 +972,9 @@ impl UnOp { } } -/// A statement +/// A statement. No `attrs` or `tokens` fields because each `StmtKind` variant +/// contains an AST node with those fields. (Except for `StmtKind::Empty`, +/// which never has attrs or tokens) #[derive(Clone, Encodable, Decodable, Debug)] pub struct Stmt { pub id: NodeId, From 1fbb3eca67032721e6f286e2a6f20da1b5659cc5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Jun 2024 10:12:37 +1000 Subject: [PATCH 02/11] Expand another comment. --- compiler/rustc_parse/src/parser/attr_wrapper.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_parse/src/parser/attr_wrapper.rs b/compiler/rustc_parse/src/parser/attr_wrapper.rs index 62c8f9f5dacb1..042ee21e3f7c0 100644 --- a/compiler/rustc_parse/src/parser/attr_wrapper.rs +++ b/compiler/rustc_parse/src/parser/attr_wrapper.rs @@ -15,7 +15,7 @@ use std::ops::Range; /// for the attribute target. This allows us to perform cfg-expansion on /// a token stream before we invoke a derive proc-macro. /// -/// This wrapper prevents direct access to the underlying `ast::AttrVec>`. +/// This wrapper prevents direct access to the underlying `ast::AttrVec`. /// Parsing code can only get access to the underlying attributes /// by passing an `AttrWrapper` to `collect_tokens_trailing_tokens`. /// This makes it difficult to accidentally construct an AST node @@ -177,6 +177,10 @@ impl<'a> Parser<'a> { /// into a `LazyAttrTokenStream`, and returned along with the result /// of the callback. /// + /// The `attrs` passed in are in `AttrWrapper` form, which is opaque. The + /// `AttrVec` within is passed to `f`. See the comment on `AttrWrapper` for + /// details. + /// /// Note: If your callback consumes an opening delimiter /// (including the case where you call `collect_tokens` /// when the current token is an opening delimiter), From 1c28229ada005ed899f8ce71315baaa5420e1ed6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Jun 2024 20:00:45 +1000 Subject: [PATCH 03/11] Simplify `Parser::parse_expr_dot_or_call`. The call in `parse_expr_prefix` for the `++` case passes an empty `attrs`, but it doesn' need to. This commit changes it to pass the parsed `attrs`, which doesn't change any behaviour. As a result, `parse_expr_dot_or_call` no longer needs an `Option` argument, and no longer needs to call `parse_or_use_outer_attributes`. --- compiler/rustc_parse/src/parser/expr.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index e15d6ab2123a5..cdeb291189944 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -629,7 +629,7 @@ impl<'a> Parser<'a> { this.bump(); this.bump(); - let operand_expr = this.parse_expr_dot_or_call(Default::default())?; + let operand_expr = this.parse_expr_dot_or_call(attrs)?; this.recover_from_prefix_increment(operand_expr, pre_span, starts_stmt) } token::Ident(..) if this.token.is_keyword(kw::Box) => { @@ -638,7 +638,7 @@ impl<'a> Parser<'a> { token::Ident(..) if this.may_recover() && this.is_mistaken_not_ident_negation() => { make_it!(this, attrs, |this, _| this.recover_not_expr(lo)) } - _ => return this.parse_expr_dot_or_call(Some(attrs)), + _ => return this.parse_expr_dot_or_call(attrs), } } @@ -927,8 +927,7 @@ impl<'a> Parser<'a> { } /// Parses `a.b` or `a(13)` or `a[4]` or just `a`. - fn parse_expr_dot_or_call(&mut self, attrs: Option) -> PResult<'a, P> { - let attrs = self.parse_or_use_outer_attributes(attrs)?; + 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); From 42e47dfe8251c8356c409485d66164b6fceb03c6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Jun 2024 20:21:05 +1000 Subject: [PATCH 04/11] Remove `From` impls for `LhsExpr`. The `Option` one maps to the first two variants, and the `P` one maps to the third. Weird. The code is shorter and clearer without them. --- compiler/rustc_parse/src/parser/expr.rs | 28 +++++++------------------ compiler/rustc_parse/src/parser/pat.rs | 7 +++---- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index cdeb291189944..a0f57ffecfa62 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -75,25 +75,6 @@ pub(super) enum LhsExpr { AlreadyParsed { expr: P, starts_statement: bool }, } -impl From> for LhsExpr { - /// Converts `Some(attrs)` into `LhsExpr::AttributesParsed(attrs)` - /// and `None` into `LhsExpr::NotYetParsed`. - /// - /// This conversion does not allocate. - fn from(o: Option) -> Self { - if let Some(attrs) = o { LhsExpr::AttributesParsed(attrs) } else { LhsExpr::NotYetParsed } - } -} - -impl From> for LhsExpr { - /// Converts the `expr: P` into `LhsExpr::AlreadyParsed { expr, starts_statement: false }`. - /// - /// This conversion does not allocate. - fn from(expr: P) -> Self { - LhsExpr::AlreadyParsed { expr, starts_statement: false } - } -} - #[derive(Debug)] enum DestructuredFloat { /// 1e2 @@ -166,7 +147,11 @@ impl<'a> Parser<'a> { &mut self, already_parsed_attrs: Option, ) -> PResult<'a, P> { - self.parse_expr_assoc_with(0, already_parsed_attrs.into()) + let lhs = match already_parsed_attrs { + Some(attrs) => LhsExpr::AttributesParsed(attrs), + None => LhsExpr::NotYetParsed, + }; + self.parse_expr_assoc_with(0, lhs) } /// Parses an associative expression with operators of at least `min_prec` precedence. @@ -2660,7 +2645,8 @@ impl<'a> Parser<'a> { } else { self.expect(&token::Eq)?; } - let expr = self.parse_expr_assoc_with(1 + prec_let_scrutinee_needs_par(), None.into())?; + let expr = + self.parse_expr_assoc_with(1 + prec_let_scrutinee_needs_par(), LhsExpr::NotYetParsed)?; let span = lo.to(expr.span); Ok(self.mk_expr(span, ExprKind::Let(pat, expr, span, recovered))) } diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index 8af415f7c9dd3..a3efecffcc652 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -10,7 +10,7 @@ use crate::errors::{ UnexpectedParenInRangePatSugg, UnexpectedVertVertBeforeFunctionParam, UnexpectedVertVertInPattern, }; -use crate::parser::expr::could_be_unclosed_char_literal; +use crate::parser::expr::{could_be_unclosed_char_literal, LhsExpr}; use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole}; use rustc_ast::mut_visit::{noop_visit_pat, MutVisitor}; use rustc_ast::ptr::P; @@ -398,9 +398,8 @@ impl<'a> Parser<'a> { // Parse an associative expression such as `+ expr`, `% expr`, ... // Assignements, ranges and `|` are disabled by [`Restrictions::IS_PAT`]. - if let Ok(expr) = - snapshot.parse_expr_assoc_with(0, expr.into()).map_err(|err| err.cancel()) - { + let lhs = LhsExpr::AlreadyParsed { expr, starts_statement: false }; + if let Ok(expr) = snapshot.parse_expr_assoc_with(0, lhs).map_err(|err| err.cancel()) { // We got a valid expression. self.restore_snapshot(snapshot); self.restrictions.remove(Restrictions::IS_PAT); From 25523ba382037494e8f8f17779a5f1c72f6bddb2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 18 Jun 2024 20:40:27 +1000 Subject: [PATCH 05/11] Refactor `LhsExpr`. Combine `NotYetParsed` and `AttributesParsed` into a single variant, because (a) that reflects the structure of the code that consumes `LhsExpr`, and (b) because that variant will have the `Option` removed in a later commit. --- compiler/rustc_parse/src/parser/expr.rs | 67 +++++++++++++------------ compiler/rustc_parse/src/parser/pat.rs | 2 +- compiler/rustc_parse/src/parser/stmt.rs | 11 ++-- 3 files changed, 38 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index a0f57ffecfa62..eb3421cfee4af 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -70,9 +70,10 @@ macro_rules! maybe_whole_expr { #[derive(Debug)] pub(super) enum LhsExpr { - NotYetParsed, - AttributesParsed(AttrWrapper), - AlreadyParsed { expr: P, starts_statement: bool }, + // Already parsed either (a) nothing or (b) just the outer attributes. + Unparsed { attrs: Option }, + // Already parsed the expression. + Parsed { expr: P, starts_statement: bool }, } #[derive(Debug)] @@ -133,9 +134,9 @@ impl<'a> Parser<'a> { pub(super) fn parse_expr_res( &mut self, r: Restrictions, - already_parsed_attrs: Option, + attrs: Option, ) -> PResult<'a, P> { - self.with_res(r, |this| this.parse_expr_assoc(already_parsed_attrs)) + self.with_res(r, |this| this.parse_expr_assoc(attrs)) } /// Parses an associative expression. @@ -143,15 +144,8 @@ impl<'a> Parser<'a> { /// This parses an expression accounting for associativity and precedence of the operators in /// the expression. #[inline] - fn parse_expr_assoc( - &mut self, - already_parsed_attrs: Option, - ) -> PResult<'a, P> { - let lhs = match already_parsed_attrs { - Some(attrs) => LhsExpr::AttributesParsed(attrs), - None => LhsExpr::NotYetParsed, - }; - self.parse_expr_assoc_with(0, lhs) + fn parse_expr_assoc(&mut self, attrs: Option) -> PResult<'a, P> { + self.parse_expr_assoc_with(0, LhsExpr::Unparsed { attrs }) } /// Parses an associative expression with operators of at least `min_prec` precedence. @@ -161,18 +155,17 @@ impl<'a> Parser<'a> { lhs: LhsExpr, ) -> PResult<'a, P> { let mut starts_stmt = false; - let mut lhs = if let LhsExpr::AlreadyParsed { expr, starts_statement } = lhs { - starts_stmt = starts_statement; - expr - } else { - let attrs = match lhs { - LhsExpr::AttributesParsed(attrs) => Some(attrs), - _ => None, - }; - if self.token.is_range_separator() { - return self.parse_expr_prefix_range(attrs); - } else { - self.parse_expr_prefix(attrs)? + let mut lhs = match lhs { + LhsExpr::Parsed { expr, starts_statement } => { + starts_stmt = starts_statement; + expr + } + LhsExpr::Unparsed { attrs } => { + if self.token.is_range_separator() { + return self.parse_expr_prefix_range(attrs); + } else { + self.parse_expr_prefix(attrs)? + } } }; @@ -310,7 +303,10 @@ impl<'a> Parser<'a> { Fixity::None => 1, }; let rhs = self.with_res(restrictions - Restrictions::STMT_EXPR, |this| { - this.parse_expr_assoc_with(prec + prec_adjustment, LhsExpr::NotYetParsed) + this.parse_expr_assoc_with( + prec + prec_adjustment, + LhsExpr::Unparsed { attrs: None }, + ) })?; let span = self.mk_expr_sp(&lhs, lhs_span, rhs.span); @@ -484,7 +480,7 @@ impl<'a> Parser<'a> { let rhs = if self.is_at_start_of_range_notation_rhs() { let maybe_lt = self.token.clone(); Some( - self.parse_expr_assoc_with(prec + 1, LhsExpr::NotYetParsed) + self.parse_expr_assoc_with(prec + 1, LhsExpr::Unparsed { attrs: None }) .map_err(|err| self.maybe_err_dotdotlt_syntax(maybe_lt, err))?, ) } else { @@ -539,9 +535,12 @@ impl<'a> Parser<'a> { this.bump(); let (span, opt_end) = if this.is_at_start_of_range_notation_rhs() { // RHS must be parsed with more associativity than the dots. - this.parse_expr_assoc_with(op.unwrap().precedence() + 1, LhsExpr::NotYetParsed) - .map(|x| (lo.to(x.span), Some(x))) - .map_err(|err| this.maybe_err_dotdotlt_syntax(maybe_lt, err))? + this.parse_expr_assoc_with( + op.unwrap().precedence() + 1, + LhsExpr::Unparsed { attrs: None }, + ) + .map(|x| (lo.to(x.span), Some(x))) + .map_err(|err| this.maybe_err_dotdotlt_syntax(maybe_lt, err))? } else { (lo, None) }; @@ -2645,8 +2644,10 @@ impl<'a> Parser<'a> { } else { self.expect(&token::Eq)?; } - let expr = - self.parse_expr_assoc_with(1 + prec_let_scrutinee_needs_par(), LhsExpr::NotYetParsed)?; + let expr = self.parse_expr_assoc_with( + 1 + prec_let_scrutinee_needs_par(), + LhsExpr::Unparsed { attrs: None }, + )?; let span = lo.to(expr.span); Ok(self.mk_expr(span, ExprKind::Let(pat, expr, span, recovered))) } diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index a3efecffcc652..921e71a1f9643 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -398,7 +398,7 @@ impl<'a> Parser<'a> { // Parse an associative expression such as `+ expr`, `% expr`, ... // Assignements, ranges and `|` are disabled by [`Restrictions::IS_PAT`]. - let lhs = LhsExpr::AlreadyParsed { expr, starts_statement: false }; + let lhs = LhsExpr::Parsed { expr, starts_statement: false }; if let Ok(expr) = snapshot.parse_expr_assoc_with(0, lhs).map_err(|err| err.cancel()) { // We got a valid expression. self.restore_snapshot(snapshot); diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 104aae9b257e2..8684f2ed829d0 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -174,10 +174,7 @@ impl<'a> Parser<'a> { // Perform this outside of the `collect_tokens_trailing_token` closure, // since our outer attributes do not apply to this part of the expression let expr = self.with_res(Restrictions::STMT_EXPR, |this| { - this.parse_expr_assoc_with( - 0, - LhsExpr::AlreadyParsed { expr, starts_statement: true }, - ) + this.parse_expr_assoc_with(0, LhsExpr::Parsed { expr, starts_statement: true }) })?; Ok(self.mk_stmt(lo.to(self.prev_token.span), StmtKind::Expr(expr))) } else { @@ -210,10 +207,8 @@ impl<'a> Parser<'a> { 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_assoc_with( - 0, - LhsExpr::AlreadyParsed { expr: e, starts_statement: false }, - )?; + let e = self + .parse_expr_assoc_with(0, LhsExpr::Parsed { expr: e, starts_statement: false })?; StmtKind::Expr(e) }; Ok(self.mk_stmt(lo.to(hi), kind)) From ead0a45202af9cefb9607a621d27da4927700229 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Jun 2024 10:34:05 +1000 Subject: [PATCH 06/11] Inline and remove `parse_expr_assoc`. It has a single call site. --- compiler/rustc_parse/src/parser/expr.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index eb3421cfee4af..272067dca8726 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -136,16 +136,7 @@ impl<'a> Parser<'a> { r: Restrictions, attrs: Option, ) -> PResult<'a, P> { - self.with_res(r, |this| this.parse_expr_assoc(attrs)) - } - - /// Parses an associative expression. - /// - /// This parses an expression accounting for associativity and precedence of the operators in - /// the expression. - #[inline] - fn parse_expr_assoc(&mut self, attrs: Option) -> PResult<'a, P> { - self.parse_expr_assoc_with(0, LhsExpr::Unparsed { attrs }) + self.with_res(r, |this| this.parse_expr_assoc_with(0, LhsExpr::Unparsed { attrs })) } /// Parses an associative expression with operators of at least `min_prec` precedence. From 802779f77ddaac18865e5e52e01c5e4d122e9090 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Jun 2024 14:41:01 +1000 Subject: [PATCH 07/11] Move `parse_or_use_outer_attributes` out of `parse_expr_prefix`. This eliminates one `Option` argument. --- compiler/rustc_parse/src/parser/expr.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 272067dca8726..e96b97da9fdf2 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -155,6 +155,7 @@ impl<'a> Parser<'a> { if self.token.is_range_separator() { return self.parse_expr_prefix_range(attrs); } else { + let attrs = self.parse_or_use_outer_attributes(attrs)?; self.parse_expr_prefix(attrs)? } } @@ -541,8 +542,7 @@ impl<'a> Parser<'a> { } /// Parses a prefix-unary-operator expr. - fn parse_expr_prefix(&mut self, attrs: Option) -> PResult<'a, P> { - let attrs = self.parse_or_use_outer_attributes(attrs)?; + fn parse_expr_prefix(&mut self, attrs: AttrWrapper) -> PResult<'a, P> { let lo = self.token.span; macro_rules! make_it { @@ -591,7 +591,8 @@ impl<'a> Parser<'a> { this.dcx().emit_err(err); this.bump(); - this.parse_expr_prefix(None) + let attrs = this.parse_outer_attributes()?; + this.parse_expr_prefix(attrs) } // Recover from `++x`: token::BinOp(token::Plus) @@ -619,7 +620,8 @@ impl<'a> Parser<'a> { fn parse_expr_prefix_common(&mut self, lo: Span) -> PResult<'a, (Span, P)> { self.bump(); - let expr = self.parse_expr_prefix(None)?; + let attrs = self.parse_outer_attributes()?; + let expr = self.parse_expr_prefix(attrs)?; let span = self.interpolated_or_expr_span(&expr); Ok((lo.to(span), expr)) } @@ -872,7 +874,8 @@ impl<'a> Parser<'a> { let expr = if self.token.is_range_separator() { self.parse_expr_prefix_range(None) } else { - self.parse_expr_prefix(None) + let attrs = self.parse_outer_attributes()?; + self.parse_expr_prefix(attrs) }?; let hi = self.interpolated_or_expr_span(&expr); let span = lo.to(hi); From aaa220e8757d1d2bb3a7b4742db9e289c8454dc2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Jun 2024 15:37:31 +1000 Subject: [PATCH 08/11] Move `parse_or_use_outer_attributes` out of `parse_expr_prefix_range`. This eliminates another `Option` argument and changes one obscure error message. --- compiler/rustc_parse/messages.ftl | 2 ++ compiler/rustc_parse/src/errors.rs | 7 +++++++ compiler/rustc_parse/src/parser/expr.rs | 19 ++++++++++--------- .../attribute/attr-stmt-expr-attr-bad.rs | 4 ++-- .../attribute/attr-stmt-expr-attr-bad.stderr | 8 ++++---- 5 files changed, 25 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index f678d11213c71..efb9526eabcfd 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -133,6 +133,8 @@ parse_dot_dot_dot_for_remaining_fields = expected field pattern, found `{$token_ parse_dot_dot_dot_range_to_pattern_not_allowed = range-to patterns with `...` are not allowed .suggestion = use `..=` instead +parse_dot_dot_range_attribute = attributes are not allowed on range expressions starting with `..` + parse_dotdotdot = unexpected token: `...` .suggest_exclusive_range = use `..` for an exclusive range .suggest_inclusive_range = or `..=` for an inclusive range diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 6c1fcbe06fc5e..f0ef1112f9e3f 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -2989,3 +2989,10 @@ pub(crate) struct ExprRArrowCall { #[suggestion(style = "short", applicability = "machine-applicable", code = ".")] pub span: Span, } + +#[derive(Diagnostic)] +#[diag(parse_dot_dot_range_attribute)] +pub(crate) struct DotDotRangeAttribute { + #[primary_span] + pub span: Span, +} diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index e96b97da9fdf2..ab3f8a5dbeb86 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -152,10 +152,10 @@ impl<'a> Parser<'a> { expr } LhsExpr::Unparsed { attrs } => { + let attrs = self.parse_or_use_outer_attributes(attrs)?; if self.token.is_range_separator() { return self.parse_expr_prefix_range(attrs); } else { - let attrs = self.parse_or_use_outer_attributes(attrs)?; self.parse_expr_prefix(attrs)? } } @@ -499,7 +499,12 @@ impl<'a> Parser<'a> { } /// Parses prefix-forms of range notation: `..expr`, `..`, `..=expr`. - fn parse_expr_prefix_range(&mut self, attrs: Option) -> PResult<'a, P> { + fn parse_expr_prefix_range(&mut self, attrs: AttrWrapper) -> PResult<'a, P> { + if !attrs.is_empty() { + let err = errors::DotDotRangeAttribute { span: self.token.span }; + self.dcx().emit_err(err); + } + // Check for deprecated `...` syntax. if self.token == token::DotDotDot { self.err_dotdotdot_syntax(self.token.span); @@ -516,11 +521,7 @@ impl<'a> Parser<'a> { _ => RangeLimits::Closed, }; let op = AssocOp::from_token(&self.token); - // FIXME: `parse_prefix_range_expr` is called when the current - // token is `DotDot`, `DotDotDot`, or `DotDotEq`. If we haven't already - // parsed attributes, then trying to parse them here will always fail. - // We should figure out how we want attributes on range expressions to work. - let attrs = self.parse_or_use_outer_attributes(attrs)?; + let attrs = self.parse_outer_attributes()?; self.collect_tokens_for_expr(attrs, |this, attrs| { let lo = this.token.span; let maybe_lt = this.look_ahead(1, |t| t.clone()); @@ -871,10 +872,10 @@ impl<'a> Parser<'a> { let has_lifetime = self.token.is_lifetime() && self.look_ahead(1, |t| t != &token::Colon); let lifetime = has_lifetime.then(|| self.expect_lifetime()); // For recovery, see below. let (borrow_kind, mutbl) = self.parse_borrow_modifiers(lo); + let attrs = self.parse_outer_attributes()?; let expr = if self.token.is_range_separator() { - self.parse_expr_prefix_range(None) + self.parse_expr_prefix_range(attrs) } else { - let attrs = self.parse_outer_attributes()?; self.parse_expr_prefix(attrs) }?; let hi = self.interpolated_or_expr_span(&expr); diff --git a/tests/ui/parser/attribute/attr-stmt-expr-attr-bad.rs b/tests/ui/parser/attribute/attr-stmt-expr-attr-bad.rs index d1950087c4c2d..26761a1d2544c 100644 --- a/tests/ui/parser/attribute/attr-stmt-expr-attr-bad.rs +++ b/tests/ui/parser/attribute/attr-stmt-expr-attr-bad.rs @@ -28,9 +28,9 @@ fn main() {} #[cfg(FALSE)] fn e() { let _ = move || #![attr] {foo}; } //~^ ERROR an inner attribute is not permitted in this context #[cfg(FALSE)] fn e() { let _ = #[attr] ..#[attr] 0; } -//~^ ERROR expected expression, found `..` +//~^ ERROR attributes are not allowed on range expressions starting with `..` #[cfg(FALSE)] fn e() { let _ = #[attr] ..; } -//~^ ERROR expected expression, found `..` +//~^ ERROR attributes are not allowed on range expressions starting with `..` #[cfg(FALSE)] fn e() { let _ = #[attr] &#![attr] 0; } //~^ ERROR an inner attribute is not permitted in this context #[cfg(FALSE)] fn e() { let _ = #[attr] &mut #![attr] 0; } diff --git a/tests/ui/parser/attribute/attr-stmt-expr-attr-bad.stderr b/tests/ui/parser/attribute/attr-stmt-expr-attr-bad.stderr index e46c591080d43..3593c5182ce9f 100644 --- a/tests/ui/parser/attribute/attr-stmt-expr-attr-bad.stderr +++ b/tests/ui/parser/attribute/attr-stmt-expr-attr-bad.stderr @@ -119,17 +119,17 @@ LL | #[cfg(FALSE)] fn e() { let _ = move || #![attr] {foo}; } = note: inner attributes, like `#![no_std]`, annotate the item enclosing them, and are usually found at the beginning of source files = note: outer attributes, like `#[test]`, annotate the item following them -error: expected expression, found `..` +error: attributes are not allowed on range expressions starting with `..` --> $DIR/attr-stmt-expr-attr-bad.rs:30:40 | LL | #[cfg(FALSE)] fn e() { let _ = #[attr] ..#[attr] 0; } - | ^^ expected expression + | ^^ -error: expected expression, found `..` +error: attributes are not allowed on range expressions starting with `..` --> $DIR/attr-stmt-expr-attr-bad.rs:32:40 | LL | #[cfg(FALSE)] fn e() { let _ = #[attr] ..; } - | ^^ expected expression + | ^^ error: an inner attribute is not permitted in this context --> $DIR/attr-stmt-expr-attr-bad.rs:34:41 From 43eae4cef4f4e519641e0f0a3915b8023db92f00 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Jun 2024 16:06:16 +1000 Subject: [PATCH 09/11] Simplify `LhsExpr::Unparsed`. By making the `AttrWrapper` non-optional. --- compiler/rustc_parse/src/parser/expr.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index ab3f8a5dbeb86..aea04ceca914f 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -70,8 +70,8 @@ macro_rules! maybe_whole_expr { #[derive(Debug)] pub(super) enum LhsExpr { - // Already parsed either (a) nothing or (b) just the outer attributes. - Unparsed { attrs: Option }, + // Already parsed just the outer attributes. + Unparsed { attrs: AttrWrapper }, // Already parsed the expression. Parsed { expr: P, starts_statement: bool }, } @@ -136,6 +136,7 @@ impl<'a> Parser<'a> { r: Restrictions, attrs: Option, ) -> PResult<'a, P> { + let attrs = self.parse_or_use_outer_attributes(attrs)?; self.with_res(r, |this| this.parse_expr_assoc_with(0, LhsExpr::Unparsed { attrs })) } @@ -152,7 +153,6 @@ impl<'a> Parser<'a> { expr } LhsExpr::Unparsed { attrs } => { - let attrs = self.parse_or_use_outer_attributes(attrs)?; if self.token.is_range_separator() { return self.parse_expr_prefix_range(attrs); } else { @@ -295,10 +295,8 @@ impl<'a> Parser<'a> { Fixity::None => 1, }; let rhs = self.with_res(restrictions - Restrictions::STMT_EXPR, |this| { - this.parse_expr_assoc_with( - prec + prec_adjustment, - LhsExpr::Unparsed { attrs: None }, - ) + let attrs = this.parse_outer_attributes()?; + this.parse_expr_assoc_with(prec + prec_adjustment, LhsExpr::Unparsed { attrs }) })?; let span = self.mk_expr_sp(&lhs, lhs_span, rhs.span); @@ -471,8 +469,9 @@ impl<'a> Parser<'a> { ) -> PResult<'a, P> { let rhs = if self.is_at_start_of_range_notation_rhs() { let maybe_lt = self.token.clone(); + let attrs = self.parse_outer_attributes()?; Some( - self.parse_expr_assoc_with(prec + 1, LhsExpr::Unparsed { attrs: None }) + self.parse_expr_assoc_with(prec + 1, LhsExpr::Unparsed { attrs }) .map_err(|err| self.maybe_err_dotdotlt_syntax(maybe_lt, err))?, ) } else { @@ -528,9 +527,10 @@ impl<'a> Parser<'a> { this.bump(); let (span, opt_end) = if this.is_at_start_of_range_notation_rhs() { // RHS must be parsed with more associativity than the dots. + let attrs = this.parse_outer_attributes()?; this.parse_expr_assoc_with( op.unwrap().precedence() + 1, - LhsExpr::Unparsed { attrs: None }, + LhsExpr::Unparsed { attrs }, ) .map(|x| (lo.to(x.span), Some(x))) .map_err(|err| this.maybe_err_dotdotlt_syntax(maybe_lt, err))? @@ -2639,9 +2639,10 @@ impl<'a> Parser<'a> { } else { self.expect(&token::Eq)?; } + let attrs = self.parse_outer_attributes()?; let expr = self.parse_expr_assoc_with( 1 + prec_let_scrutinee_needs_par(), - LhsExpr::Unparsed { attrs: None }, + LhsExpr::Unparsed { attrs }, )?; let span = lo.to(expr.span); Ok(self.mk_expr(span, ExprKind::Let(pat, expr, span, recovered))) From 8170acb197d7658505949cefc12e01dfc4c8feab Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Jun 2024 16:24:21 +1000 Subject: [PATCH 10/11] Refactor `parse_expr_res`. This removes the final `Option` argument. --- .../rustc_parse/src/parser/diagnostics.rs | 13 ++++++-- compiler/rustc_parse/src/parser/expr.rs | 30 ++++++++++++------- compiler/rustc_parse/src/parser/mod.rs | 11 ------- compiler/rustc_parse/src/parser/path.rs | 3 +- compiler/rustc_parse/src/parser/stmt.rs | 4 +-- 5 files changed, 33 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 2bb6fb53869bc..17e62a8e0468c 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -2502,7 +2502,8 @@ impl<'a> Parser<'a> { /// wrapped in braces. pub(super) fn handle_unambiguous_unbraced_const_arg(&mut self) -> PResult<'a, P> { let start = self.token.span; - let expr = self.parse_expr_res(Restrictions::CONST_EXPR, None).map_err(|mut err| { + let attrs = self.parse_outer_attributes()?; + let expr = self.parse_expr_res(Restrictions::CONST_EXPR, attrs).map_err(|mut err| { err.span_label( start.shrink_to_lo(), "while parsing a const generic argument starting here", @@ -2624,7 +2625,10 @@ impl<'a> Parser<'a> { if is_op_or_dot { self.bump(); } - match self.parse_expr_res(Restrictions::CONST_EXPR, None) { + match (|| { + let attrs = self.parse_outer_attributes()?; + self.parse_expr_res(Restrictions::CONST_EXPR, attrs) + })() { Ok(expr) => { // Find a mistake like `MyTrait`. if token::EqEq == snapshot.token.kind { @@ -2678,7 +2682,10 @@ impl<'a> Parser<'a> { &mut self, mut snapshot: SnapshotParser<'a>, ) -> Option> { - match snapshot.parse_expr_res(Restrictions::CONST_EXPR, None) { + match (|| { + let attrs = self.parse_outer_attributes()?; + snapshot.parse_expr_res(Restrictions::CONST_EXPR, attrs) + })() { // Since we don't know the exact reason why we failed to parse the type or the // expression, employ a simple heuristic to weed out some pathological cases. Ok(expr) if let token::Comma | token::Gt = snapshot.token.kind => { diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index aea04ceca914f..d27c612bbc1ec 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -94,7 +94,8 @@ impl<'a> Parser<'a> { pub fn parse_expr(&mut self) -> PResult<'a, P> { self.current_closure.take(); - self.parse_expr_res(Restrictions::empty(), None) + let attrs = self.parse_outer_attributes()?; + self.parse_expr_res(Restrictions::empty(), attrs) } /// Parses an expression, forcing tokens to be collected @@ -107,7 +108,8 @@ impl<'a> Parser<'a> { } fn parse_expr_catch_underscore(&mut self, restrictions: Restrictions) -> PResult<'a, P> { - match self.parse_expr_res(restrictions, None) { + let attrs = self.parse_outer_attributes()?; + match self.parse_expr_res(restrictions, attrs) { Ok(expr) => Ok(expr), Err(err) => match self.token.ident() { Some((Ident { name: kw::Underscore, .. }, IdentIsRaw::No)) @@ -134,9 +136,8 @@ impl<'a> Parser<'a> { pub(super) fn parse_expr_res( &mut self, r: Restrictions, - attrs: Option, + attrs: AttrWrapper, ) -> PResult<'a, P> { - let attrs = self.parse_or_use_outer_attributes(attrs)?; self.with_res(r, |this| this.parse_expr_assoc_with(0, LhsExpr::Unparsed { attrs })) } @@ -2343,7 +2344,8 @@ impl<'a> Parser<'a> { self.restrictions - Restrictions::STMT_EXPR - Restrictions::ALLOW_LET; let prev = self.prev_token.clone(); let token = self.token.clone(); - match self.parse_expr_res(restrictions, None) { + let attrs = self.parse_outer_attributes()?; + match self.parse_expr_res(restrictions, attrs) { Ok(expr) => expr, Err(err) => self.recover_closure_body(err, before, prev, token, lo, decl_hi)?, } @@ -2591,8 +2593,9 @@ impl<'a> Parser<'a> { /// Parses the condition of a `if` or `while` expression. fn parse_expr_cond(&mut self) -> PResult<'a, P> { + let attrs = self.parse_outer_attributes()?; let mut cond = - self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL | Restrictions::ALLOW_LET, None)?; + self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL | Restrictions::ALLOW_LET, attrs)?; CondChecker::new(self).visit_expr(&mut cond); @@ -2776,7 +2779,8 @@ impl<'a> Parser<'a> { (Err(err), Some((start_span, left))) if self.eat_keyword(kw::In) => { // We know for sure we have seen `for ($SOMETHING in`. In the happy path this would // happen right before the return of this method. - let expr = match self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None) { + let attrs = self.parse_outer_attributes()?; + let expr = match self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, attrs) { Ok(expr) => expr, Err(expr_err) => { // We don't know what followed the `in`, so cancel and bubble up the @@ -2810,7 +2814,8 @@ impl<'a> Parser<'a> { self.error_missing_in_for_loop(); } self.check_for_for_in_in_typo(self.prev_token.span); - let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?; + let attrs = self.parse_outer_attributes()?; + let expr = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, attrs)?; Ok((pat, expr)) } @@ -2922,7 +2927,8 @@ impl<'a> Parser<'a> { /// Parses a `match ... { ... }` expression (`match` token already eaten). fn parse_expr_match(&mut self) -> PResult<'a, P> { let match_span = self.prev_token.span; - let scrutinee = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?; + let attrs = self.parse_outer_attributes()?; + let scrutinee = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, attrs)?; self.parse_match_block(match_span, match_span, scrutinee, MatchKind::Prefix) } @@ -3126,8 +3132,9 @@ impl<'a> Parser<'a> { let arrow_span = this.prev_token.span; let arm_start_span = this.token.span; + let attrs = this.parse_outer_attributes()?; let expr = - this.parse_expr_res(Restrictions::STMT_EXPR, None).map_err(|mut err| { + this.parse_expr_res(Restrictions::STMT_EXPR, attrs).map_err(|mut err| { err.span_label(arrow_span, "while parsing the `match` arm starting here"); err })?; @@ -3332,7 +3339,8 @@ impl<'a> Parser<'a> { } fn parse_match_guard_condition(&mut self) -> PResult<'a, P> { - self.parse_expr_res(Restrictions::ALLOW_LET | Restrictions::IN_IF_GUARD, None).map_err( + let attrs = self.parse_outer_attributes()?; + self.parse_expr_res(Restrictions::ALLOW_LET | Restrictions::IN_IF_GUARD, attrs).map_err( |mut err| { if self.prev_token == token::OpenDelim(Delimiter::Brace) { let sugg_sp = self.prev_token.span.shrink_to_lo(); diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index adf04fcf2241d..f4cab480669be 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -1337,17 +1337,6 @@ impl<'a> Parser<'a> { }) } - fn parse_or_use_outer_attributes( - &mut self, - already_parsed_attrs: Option, - ) -> PResult<'a, AttrWrapper> { - if let Some(attrs) = already_parsed_attrs { - Ok(attrs) - } else { - self.parse_outer_attributes() - } - } - /// Parses a single token tree from the input. pub fn parse_token_tree(&mut self) -> TokenTree { match self.token.kind { diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index 9beecd9849fbd..da8d1194325b4 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -920,7 +920,8 @@ impl<'a> Parser<'a> { // Fall back by trying to parse a const-expr expression. If we successfully do so, // then we should report an error that it needs to be wrapped in braces. let snapshot = self.create_snapshot_for_diagnostic(); - match self.parse_expr_res(Restrictions::CONST_EXPR, None) { + let attrs = self.parse_outer_attributes()?; + match self.parse_expr_res(Restrictions::CONST_EXPR, attrs) { Ok(expr) => { return Ok(Some(self.dummy_const_arg_needs_braces( self.dcx().struct_span_err(expr.span, "invalid const generic expression"), diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 8684f2ed829d0..d65f6ff68eeeb 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -126,9 +126,9 @@ impl<'a> Parser<'a> { // Remainder are line-expr stmts. let e = match force_collect { ForceCollect::Yes => self.collect_tokens_no_attrs(|this| { - this.parse_expr_res(Restrictions::STMT_EXPR, Some(attrs)) + this.parse_expr_res(Restrictions::STMT_EXPR, attrs) })?, - ForceCollect::No => self.parse_expr_res(Restrictions::STMT_EXPR, Some(attrs))?, + ForceCollect::No => self.parse_expr_res(Restrictions::STMT_EXPR, attrs)?, }; if matches!(e.kind, ExprKind::Assign(..)) && self.eat_keyword(kw::Else) { let bl = self.parse_block()?; From 64c2e9ed3bfb6d6e8f9463f90d4eefa736242b8e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Jun 2024 17:33:46 +1000 Subject: [PATCH 11/11] Change how `parse_expr_force_collect` works. It now parses outer attributes before collecting tokens. This avoids the problem where the outer attribute tokens were being stored twice -- for the attribute tokesn, and also for the expression tokens. Fixes #86055. --- compiler/rustc_parse/src/parser/expr.rs | 7 +++-- .../expr-stmt-nonterminal-tokens.stdout | 27 +------------------ 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index d27c612bbc1ec..4094fb53659b3 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -98,9 +98,12 @@ impl<'a> Parser<'a> { self.parse_expr_res(Restrictions::empty(), attrs) } - /// Parses an expression, forcing tokens to be collected + /// Parses an expression, forcing tokens to be collected. pub fn parse_expr_force_collect(&mut self) -> PResult<'a, P> { - self.collect_tokens_no_attrs(|this| this.parse_expr()) + self.current_closure.take(); + + let attrs = self.parse_outer_attributes()?; + self.collect_tokens_no_attrs(|this| this.parse_expr_res(Restrictions::empty(), attrs)) } pub fn parse_expr_anon_const(&mut self) -> PResult<'a, AnonConst> { diff --git a/tests/ui/proc-macro/expr-stmt-nonterminal-tokens.stdout b/tests/ui/proc-macro/expr-stmt-nonterminal-tokens.stdout index 6523f2485cd1f..0168689b605ff 100644 --- a/tests/ui/proc-macro/expr-stmt-nonterminal-tokens.stdout +++ b/tests/ui/proc-macro/expr-stmt-nonterminal-tokens.stdout @@ -1,4 +1,4 @@ -PRINT-DERIVE INPUT (DISPLAY): enum E { V = { let _ = #[allow(warnings)] #[allow(warnings)] 0; 0 }, } +PRINT-DERIVE INPUT (DISPLAY): enum E { V = { let _ = #[allow(warnings)] 0; 0 }, } PRINT-DERIVE INPUT (DEBUG): TokenStream [ Ident { ident: "enum", @@ -39,31 +39,6 @@ PRINT-DERIVE INPUT (DEBUG): TokenStream [ Group { delimiter: None, stream: TokenStream [ - Punct { - ch: '#', - spacing: Alone, - span: #0 bytes(543..544), - }, - Group { - delimiter: Bracket, - stream: TokenStream [ - Ident { - ident: "allow", - span: #0 bytes(545..550), - }, - Group { - delimiter: Parenthesis, - stream: TokenStream [ - Ident { - ident: "warnings", - span: #0 bytes(551..559), - }, - ], - span: #0 bytes(550..560), - }, - ], - span: #0 bytes(544..561), - }, Punct { ch: '#', spacing: Alone,