From cbb8714a3f8a04cce698719df338fb095c40f479 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 29 Dec 2023 16:28:47 -0800 Subject: Mark expr_requires_semi_to_be_stmt call sites For each of these, we need to decide whether they need to be using `expr_requires_semi_to_be_stmt`, or `expr_requires_comma_to_be_match_arm`, which are supposed to be 2 different behaviors. Previously they were conflated into one, causing either too much or too little parenthesization. --- compiler/rustc_parse/src/parser/expr.rs | 6 +++--- compiler/rustc_parse/src/parser/stmt.rs | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 577003e94fb..951c3495995 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -498,7 +498,7 @@ impl<'a> Parser<'a> { /// Checks if this expression is a successfully parsed statement. fn expr_is_complete(&self, e: &Expr) -> bool { self.restrictions.contains(Restrictions::STMT_EXPR) - && !classify::expr_requires_semi_to_be_stmt(e) + && !classify::expr_requires_semi_to_be_stmt_FIXME(e) } /// Parses `x..y`, `x..=y`, and `x..`/`x..=`. @@ -2694,7 +2694,7 @@ impl<'a> Parser<'a> { // If it's not a free-standing expression, and is followed by a block, // then it's very likely the condition to an `else if`. if self.check(&TokenKind::OpenDelim(Delimiter::Brace)) - && classify::expr_requires_semi_to_be_stmt(&cond) => + && classify::expr_requires_semi_to_be_stmt_FIXME(&cond) => { self.dcx().emit_err(errors::ExpectedElseBlock { first_tok_span, @@ -3136,7 +3136,7 @@ impl<'a> Parser<'a> { err })?; - let require_comma = classify::expr_requires_semi_to_be_stmt(&expr) + let require_comma = classify::expr_requires_semi_to_be_stmt_FIXME(&expr) && this.token != token::CloseDelim(Delimiter::Brace); if !require_comma { diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index d70afebf1b2..f64a480a18c 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -648,7 +648,7 @@ impl<'a> Parser<'a> { match &mut stmt.kind { // Expression without semicolon. StmtKind::Expr(expr) - if classify::expr_requires_semi_to_be_stmt(expr) + if classify::expr_requires_semi_to_be_stmt_FIXME(expr) && !expr.attrs.is_empty() && ![token::Eof, token::Semi, token::CloseDelim(Delimiter::Brace)] .contains(&self.token.kind) => @@ -662,7 +662,8 @@ impl<'a> Parser<'a> { // Expression without semicolon. StmtKind::Expr(expr) - if self.token != token::Eof && classify::expr_requires_semi_to_be_stmt(expr) => + if self.token != token::Eof + && classify::expr_requires_semi_to_be_stmt_FIXME(expr) => { // Just check for errors and recover; do not eat semicolon yet. -- cgit 1.4.1-3-g733a5 From 9e1cf2098d68356bccf7112bbff1d9b565e80a02 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 29 Dec 2023 16:30:34 -0800 Subject: Macro call with braces does not require semicolon to be statement This commit by itself is supposed to have no effect on behavior. All of the call sites are updated to preserve their previous behavior. The behavior changes are in the commits that follow. --- compiler/rustc_ast/src/util/classify.rs | 30 ++++++++++++---------- compiler/rustc_ast_pretty/src/pprust/state.rs | 5 +++- .../rustc_ast_pretty/src/pprust/state/fixup.rs | 6 ++++- compiler/rustc_lint/src/unused.rs | 5 +++- compiler/rustc_parse/src/parser/expr.rs | 16 +++++++++--- compiler/rustc_parse/src/parser/stmt.rs | 11 +++++--- 6 files changed, 50 insertions(+), 23 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_ast/src/util/classify.rs b/compiler/rustc_ast/src/util/classify.rs index 5ed8d95b12d..86383af1f7c 100644 --- a/compiler/rustc_ast/src/util/classify.rs +++ b/compiler/rustc_ast/src/util/classify.rs @@ -42,19 +42,23 @@ use crate::{ast, token::Delimiter}; /// _ => m! {} - 1, // binary subtraction operator /// } /// ``` -#[allow(non_snake_case)] -pub fn expr_requires_semi_to_be_stmt_FIXME(e: &ast::Expr) -> bool { - !matches!( - e.kind, - ast::ExprKind::If(..) - | ast::ExprKind::Match(..) - | ast::ExprKind::Block(..) - | ast::ExprKind::While(..) - | ast::ExprKind::Loop(..) - | ast::ExprKind::ForLoop { .. } - | ast::ExprKind::TryBlock(..) - | ast::ExprKind::ConstBlock(..) - ) +pub fn expr_requires_semi_to_be_stmt(e: &ast::Expr) -> bool { + use ast::ExprKind::*; + + match &e.kind { + If(..) + | Match(..) + | Block(..) + | While(..) + | Loop(..) + | ForLoop { .. } + | TryBlock(..) + | ConstBlock(..) => false, + + MacCall(mac_call) => mac_call.args.delim != Delimiter::Brace, + + _ => true, + } } /// If an expression ends with `}`, returns the innermost expression ending in the `}` diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index be98b7d37d4..fe17ff41bc3 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1253,7 +1253,10 @@ impl<'a> State<'a> { ast::StmtKind::Expr(expr) => { self.space_if_not_bol(); self.print_expr_outer_attr_style(expr, false, FixupContext::new_stmt()); - if classify::expr_requires_semi_to_be_stmt_FIXME(expr) { + if match expr.kind { + ast::ExprKind::MacCall(_) => true, + _ => classify::expr_requires_semi_to_be_stmt(expr), + } { self.word(";"); } } diff --git a/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs b/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs index 363243215a3..9934f972f9b 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs @@ -128,7 +128,11 @@ impl FixupContext { /// The documentation on `FixupContext::leftmost_subexpression_in_stmt` has /// examples. pub fn would_cause_statement_boundary(self, expr: &Expr) -> bool { - self.leftmost_subexpression_in_stmt && !classify::expr_requires_semi_to_be_stmt_FIXME(expr) + self.leftmost_subexpression_in_stmt + && match expr.kind { + ExprKind::MacCall(_) => false, + _ => !classify::expr_requires_semi_to_be_stmt(expr), + } } /// Determine whether parentheses are needed around the given `let` diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 7dca7017950..19b71564e2e 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -688,7 +688,10 @@ trait UnusedDelimLint { ExprKind::Index(base, _subscript, _) => base, _ => break, }; - if !classify::expr_requires_semi_to_be_stmt_FIXME(innermost) { + if match innermost.kind { + ExprKind::MacCall(_) => false, + _ => !classify::expr_requires_semi_to_be_stmt(innermost), + } { return true; } } diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 951c3495995..5f7bd0835d3 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -498,7 +498,10 @@ impl<'a> Parser<'a> { /// Checks if this expression is a successfully parsed statement. fn expr_is_complete(&self, e: &Expr) -> bool { self.restrictions.contains(Restrictions::STMT_EXPR) - && !classify::expr_requires_semi_to_be_stmt_FIXME(e) + && match e.kind { + ExprKind::MacCall(_) => false, + _ => !classify::expr_requires_semi_to_be_stmt(e), + } } /// Parses `x..y`, `x..=y`, and `x..`/`x..=`. @@ -2694,7 +2697,10 @@ impl<'a> Parser<'a> { // If it's not a free-standing expression, and is followed by a block, // then it's very likely the condition to an `else if`. if self.check(&TokenKind::OpenDelim(Delimiter::Brace)) - && classify::expr_requires_semi_to_be_stmt_FIXME(&cond) => + && match cond.kind { + ExprKind::MacCall(_) => true, + _ => classify::expr_requires_semi_to_be_stmt(&cond), + } => { self.dcx().emit_err(errors::ExpectedElseBlock { first_tok_span, @@ -3136,8 +3142,10 @@ impl<'a> Parser<'a> { err })?; - let require_comma = classify::expr_requires_semi_to_be_stmt_FIXME(&expr) - && this.token != token::CloseDelim(Delimiter::Brace); + let require_comma = match expr.kind { + ExprKind::MacCall(_) => true, + _ => classify::expr_requires_semi_to_be_stmt(&expr), + } && this.token != token::CloseDelim(Delimiter::Brace); if !require_comma { arm_body = Some(expr); diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index f64a480a18c..684799eb6a7 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -648,8 +648,10 @@ impl<'a> Parser<'a> { match &mut stmt.kind { // Expression without semicolon. StmtKind::Expr(expr) - if classify::expr_requires_semi_to_be_stmt_FIXME(expr) - && !expr.attrs.is_empty() + if match expr.kind { + ExprKind::MacCall(_) => true, + _ => classify::expr_requires_semi_to_be_stmt(expr), + } && !expr.attrs.is_empty() && ![token::Eof, token::Semi, token::CloseDelim(Delimiter::Brace)] .contains(&self.token.kind) => { @@ -663,7 +665,10 @@ impl<'a> Parser<'a> { // Expression without semicolon. StmtKind::Expr(expr) if self.token != token::Eof - && classify::expr_requires_semi_to_be_stmt_FIXME(expr) => + && match expr.kind { + ExprKind::MacCall(_) => true, + _ => classify::expr_requires_semi_to_be_stmt(expr), + } => { // Just check for errors and recover; do not eat semicolon yet. -- cgit 1.4.1-3-g733a5 From 8adcaf5df27ac63b5e83612bd08cae12d3d1725e Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 29 Dec 2023 16:13:55 -0800 Subject: Mark Parser::expr_is_complete call sites --- compiler/rustc_parse/src/parser/expr.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 5f7bd0835d3..602dbbb5725 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -190,7 +190,7 @@ impl<'a> Parser<'a> { } }; - if !self.should_continue_as_assoc_expr(&lhs) { + if !self.should_continue_as_assoc_expr_FIXME(&lhs) { return Ok(lhs); } @@ -383,8 +383,9 @@ impl<'a> Parser<'a> { Ok(lhs) } - fn should_continue_as_assoc_expr(&mut self, lhs: &Expr) -> bool { - match (self.expr_is_complete(lhs), AssocOp::from_token(&self.token)) { + #[allow(non_snake_case)] + fn should_continue_as_assoc_expr_FIXME(&mut self, lhs: &Expr) -> bool { + match (self.expr_is_complete_FIXME(lhs), AssocOp::from_token(&self.token)) { // Semi-statement forms are odd: // See https://github.com/rust-lang/rust/issues/29071 (true, None) => false, @@ -496,7 +497,8 @@ impl<'a> Parser<'a> { } /// Checks if this expression is a successfully parsed statement. - fn expr_is_complete(&self, e: &Expr) -> bool { + #[allow(non_snake_case)] + fn expr_is_complete_FIXME(&self, e: &Expr) -> bool { self.restrictions.contains(Restrictions::STMT_EXPR) && match e.kind { ExprKind::MacCall(_) => false, @@ -1012,7 +1014,7 @@ impl<'a> Parser<'a> { e = self.parse_dot_suffix_expr(lo, e)?; continue; } - if self.expr_is_complete(&e) { + if self.expr_is_complete_FIXME(&e) { return Ok(e); } e = match self.token.kind { -- cgit 1.4.1-3-g733a5 From 9dbe33d256e6b3919d2fc0a6561b78d2ed3c628c Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 29 Dec 2023 16:41:25 -0800 Subject: Document MacCall special case in Parser::expr_is_complete --- compiler/rustc_parse/src/parser/expr.rs | 51 ++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 7 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 602dbbb5725..98b41013a2c 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -190,7 +190,7 @@ impl<'a> Parser<'a> { } }; - if !self.should_continue_as_assoc_expr_FIXME(&lhs) { + if !self.should_continue_as_assoc_expr(&lhs) { return Ok(lhs); } @@ -383,9 +383,8 @@ impl<'a> Parser<'a> { Ok(lhs) } - #[allow(non_snake_case)] - fn should_continue_as_assoc_expr_FIXME(&mut self, lhs: &Expr) -> bool { - match (self.expr_is_complete_FIXME(lhs), AssocOp::from_token(&self.token)) { + fn should_continue_as_assoc_expr(&mut self, lhs: &Expr) -> bool { + match (self.expr_is_complete(lhs), AssocOp::from_token(&self.token)) { // Semi-statement forms are odd: // See https://github.com/rust-lang/rust/issues/29071 (true, None) => false, @@ -497,10 +496,48 @@ impl<'a> Parser<'a> { } /// Checks if this expression is a successfully parsed statement. - #[allow(non_snake_case)] - fn expr_is_complete_FIXME(&self, e: &Expr) -> bool { + /// + /// This determines whether to continue parsing more of an expression in a + /// match arm (false) vs continue to the next arm (true). + /// + /// ```ignore (illustrative) + /// match ... { + /// // Is this calling $e as a function, or is it the start of a new arm + /// // with a tuple pattern? + /// _ => $e ( + /// ^ ) + /// + /// // Is this an Index operation, or new arm with a slice pattern? + /// _ => $e [ + /// ^ ] + /// + /// // Is this a binary operator, or leading vert in a new arm? Same for + /// // other punctuation which can either be a binary operator in + /// // expression or unary operator in pattern, such as `&` and `-`. + /// _ => $e | + /// ^ + /// } + /// ``` + /// + /// If $e is something like `path::to` or `(…)`, continue parsing the same + /// arm. + /// + /// If $e is something like `{}` or `if … {}`, then terminate the current + /// arm and parse a new arm. + fn expr_is_complete(&self, e: &Expr) -> bool { self.restrictions.contains(Restrictions::STMT_EXPR) && match e.kind { + // Surprising special case: even though braced macro calls like + // `m! {}` normally introduce a statement boundary when found at + // the head of a statement, in match arms they do not terminate + // the arm. + // + // let _ = { m! {} () }; // macro call followed by unit + // + // match ... { + // _ => m! {} (), // macro that expands to a function, which is then called + // } + // ExprKind::MacCall(_) => false, _ => !classify::expr_requires_semi_to_be_stmt(e), } @@ -1014,7 +1051,7 @@ impl<'a> Parser<'a> { e = self.parse_dot_suffix_expr(lo, e)?; continue; } - if self.expr_is_complete_FIXME(&e) { + if self.expr_is_complete(&e) { return Ok(e); } e = match self.token.kind { -- cgit 1.4.1-3-g733a5 From 728e117166e1dab6f8333d61e1315172c558fce5 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 29 Dec 2023 17:15:34 -0800 Subject: Document MacCall special case in Parser::parse_arm --- compiler/rustc_parse/src/parser/expr.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 98b41013a2c..bb0873a814d 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -3182,6 +3182,17 @@ impl<'a> Parser<'a> { })?; let require_comma = match expr.kind { + // Special case: braced macro calls require comma in a match + // arm, even though they do not require semicolon in a + // statement. + // + // m! {} // okay without semicolon + // + // match ... { + // _ => m! {}, // requires comma + // _ => ... + // } + // ExprKind::MacCall(_) => true, _ => classify::expr_requires_semi_to_be_stmt(&expr), } && this.token != token::CloseDelim(Delimiter::Brace); -- cgit 1.4.1-3-g733a5 From aedc1b6ad4845242d06a3f7cfb9b57db227b3511 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 29 Dec 2023 17:37:32 -0800 Subject: Remove MacCall special case from recovery after missing 'if' after 'else' The change to the test is a little goofy because the compiler was guessing "correctly" before that `falsy! {}` is the condition as opposed to the else body. But I believe this change is fundamentally correct. Braced macro invocations in statement position are most often item-like (`thread_local! {...}`) as opposed to parenthesized macro invocations which are condition-like (`cfg!(...)`). --- compiler/rustc_parse/src/parser/expr.rs | 34 +++++++++++++++++++++++++++------ tests/ui/parser/else-no-if.stderr | 10 ++++------ 2 files changed, 32 insertions(+), 12 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index bb0873a814d..52e3e33691a 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -2733,13 +2733,35 @@ impl<'a> Parser<'a> { let first_tok_span = self.token.span; match self.parse_expr() { Ok(cond) - // If it's not a free-standing expression, and is followed by a block, - // then it's very likely the condition to an `else if`. + // Try to guess the difference between a "condition-like" vs + // "statement-like" expression. + // + // We are seeing the following code, in which $cond is neither + // ExprKind::Block nor ExprKind::If (the 2 cases wherein this + // would be valid syntax). + // + // if ... { + // } else $cond + // + // If $cond is "condition-like" such as ExprKind::Binary, we + // want to suggest inserting `if`. + // + // if ... { + // } else if a == b { + // ^^ + // } + // + // If $cond is "statement-like" such as ExprKind::While then we + // want to suggest wrapping in braces. + // + // if ... { + // } else { + // ^ + // while true {} + // } + // ^ if self.check(&TokenKind::OpenDelim(Delimiter::Brace)) - && match cond.kind { - ExprKind::MacCall(_) => true, - _ => classify::expr_requires_semi_to_be_stmt(&cond), - } => + && classify::expr_requires_semi_to_be_stmt(&cond) => { self.dcx().emit_err(errors::ExpectedElseBlock { first_tok_span, diff --git a/tests/ui/parser/else-no-if.stderr b/tests/ui/parser/else-no-if.stderr index 9954505e7c8..2e3e8f6b50e 100644 --- a/tests/ui/parser/else-no-if.stderr +++ b/tests/ui/parser/else-no-if.stderr @@ -74,14 +74,12 @@ error: expected `{`, found `falsy` --> $DIR/else-no-if.rs:47:12 | LL | } else falsy! {} { - | ---- ^^^^^ - | | - | expected an `if` or a block after this `else` + | ^^^^^ expected `{` | -help: add an `if` if this is the condition of a chained `else if` statement +help: try placing this code inside a block | -LL | } else if falsy! {} { - | ++ +LL | } else { falsy! {} } { + | + + error: expected `{`, found `falsy` --> $DIR/else-no-if.rs:54:12 -- cgit 1.4.1-3-g733a5 From 53521faf06cb5ada4df7984fcd53b602b4ee2dd0 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 29 Dec 2023 18:04:04 -0800 Subject: Remove MacCall special cases from Parser::parse_full_stmt It is impossible for expr here to be a braced macro call. Expr comes from `parse_stmt_without_recovery`, in which macro calls are parsed by `parse_stmt_mac`. See this part: let kind = if (style == MacStmtStyle::Braces && self.token != token::Dot && self.token != token::Question) || self.token == token::Semi || self.token == token::Eof { StmtKind::MacCall(P(MacCallStmt { mac, style, attrs, tokens: None })) } else { // 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_assoc_with( 0, LhsExpr::AlreadyParsed { expr: e, starts_statement: false }, )?; StmtKind::Expr(e) }; A braced macro call at the head of a statement is always either extended into ExprKind::Field / MethodCall / Await / Try / Binary, or else returned as StmtKind::MacCall. We can never get a StmtKind::Expr containing ExprKind::MacCall containing brace delimiter. --- compiler/rustc_parse/src/parser/stmt.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 684799eb6a7..d70afebf1b2 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -648,10 +648,8 @@ impl<'a> Parser<'a> { match &mut stmt.kind { // Expression without semicolon. StmtKind::Expr(expr) - if match expr.kind { - ExprKind::MacCall(_) => true, - _ => classify::expr_requires_semi_to_be_stmt(expr), - } && !expr.attrs.is_empty() + if classify::expr_requires_semi_to_be_stmt(expr) + && !expr.attrs.is_empty() && ![token::Eof, token::Semi, token::CloseDelim(Delimiter::Brace)] .contains(&self.token.kind) => { @@ -664,11 +662,7 @@ impl<'a> Parser<'a> { // Expression without semicolon. StmtKind::Expr(expr) - if self.token != token::Eof - && match expr.kind { - ExprKind::MacCall(_) => true, - _ => classify::expr_requires_semi_to_be_stmt(expr), - } => + if self.token != token::Eof && classify::expr_requires_semi_to_be_stmt(expr) => { // Just check for errors and recover; do not eat semicolon yet. -- cgit 1.4.1-3-g733a5 From 10227eaee70fb9e042bb0b317ad562677c81661d Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 19 Apr 2024 18:46:16 -0700 Subject: Add classify::expr_is_complete --- compiler/rustc_ast/src/util/classify.rs | 96 +++++++++++++++++++-------------- compiler/rustc_parse/src/parser/expr.rs | 62 ++------------------- 2 files changed, 60 insertions(+), 98 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_ast/src/util/classify.rs b/compiler/rustc_ast/src/util/classify.rs index 86383af1f7c..f6e9e1a87c4 100644 --- a/compiler/rustc_ast/src/util/classify.rs +++ b/compiler/rustc_ast/src/util/classify.rs @@ -1,12 +1,60 @@ //! Routines the parser and pretty-printer use to classify AST nodes. +use crate::ast::ExprKind::*; use crate::{ast, token::Delimiter}; +/// This classification determines whether various syntactic positions break out +/// of parsing the current expression (true) or continue parsing more of the +/// same expression (false). +/// +/// For example, it's relevant in the parsing of match arms: +/// +/// ```ignore (illustrative) +/// match ... { +/// // Is this calling $e as a function, or is it the start of a new arm +/// // with a tuple pattern? +/// _ => $e ( +/// ^ ) +/// +/// // Is this an Index operation, or new arm with a slice pattern? +/// _ => $e [ +/// ^ ] +/// +/// // Is this a binary operator, or leading vert in a new arm? Same for +/// // other punctuation which can either be a binary operator in +/// // expression or unary operator in pattern, such as `&` and `-`. +/// _ => $e | +/// ^ +/// } +/// ``` +/// +/// If $e is something like `{}` or `if … {}`, then terminate the current +/// arm and parse a new arm. +/// +/// If $e is something like `path::to` or `(…)`, continue parsing the same +/// arm. +/// +/// *Almost* the same classification is used as an early bail-out for parsing +/// statements. See `expr_requires_semi_to_be_stmt`. +pub fn expr_is_complete(e: &ast::Expr) -> bool { + matches!( + e.kind, + If(..) + | Match(..) + | Block(..) + | While(..) + | Loop(..) + | ForLoop { .. } + | TryBlock(..) + | ConstBlock(..) + ) +} + /// Does this expression require a semicolon to be treated as a statement? /// /// The negation of this: "can this expression be used as a statement without a -/// semicolon" -- is used as an early bail-out in the parser so that, for -/// instance, +/// semicolon" -- is used as an early bail-out when parsing statements so that, +/// for instance, /// /// ```ignore (illustrative) /// if true {...} else {...} @@ -15,56 +63,26 @@ use crate::{ast, token::Delimiter}; /// /// isn't parsed as `(if true {...} else {...} | x) | 5`. /// -/// Nearly the same early bail-out also occurs in the right-hand side of match -/// arms: +/// Surprising special case: even though braced macro calls like `m! {}` +/// normally do not introduce a boundary when found at the head of a match arm, +/// they do terminate the parsing of a statement. /// /// ```ignore (illustrative) -/// match i { -/// 0 => if true {...} else {...} -/// | x => {} +/// match ... { +/// _ => m! {} (), // macro that expands to a function, which is then called /// } -/// ``` -/// -/// Here the `|` is a leading vert in a second match arm. It is not a binary -/// operator with the If as its left operand. If the first arm were some other -/// expression for which `expr_requires_semi_to_be_stmt` returns true, then the -/// `|` on the next line would be a binary operator (leading to a parse error). /// -/// The statement case and the match-arm case are "nearly" the same early -/// bail-out because of 1 edge case. Macro calls with brace delimiter terminate -/// a statement without a semicolon, but do not terminate a match-arm without -/// comma. -/// -/// ```ignore (illustrative) -/// m! {} - 1; // two statements: a macro call followed by -1 literal -/// -/// match () { -/// _ => m! {} - 1, // binary subtraction operator -/// } +/// let _ = { m! {} () }; // macro call followed by unit /// ``` pub fn expr_requires_semi_to_be_stmt(e: &ast::Expr) -> bool { - use ast::ExprKind::*; - match &e.kind { - If(..) - | Match(..) - | Block(..) - | While(..) - | Loop(..) - | ForLoop { .. } - | TryBlock(..) - | ConstBlock(..) => false, - MacCall(mac_call) => mac_call.args.delim != Delimiter::Brace, - - _ => true, + _ => !expr_is_complete(e), } } /// If an expression ends with `}`, returns the innermost expression ending in the `}` pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> { - use ast::ExprKind::*; - loop { match &expr.kind { AddrOf(_, _, e) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 52e3e33691a..441aa5b0806 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -496,51 +496,8 @@ impl<'a> Parser<'a> { } /// Checks if this expression is a successfully parsed statement. - /// - /// This determines whether to continue parsing more of an expression in a - /// match arm (false) vs continue to the next arm (true). - /// - /// ```ignore (illustrative) - /// match ... { - /// // Is this calling $e as a function, or is it the start of a new arm - /// // with a tuple pattern? - /// _ => $e ( - /// ^ ) - /// - /// // Is this an Index operation, or new arm with a slice pattern? - /// _ => $e [ - /// ^ ] - /// - /// // Is this a binary operator, or leading vert in a new arm? Same for - /// // other punctuation which can either be a binary operator in - /// // expression or unary operator in pattern, such as `&` and `-`. - /// _ => $e | - /// ^ - /// } - /// ``` - /// - /// If $e is something like `path::to` or `(…)`, continue parsing the same - /// arm. - /// - /// If $e is something like `{}` or `if … {}`, then terminate the current - /// arm and parse a new arm. fn expr_is_complete(&self, e: &Expr) -> bool { - self.restrictions.contains(Restrictions::STMT_EXPR) - && match e.kind { - // Surprising special case: even though braced macro calls like - // `m! {}` normally introduce a statement boundary when found at - // the head of a statement, in match arms they do not terminate - // the arm. - // - // let _ = { m! {} () }; // macro call followed by unit - // - // match ... { - // _ => m! {} (), // macro that expands to a function, which is then called - // } - // - ExprKind::MacCall(_) => false, - _ => !classify::expr_requires_semi_to_be_stmt(e), - } + self.restrictions.contains(Restrictions::STMT_EXPR) && classify::expr_is_complete(e) } /// Parses `x..y`, `x..=y`, and `x..`/`x..=`. @@ -3203,21 +3160,8 @@ impl<'a> Parser<'a> { err })?; - let require_comma = match expr.kind { - // Special case: braced macro calls require comma in a match - // arm, even though they do not require semicolon in a - // statement. - // - // m! {} // okay without semicolon - // - // match ... { - // _ => m! {}, // requires comma - // _ => ... - // } - // - ExprKind::MacCall(_) => true, - _ => classify::expr_requires_semi_to_be_stmt(&expr), - } && this.token != token::CloseDelim(Delimiter::Brace); + let require_comma = !classify::expr_is_complete(&expr) + && this.token != token::CloseDelim(Delimiter::Brace); if !require_comma { arm_body = Some(expr); -- cgit 1.4.1-3-g733a5 From 9a63a42cb787476930f094fdbd9885251ae01de0 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 22 Apr 2024 16:29:27 +1000 Subject: Remove a `Span` from `TokenKind::Interpolated`. This span records the declaration of the metavariable in the LHS of the macro. It's used in a couple of error messages. Unfortunately, it gets in the way of the long-term goal of removing `TokenKind::Interpolated`. So this commit removes it, which degrades a couple of (obscure) error messages but makes things simpler and enables the next commit. --- compiler/rustc_ast/src/attr/mod.rs | 2 +- compiler/rustc_ast/src/mut_visit.rs | 2 -- compiler/rustc_ast/src/token.rs | 33 +++++++++++++++----------- compiler/rustc_ast/src/tokenstream.rs | 6 ++--- compiler/rustc_ast_pretty/src/pprust/state.rs | 2 +- compiler/rustc_expand/src/mbe/diagnostics.rs | 6 ----- compiler/rustc_expand/src/mbe/macro_parser.rs | 11 +++------ compiler/rustc_expand/src/proc_macro.rs | 2 +- compiler/rustc_expand/src/proc_macro_server.rs | 6 ++--- compiler/rustc_parse/src/parser/attr.rs | 2 +- compiler/rustc_parse/src/parser/diagnostics.rs | 23 +++++++----------- compiler/rustc_parse/src/parser/expr.rs | 2 +- compiler/rustc_parse/src/parser/item.rs | 2 +- compiler/rustc_parse/src/parser/mod.rs | 28 +++++++--------------- compiler/rustc_parse/src/parser/nonterminal.rs | 18 +++++++------- compiler/rustc_parse/src/parser/pat.rs | 2 +- compiler/rustc_parse/src/parser/path.rs | 2 +- tests/ui/macros/nonterminal-matching.stderr | 2 -- tests/ui/macros/trace_faulty_macros.stderr | 8 ++----- 19 files changed, 62 insertions(+), 97 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs index 0f12fd31975..0684163617f 100644 --- a/compiler/rustc_ast/src/attr/mod.rs +++ b/compiler/rustc_ast/src/attr/mod.rs @@ -345,7 +345,7 @@ impl MetaItem { let span = span.with_hi(segments.last().unwrap().ident.span.hi()); Path { span, segments, tokens: None } } - Some(TokenTree::Token(Token { kind: token::Interpolated(nt), .. }, _)) => match &nt.0 { + Some(TokenTree::Token(Token { kind: token::Interpolated(nt), .. }, _)) => match &**nt { token::Nonterminal::NtMeta(item) => return item.meta(item.path.span), token::Nonterminal::NtPath(path) => (**path).clone(), _ => return None, diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 379c2e273eb..07358047025 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -783,8 +783,6 @@ pub fn visit_token(t: &mut Token, vis: &mut T) { } token::Interpolated(nt) => { let nt = Lrc::make_mut(nt); - let (nt, sp) = (&mut nt.0, &mut nt.1); - vis.visit_span(sp); visit_nonterminal(nt, vis); } _ => {} diff --git a/compiler/rustc_ast/src/token.rs b/compiler/rustc_ast/src/token.rs index 72f89737f99..590c3daf0c2 100644 --- a/compiler/rustc_ast/src/token.rs +++ b/compiler/rustc_ast/src/token.rs @@ -111,7 +111,7 @@ impl Lit { Ident(name, IdentIsRaw::No) if name.is_bool_lit() => Some(Lit::new(Bool, name, None)), Literal(token_lit) => Some(token_lit), Interpolated(ref nt) - if let NtExpr(expr) | NtLiteral(expr) = &nt.0 + if let NtExpr(expr) | NtLiteral(expr) = &**nt && let ast::ExprKind::Lit(token_lit) = expr.kind => { Some(token_lit) @@ -333,7 +333,11 @@ pub enum TokenKind { /// - It prevents `Token` from implementing `Copy`. /// It adds complexity and likely slows things down. Please don't add new /// occurrences of this token kind! - Interpolated(Lrc<(Nonterminal, Span)>), + /// + /// The span in the surrounding `Token` is that of the metavariable in the + /// macro's RHS. The span within the Nonterminal is that of the fragment + /// passed to the macro at the call site. + Interpolated(Lrc), /// A doc comment token. /// `Symbol` is the doc comment's data excluding its "quotes" (`///`, `/**`, etc) @@ -441,7 +445,7 @@ impl Token { /// if they keep spans or perform edition checks. pub fn uninterpolated_span(&self) -> Span { match &self.kind { - Interpolated(nt) => nt.0.use_span(), + Interpolated(nt) => nt.use_span(), _ => self.span, } } @@ -486,7 +490,7 @@ impl Token { PathSep | // global path Lifetime(..) | // labeled loop Pound => true, // expression attributes - Interpolated(ref nt) => matches!(&nt.0, NtLiteral(..) | + Interpolated(ref nt) => matches!(&**nt, NtLiteral(..) | NtExpr(..) | NtBlock(..) | NtPath(..)), @@ -510,7 +514,7 @@ impl Token { | DotDot | DotDotDot | DotDotEq // ranges | Lt | BinOp(Shl) // associated path | PathSep => true, // global path - Interpolated(ref nt) => matches!(&nt.0, NtLiteral(..) | + Interpolated(ref nt) => matches!(&**nt, NtLiteral(..) | NtPat(..) | NtBlock(..) | NtPath(..)), @@ -533,7 +537,7 @@ impl Token { Lifetime(..) | // lifetime bound in trait object Lt | BinOp(Shl) | // associated path PathSep => true, // global path - Interpolated(ref nt) => matches!(&nt.0, NtTy(..) | NtPath(..)), + Interpolated(ref nt) => matches!(&**nt, NtTy(..) | NtPath(..)), // For anonymous structs or unions, which only appear in specific positions // (type of struct fields or union fields), we don't consider them as regular types _ => false, @@ -544,7 +548,7 @@ impl Token { pub fn can_begin_const_arg(&self) -> bool { match self.kind { OpenDelim(Delimiter::Brace) => true, - Interpolated(ref nt) => matches!(&nt.0, NtExpr(..) | NtBlock(..) | NtLiteral(..)), + Interpolated(ref nt) => matches!(&**nt, NtExpr(..) | NtBlock(..) | NtLiteral(..)), _ => self.can_begin_literal_maybe_minus(), } } @@ -589,7 +593,7 @@ impl Token { match self.uninterpolate().kind { Literal(..) | BinOp(Minus) => true, Ident(name, IdentIsRaw::No) if name.is_bool_lit() => true, - Interpolated(ref nt) => match &nt.0 { + Interpolated(ref nt) => match &**nt { NtLiteral(_) => true, NtExpr(e) => match &e.kind { ast::ExprKind::Lit(_) => true, @@ -610,7 +614,7 @@ impl Token { /// otherwise returns the original token. pub fn uninterpolate(&self) -> Cow<'_, Token> { match &self.kind { - Interpolated(nt) => match &nt.0 { + Interpolated(nt) => match &**nt { NtIdent(ident, is_raw) => { Cow::Owned(Token::new(Ident(ident.name, *is_raw), ident.span)) } @@ -627,7 +631,7 @@ impl Token { // We avoid using `Token::uninterpolate` here because it's slow. match &self.kind { &Ident(name, is_raw) => Some((Ident::new(name, self.span), is_raw)), - Interpolated(nt) => match &nt.0 { + Interpolated(nt) => match &**nt { NtIdent(ident, is_raw) => Some((*ident, *is_raw)), _ => None, }, @@ -641,7 +645,7 @@ impl Token { // We avoid using `Token::uninterpolate` here because it's slow. match &self.kind { &Lifetime(name) => Some(Ident::new(name, self.span)), - Interpolated(nt) => match &nt.0 { + Interpolated(nt) => match &**nt { NtLifetime(ident) => Some(*ident), _ => None, }, @@ -668,7 +672,7 @@ impl Token { /// Returns `true` if the token is an interpolated path. fn is_whole_path(&self) -> bool { if let Interpolated(nt) = &self.kind - && let NtPath(..) = &nt.0 + && let NtPath(..) = &**nt { return true; } @@ -681,7 +685,7 @@ impl Token { /// (which happens while parsing the result of macro expansion)? pub fn is_whole_expr(&self) -> bool { if let Interpolated(nt) = &self.kind - && let NtExpr(_) | NtLiteral(_) | NtPath(_) | NtBlock(_) = &nt.0 + && let NtExpr(_) | NtLiteral(_) | NtPath(_) | NtBlock(_) = &**nt { return true; } @@ -692,7 +696,7 @@ impl Token { /// Is the token an interpolated block (`$b:block`)? pub fn is_whole_block(&self) -> bool { if let Interpolated(nt) = &self.kind - && let NtBlock(..) = &nt.0 + && let NtBlock(..) = &**nt { return true; } @@ -857,6 +861,7 @@ pub enum Nonterminal { NtPat(P), NtExpr(P), NtTy(P), + /// The span is for the identifier argument passed to the macro. NtIdent(Ident, IdentIsRaw), NtLifetime(Ident), NtLiteral(P), diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index 8e80161af1b..d532a884184 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -490,14 +490,14 @@ impl TokenStream { fn flatten_token(token: &Token, spacing: Spacing) -> TokenTree { match &token.kind { - token::Interpolated(nt) if let token::NtIdent(ident, is_raw) = nt.0 => { - TokenTree::Token(Token::new(token::Ident(ident.name, is_raw), ident.span), spacing) + token::Interpolated(nt) if let token::NtIdent(ident, is_raw) = &**nt => { + TokenTree::Token(Token::new(token::Ident(ident.name, *is_raw), ident.span), spacing) } token::Interpolated(nt) => TokenTree::Delimited( DelimSpan::from_single(token.span), DelimSpacing::new(Spacing::JointHidden, spacing), Delimiter::Invisible, - TokenStream::from_nonterminal_ast(&nt.0).flattened(), + TokenStream::from_nonterminal_ast(&nt).flattened(), ), _ => TokenTree::Token(token.clone(), spacing), } diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 2c176828c84..f438cee4cd2 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -926,7 +926,7 @@ pub trait PrintState<'a>: std::ops::Deref + std::ops::Dere } token::Eof => "".into(), - token::Interpolated(ref nt) => self.nonterminal_to_string(&nt.0).into(), + token::Interpolated(ref nt) => self.nonterminal_to_string(&nt).into(), } } diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index 464361cb402..3fee39dd085 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -73,12 +73,6 @@ pub(super) fn failed_to_match_macro<'cx>( && (matches!(expected_token.kind, TokenKind::Interpolated(_)) || matches!(token.kind, TokenKind::Interpolated(_))) { - if let TokenKind::Interpolated(node) = &expected_token.kind { - err.span_label(node.1, ""); - } - if let TokenKind::Interpolated(node) = &token.kind { - err.span_label(node.1, ""); - } err.note("captured metavariables except for `:tt`, `:ident` and `:lifetime` cannot be compared to other tokens"); err.note("see for more information"); diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index ffb50f4c92e..27cf6fee702 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -75,10 +75,9 @@ pub(crate) use ParseResult::*; use crate::mbe::{macro_rules::Tracker, KleeneOp, TokenTree}; -use rustc_ast::token::{self, DocComment, Nonterminal, NonterminalKind, Token}; +use rustc_ast::token::{self, DocComment, NonterminalKind, Token}; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; -use rustc_data_structures::sync::Lrc; use rustc_errors::ErrorGuaranteed; use rustc_lint_defs::pluralize; use rustc_parse::parser::{ParseNtResult, Parser}; @@ -392,7 +391,7 @@ pub(super) fn count_metavar_decls(matcher: &[TokenTree]) -> usize { #[derive(Debug, Clone)] pub(crate) enum NamedMatch { MatchedSeq(Vec), - MatchedSingle(ParseNtResult>), + MatchedSingle(ParseNtResult), } /// Performs a token equality check, ignoring syntax context (that is, an unhygienic comparison) @@ -686,11 +685,7 @@ impl TtParser { } Ok(nt) => nt, }; - mp.push_match( - next_metavar, - seq_depth, - MatchedSingle(nt.map_nt(|nt| (Lrc::new((nt, span))))), - ); + mp.push_match(next_metavar, seq_depth, MatchedSingle(nt)); mp.idx += 1; } else { unreachable!() diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 4b5c148cb55..530059e53c2 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -127,7 +127,7 @@ impl MultiItemModifier for DeriveProcMacro { Annotatable::Stmt(stmt) => token::NtStmt(stmt), _ => unreachable!(), }; - TokenStream::token_alone(token::Interpolated(Lrc::new((nt, span))), DUMMY_SP) + TokenStream::token_alone(token::Interpolated(Lrc::new(nt)), DUMMY_SP) } else { item.to_tokens() }; diff --git a/compiler/rustc_expand/src/proc_macro_server.rs b/compiler/rustc_expand/src/proc_macro_server.rs index 5a66b0fbdef..8cf96580161 100644 --- a/compiler/rustc_expand/src/proc_macro_server.rs +++ b/compiler/rustc_expand/src/proc_macro_server.rs @@ -259,7 +259,7 @@ impl FromInternal<(TokenStream, &mut Rustc<'_, '_>)> for Vec { + Interpolated(ref nt) if let NtIdent(ident, is_raw) = &**nt => { trees.push(TokenTree::Ident(Ident { sym: ident.name, is_raw: matches!(is_raw, IdentIsRaw::Yes), @@ -268,14 +268,14 @@ impl FromInternal<(TokenStream, &mut Rustc<'_, '_>)> for Vec { - let stream = TokenStream::from_nonterminal_ast(&nt.0); + let stream = TokenStream::from_nonterminal_ast(&nt); // A hack used to pass AST fragments to attribute and derive // macros as a single nonterminal token instead of a token // stream. Such token needs to be "unwrapped" and not // represented as a delimited group. // FIXME: It needs to be removed, but there are some // compatibility issues (see #73345). - if crate::base::nt_pretty_printing_compatibility_hack(&nt.0, rustc.ecx.sess) { + if crate::base::nt_pretty_printing_compatibility_hack(&nt, rustc.ecx.sess) { trees.extend(Self::from_internal((stream, rustc))); } else { trees.push(TokenTree::Group(Group { diff --git a/compiler/rustc_parse/src/parser/attr.rs b/compiler/rustc_parse/src/parser/attr.rs index d5d8060d909..a57eb70c705 100644 --- a/compiler/rustc_parse/src/parser/attr.rs +++ b/compiler/rustc_parse/src/parser/attr.rs @@ -363,7 +363,7 @@ impl<'a> Parser<'a> { // We can't use `maybe_whole` here because it would bump in the `None` // case, which we don't want. if let token::Interpolated(nt) = &self.token.kind - && let token::NtMeta(attr_item) = &nt.0 + && let token::NtMeta(attr_item) = &**nt { match attr_item.meta(attr_item.path.span) { Some(meta) => { diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 50698dbf9c1..63762f64be9 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -2372,9 +2372,9 @@ impl<'a> Parser<'a> { // in a subsequent macro invocation (#71039). let mut tok = self.token.clone(); let mut labels = vec![]; - while let TokenKind::Interpolated(node) = &tok.kind { - let tokens = node.0.tokens(); - labels.push(node.clone()); + while let TokenKind::Interpolated(nt) = &tok.kind { + let tokens = nt.tokens(); + labels.push(nt.clone()); if let Some(tokens) = tokens && let tokens = tokens.to_attr_token_stream() && let tokens = tokens.0.deref() @@ -2387,27 +2387,20 @@ impl<'a> Parser<'a> { } let mut iter = labels.into_iter().peekable(); let mut show_link = false; - while let Some(node) = iter.next() { - let descr = node.0.descr(); + while let Some(nt) = iter.next() { + let descr = nt.descr(); if let Some(next) = iter.peek() { - let next_descr = next.0.descr(); + let next_descr = next.descr(); if next_descr != descr { - err.span_label(next.1, format!("this macro fragment matcher is {next_descr}")); - err.span_label(node.1, format!("this macro fragment matcher is {descr}")); + err.span_label(next.use_span(), format!("this is expected to be {next_descr}")); err.span_label( - next.0.use_span(), - format!("this is expected to be {next_descr}"), - ); - err.span_label( - node.0.use_span(), + nt.use_span(), format!( "this is interpreted as {}, but it is expected to be {}", next_descr, descr, ), ); show_link = true; - } else { - err.span_label(node.1, ""); } } } diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 441aa5b0806..8e3cd0f8ea9 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -45,7 +45,7 @@ use thin_vec::{thin_vec, ThinVec}; macro_rules! maybe_whole_expr { ($p:expr) => { if let token::Interpolated(nt) = &$p.token.kind { - match &nt.0 { + match &**nt { token::NtExpr(e) | token::NtLiteral(e) => { let e = e.clone(); $p.bump(); diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index df6996dbc45..29957bd2f71 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -2841,7 +2841,7 @@ impl<'a> Parser<'a> { fn is_named_param(&self) -> bool { let offset = match &self.token.kind { - token::Interpolated(nt) => match &nt.0 { + token::Interpolated(nt) => match &**nt { token::NtPat(..) => return self.look_ahead(1, |t| t == &token::Colon), _ => 0, }, diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index bfb6c4a3885..cc40c8a7126 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -11,7 +11,6 @@ mod stmt; mod ty; use crate::lexer::UnmatchedDelim; -use ast::token::IdentIsRaw; pub use attr_wrapper::AttrWrapper; pub use diagnostics::AttemptLocalParseRecovery; pub(crate) use expr::ForbiddenLetReason; @@ -21,7 +20,7 @@ pub use path::PathStyle; use core::fmt; use rustc_ast::ptr::P; -use rustc_ast::token::{self, Delimiter, Token, TokenKind}; +use rustc_ast::token::{self, Delimiter, IdentIsRaw, Nonterminal, Token, TokenKind}; use rustc_ast::tokenstream::{AttributesData, DelimSpacing, DelimSpan, Spacing}; use rustc_ast::tokenstream::{TokenStream, TokenTree, TokenTreeCursor}; use rustc_ast::util::case::Case; @@ -32,6 +31,7 @@ use rustc_ast::{ }; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, Diag, FatalError, MultiSpan, PResult}; use rustc_session::parse::ParseSess; use rustc_span::symbol::{kw, sym, Ident, Symbol}; @@ -107,7 +107,7 @@ pub enum TrailingToken { macro_rules! maybe_whole { ($p:expr, $constructor:ident, |$x:ident| $e:expr) => { if let token::Interpolated(nt) = &$p.token.kind - && let token::$constructor(x) = &nt.0 + && let token::$constructor(x) = &**nt { #[allow(unused_mut)] let mut $x = x.clone(); @@ -125,7 +125,7 @@ macro_rules! maybe_recover_from_interpolated_ty_qpath { && $self.may_recover() && $self.look_ahead(1, |t| t == &token::PathSep) && let token::Interpolated(nt) = &$self.token.kind - && let token::NtTy(ty) = &nt.0 + && let token::NtTy(ty) = &**nt { let ty = ty.clone(); $self.bump(); @@ -407,7 +407,7 @@ pub(super) fn token_descr(token: &Token) -> String { (Some(TokenDescription::Keyword), _) => Some("keyword"), (Some(TokenDescription::ReservedKeyword), _) => Some("reserved keyword"), (Some(TokenDescription::DocComment), _) => Some("doc comment"), - (None, TokenKind::Interpolated(node)) => Some(node.0.descr()), + (None, TokenKind::Interpolated(node)) => Some(node.descr()), (None, _) => None, }; @@ -708,7 +708,7 @@ impl<'a> Parser<'a> { fn check_inline_const(&self, dist: usize) -> bool { self.is_keyword_ahead(dist, &[kw::Const]) && self.look_ahead(dist + 1, |t| match &t.kind { - token::Interpolated(nt) => matches!(&nt.0, token::NtBlock(..)), + token::Interpolated(nt) => matches!(&**nt, token::NtBlock(..)), token::OpenDelim(Delimiter::Brace) => true, _ => false, }) @@ -1631,19 +1631,7 @@ pub enum FlatToken { // Metavar captures of various kinds. #[derive(Clone, Debug)] -pub enum ParseNtResult { +pub enum ParseNtResult { Tt(TokenTree), - Nt(NtType), -} - -impl ParseNtResult { - pub fn map_nt(self, mut f: F) -> ParseNtResult - where - F: FnMut(T) -> U, - { - match self { - ParseNtResult::Tt(tt) => ParseNtResult::Tt(tt), - ParseNtResult::Nt(nt) => ParseNtResult::Nt(f(nt)), - } - } + Nt(Lrc), } diff --git a/compiler/rustc_parse/src/parser/nonterminal.rs b/compiler/rustc_parse/src/parser/nonterminal.rs index 73b17353ac9..3a560382d32 100644 --- a/compiler/rustc_parse/src/parser/nonterminal.rs +++ b/compiler/rustc_parse/src/parser/nonterminal.rs @@ -1,7 +1,8 @@ use rustc_ast::ptr::P; -use rustc_ast::token::{self, Delimiter, Nonterminal, Nonterminal::*, NonterminalKind, Token}; +use rustc_ast::token::{self, Delimiter, Nonterminal::*, NonterminalKind, Token}; use rustc_ast::HasTokens; use rustc_ast_pretty::pprust; +use rustc_data_structures::sync::Lrc; use rustc_errors::PResult; use rustc_span::symbol::{kw, Ident}; @@ -54,7 +55,7 @@ impl<'a> Parser<'a> { }, NonterminalKind::Block => match &token.kind { token::OpenDelim(Delimiter::Brace) => true, - token::Interpolated(nt) => match &nt.0 { + token::Interpolated(nt) => match &**nt { NtBlock(_) | NtLifetime(_) | NtStmt(_) | NtExpr(_) | NtLiteral(_) => true, NtItem(_) | NtPat(_) | NtTy(_) | NtIdent(..) | NtMeta(_) | NtPath(_) | NtVis(_) => false, @@ -63,7 +64,7 @@ impl<'a> Parser<'a> { }, NonterminalKind::Path | NonterminalKind::Meta => match &token.kind { token::PathSep | token::Ident(..) => true, - token::Interpolated(nt) => may_be_ident(&nt.0), + token::Interpolated(nt) => may_be_ident(nt), _ => false, }, NonterminalKind::PatParam { .. } | NonterminalKind::PatWithOr => match &token.kind { @@ -81,13 +82,13 @@ impl<'a> Parser<'a> { token::BinOp(token::Shl) => true, // path (double UFCS) // leading vert `|` or-pattern token::BinOp(token::Or) => matches!(kind, NonterminalKind::PatWithOr), - token::Interpolated(nt) => may_be_ident(&nt.0), + token::Interpolated(nt) => may_be_ident(nt), _ => false, }, NonterminalKind::Lifetime => match &token.kind { token::Lifetime(_) => true, token::Interpolated(nt) => { - matches!(&nt.0, NtLifetime(_)) + matches!(&**nt, NtLifetime(_)) } _ => false, }, @@ -100,10 +101,7 @@ impl<'a> Parser<'a> { /// Parse a non-terminal (e.g. MBE `:pat` or `:ident`). Inlined because there is only one call /// site. #[inline] - pub fn parse_nonterminal( - &mut self, - kind: NonterminalKind, - ) -> PResult<'a, ParseNtResult> { + pub fn parse_nonterminal(&mut self, kind: NonterminalKind) -> PResult<'a, ParseNtResult> { // A `macro_rules!` invocation may pass a captured item/expr to a proc-macro, // which requires having captured tokens available. Since we cannot determine // in advance whether or not a proc-macro will be (transitively) invoked, @@ -196,7 +194,7 @@ impl<'a> Parser<'a> { ); } - Ok(ParseNtResult::Nt(nt)) + Ok(ParseNtResult::Nt(Lrc::new(nt))) } } diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index 78d3d019bf4..8af415f7c9d 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -757,7 +757,7 @@ impl<'a> Parser<'a> { // Make sure we don't allow e.g. `let mut $p;` where `$p:pat`. if let token::Interpolated(nt) = &self.token.kind { - if let token::NtPat(..) = &nt.0 { + if let token::NtPat(..) = &**nt { self.expected_ident_found_err().emit(); } } diff --git a/compiler/rustc_parse/src/parser/path.rs b/compiler/rustc_parse/src/parser/path.rs index 3636a357978..c37d3f0441d 100644 --- a/compiler/rustc_parse/src/parser/path.rs +++ b/compiler/rustc_parse/src/parser/path.rs @@ -193,7 +193,7 @@ impl<'a> Parser<'a> { maybe_whole!(self, NtPath, |path| reject_generics_if_mod_style(self, path.into_inner())); if let token::Interpolated(nt) = &self.token.kind { - if let token::NtTy(ty) = &nt.0 { + if let token::NtTy(ty) = &**nt { if let ast::TyKind::Path(None, path) = &ty.kind { let path = path.clone(); self.bump(); diff --git a/tests/ui/macros/nonterminal-matching.stderr b/tests/ui/macros/nonterminal-matching.stderr index 499f9a7637d..d19141145fa 100644 --- a/tests/ui/macros/nonterminal-matching.stderr +++ b/tests/ui/macros/nonterminal-matching.stderr @@ -1,8 +1,6 @@ error: no rules expected the token `enum E {}` --> $DIR/nonterminal-matching.rs:19:10 | -LL | macro complex_nonterminal($nt_item: item) { - | -------------- LL | macro n(a $nt_item b) { | --------------------- when calling this macro ... diff --git a/tests/ui/macros/trace_faulty_macros.stderr b/tests/ui/macros/trace_faulty_macros.stderr index 69607600b55..665dcc7d091 100644 --- a/tests/ui/macros/trace_faulty_macros.stderr +++ b/tests/ui/macros/trace_faulty_macros.stderr @@ -73,14 +73,10 @@ error: expected expression, found pattern `1 + 1` --> $DIR/trace_faulty_macros.rs:49:37 | LL | (let $p:pat = $e:expr) => {test!(($p,$e))}; - | ------- -- this is interpreted as expression, but it is expected to be pattern - | | - | this macro fragment matcher is expression + | -- this is interpreted as expression, but it is expected to be pattern ... LL | (($p:pat, $e:pat)) => {let $p = $e;}; - | ------ ^^ expected expression - | | - | this macro fragment matcher is pattern + | ^^ expected expression ... LL | test!(let x = 1+1); | ------------------ -- cgit 1.4.1-3-g733a5 From 95e519ecbf343c5246b2777b64d223fcd0f68b4c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 22 Apr 2024 19:46:51 +1000 Subject: Remove `NtIdent` and `NtLifetime`. The extra span is now recorded in the new `TokenKind::NtIdent` and `TokenKind::NtLifetime`. These both consist of a single token, and so there's no operator precedence problems with inserting them directly into the token stream. The other way to do this would be to wrap the ident/lifetime in invisible delimiters, but there's a lot of code that assumes an interpolated ident/lifetime fits in a single token, and changing all that code to work with invisible delimiters would have been a pain. (Maybe it could be done in a follow-up.) This change might not seem like much of a win, but it's a first step toward the much bigger and long-desired removal of `Nonterminal` and `TokenKind::Interpolated`. That change is big and complex enough that it's worth doing this piece separately. (Indeed, this commit is based on part of a late commit in #114647, a prior attempt at that big and complex change.) --- compiler/rustc_ast/src/ast_traits.rs | 2 - compiler/rustc_ast/src/mut_visit.rs | 8 ++- compiler/rustc_ast/src/token.rs | 77 +++++++++++--------------- compiler/rustc_ast/src/tokenstream.rs | 25 +++++---- compiler/rustc_ast_pretty/src/pprust/state.rs | 12 ++-- compiler/rustc_expand/src/config.rs | 15 +++-- compiler/rustc_expand/src/mbe/transcribe.rs | 10 ++++ compiler/rustc_expand/src/proc_macro_server.rs | 23 +++++--- compiler/rustc_parse/src/parser/expr.rs | 4 +- compiler/rustc_parse/src/parser/mod.rs | 6 ++ compiler/rustc_parse/src/parser/nonterminal.rs | 53 +++++++++--------- 11 files changed, 131 insertions(+), 104 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_ast/src/ast_traits.rs b/compiler/rustc_ast/src/ast_traits.rs index a0486227f2a..2cf811e9122 100644 --- a/compiler/rustc_ast/src/ast_traits.rs +++ b/compiler/rustc_ast/src/ast_traits.rs @@ -240,7 +240,6 @@ impl HasTokens for Nonterminal { Nonterminal::NtPath(path) => path.tokens(), Nonterminal::NtVis(vis) => vis.tokens(), Nonterminal::NtBlock(block) => block.tokens(), - Nonterminal::NtIdent(..) | Nonterminal::NtLifetime(..) => None, } } fn tokens_mut(&mut self) -> Option<&mut Option> { @@ -254,7 +253,6 @@ impl HasTokens for Nonterminal { Nonterminal::NtPath(path) => path.tokens_mut(), Nonterminal::NtVis(vis) => vis.tokens_mut(), Nonterminal::NtBlock(block) => block.tokens_mut(), - Nonterminal::NtIdent(..) | Nonterminal::NtLifetime(..) => None, } } } diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 07358047025..5c4162295bb 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -781,6 +781,12 @@ pub fn visit_token(t: &mut Token, vis: &mut T) { *span = ident.span; return; // Avoid visiting the span for the second time. } + token::NtIdent(ident, _is_raw) => { + vis.visit_ident(ident); + } + token::NtLifetime(ident) => { + vis.visit_ident(ident); + } token::Interpolated(nt) => { let nt = Lrc::make_mut(nt); visit_nonterminal(nt, vis); @@ -832,8 +838,6 @@ fn visit_nonterminal(nt: &mut token::Nonterminal, vis: &mut T) { token::NtPat(pat) => vis.visit_pat(pat), token::NtExpr(expr) => vis.visit_expr(expr), token::NtTy(ty) => vis.visit_ty(ty), - token::NtIdent(ident, _is_raw) => vis.visit_ident(ident), - token::NtLifetime(ident) => vis.visit_ident(ident), token::NtLiteral(expr) => vis.visit_expr(expr), token::NtMeta(item) => { let AttrItem { path, args, tokens } = item.deref_mut(); diff --git a/compiler/rustc_ast/src/token.rs b/compiler/rustc_ast/src/token.rs index 590c3daf0c2..d00352ea2e1 100644 --- a/compiler/rustc_ast/src/token.rs +++ b/compiler/rustc_ast/src/token.rs @@ -318,11 +318,20 @@ pub enum TokenKind { /// It's recommended to use `Token::(ident,uninterpolate,uninterpolated_span)` to /// treat regular and interpolated identifiers in the same way. Ident(Symbol, IdentIsRaw), + /// This identifier (and its span) is the identifier passed to the + /// declarative macro. The span in the surrounding `Token` is the span of + /// the `ident` metavariable in the macro's RHS. + NtIdent(Ident, IdentIsRaw), + /// Lifetime identifier token. /// Do not forget about `NtLifetime` when you want to match on lifetime identifiers. /// It's recommended to use `Token::(lifetime,uninterpolate,uninterpolated_span)` to /// treat regular and interpolated lifetime identifiers in the same way. Lifetime(Symbol), + /// This identifier (and its span) is the lifetime passed to the + /// declarative macro. The span in the surrounding `Token` is the span of + /// the `lifetime` metavariable in the macro's RHS. + NtLifetime(Ident), /// An embedded AST node, as produced by a macro. This only exists for /// historical reasons. We'd like to get rid of it, for multiple reasons. @@ -444,8 +453,9 @@ impl Token { /// Note that keywords are also identifiers, so they should use this /// if they keep spans or perform edition checks. pub fn uninterpolated_span(&self) -> Span { - match &self.kind { - Interpolated(nt) => nt.use_span(), + match self.kind { + NtIdent(ident, _) | NtLifetime(ident) => ident.span, + Interpolated(ref nt) => nt.use_span(), _ => self.span, } } @@ -463,7 +473,7 @@ impl Token { } OpenDelim(..) | CloseDelim(..) | Literal(..) | DocComment(..) | Ident(..) - | Lifetime(..) | Interpolated(..) | Eof => false, + | NtIdent(..) | Lifetime(..) | NtLifetime(..) | Interpolated(..) | Eof => false, } } @@ -613,14 +623,9 @@ impl Token { /// into the regular identifier or lifetime token it refers to, /// otherwise returns the original token. pub fn uninterpolate(&self) -> Cow<'_, Token> { - match &self.kind { - Interpolated(nt) => match &**nt { - NtIdent(ident, is_raw) => { - Cow::Owned(Token::new(Ident(ident.name, *is_raw), ident.span)) - } - NtLifetime(ident) => Cow::Owned(Token::new(Lifetime(ident.name), ident.span)), - _ => Cow::Borrowed(self), - }, + match self.kind { + NtIdent(ident, is_raw) => Cow::Owned(Token::new(Ident(ident.name, is_raw), ident.span)), + NtLifetime(ident) => Cow::Owned(Token::new(Lifetime(ident.name), ident.span)), _ => Cow::Borrowed(self), } } @@ -629,12 +634,9 @@ impl Token { #[inline] pub fn ident(&self) -> Option<(Ident, IdentIsRaw)> { // We avoid using `Token::uninterpolate` here because it's slow. - match &self.kind { - &Ident(name, is_raw) => Some((Ident::new(name, self.span), is_raw)), - Interpolated(nt) => match &**nt { - NtIdent(ident, is_raw) => Some((*ident, *is_raw)), - _ => None, - }, + match self.kind { + Ident(name, is_raw) => Some((Ident::new(name, self.span), is_raw)), + NtIdent(ident, is_raw) => Some((ident, is_raw)), _ => None, } } @@ -643,12 +645,9 @@ impl Token { #[inline] pub fn lifetime(&self) -> Option { // We avoid using `Token::uninterpolate` here because it's slow. - match &self.kind { - &Lifetime(name) => Some(Ident::new(name, self.span)), - Interpolated(nt) => match &**nt { - NtLifetime(ident) => Some(*ident), - _ => None, - }, + match self.kind { + Lifetime(name) => Some(Ident::new(name, self.span)), + NtLifetime(ident) => Some(ident), _ => None, } } @@ -837,8 +836,10 @@ impl Token { Le | EqEq | Ne | Ge | AndAnd | OrOr | Tilde | BinOpEq(..) | At | DotDotDot | DotDotEq | Comma | Semi | PathSep | RArrow | LArrow | FatArrow | Pound | Dollar - | Question | OpenDelim(..) | CloseDelim(..) | Literal(..) | Ident(..) - | Lifetime(..) | Interpolated(..) | DocComment(..) | Eof => return None, + | Question | OpenDelim(..) | CloseDelim(..) | Literal(..) | Ident(..) | NtIdent(..) + | Lifetime(..) | NtLifetime(..) | Interpolated(..) | DocComment(..) | Eof => { + return None; + } }; Some(Token::new(kind, self.span.to(joint.span))) @@ -861,9 +862,6 @@ pub enum Nonterminal { NtPat(P), NtExpr(P), NtTy(P), - /// The span is for the identifier argument passed to the macro. - NtIdent(Ident, IdentIsRaw), - NtLifetime(Ident), NtLiteral(P), /// Stuff inside brackets for attributes NtMeta(P), @@ -958,7 +956,6 @@ impl Nonterminal { NtPat(pat) => pat.span, NtExpr(expr) | NtLiteral(expr) => expr.span, NtTy(ty) => ty.span, - NtIdent(ident, _) | NtLifetime(ident) => ident.span, NtMeta(attr_item) => attr_item.span(), NtPath(path) => path.span, NtVis(vis) => vis.span, @@ -974,8 +971,6 @@ impl Nonterminal { NtExpr(..) => "expression", NtLiteral(..) => "literal", NtTy(..) => "type", - NtIdent(..) => "identifier", - NtLifetime(..) => "lifetime", NtMeta(..) => "attribute", NtPath(..) => "path", NtVis(..) => "visibility", @@ -984,18 +979,12 @@ impl Nonterminal { } impl PartialEq for Nonterminal { - fn eq(&self, rhs: &Self) -> bool { - match (self, rhs) { - (NtIdent(ident_lhs, is_raw_lhs), NtIdent(ident_rhs, is_raw_rhs)) => { - ident_lhs == ident_rhs && is_raw_lhs == is_raw_rhs - } - (NtLifetime(ident_lhs), NtLifetime(ident_rhs)) => ident_lhs == ident_rhs, - // FIXME: Assume that all "complex" nonterminal are not equal, we can't compare them - // correctly based on data from AST. This will prevent them from matching each other - // in macros. The comparison will become possible only when each nonterminal has an - // attached token stream from which it was parsed. - _ => false, - } + fn eq(&self, _rhs: &Self) -> bool { + // FIXME: Assume that all nonterminals are not equal, we can't compare them + // correctly based on data from AST. This will prevent them from matching each other + // in macros. The comparison will become possible only when each nonterminal has an + // attached token stream from which it was parsed. + false } } @@ -1008,12 +997,10 @@ impl fmt::Debug for Nonterminal { NtPat(..) => f.pad("NtPat(..)"), NtExpr(..) => f.pad("NtExpr(..)"), NtTy(..) => f.pad("NtTy(..)"), - NtIdent(..) => f.pad("NtIdent(..)"), NtLiteral(..) => f.pad("NtLiteral(..)"), NtMeta(..) => f.pad("NtMeta(..)"), NtPath(..) => f.pad("NtPath(..)"), NtVis(..) => f.pad("NtVis(..)"), - NtLifetime(..) => f.pad("NtLifetime(..)"), } } } diff --git a/compiler/rustc_ast/src/tokenstream.rs b/compiler/rustc_ast/src/tokenstream.rs index d532a884184..fb666550e93 100644 --- a/compiler/rustc_ast/src/tokenstream.rs +++ b/compiler/rustc_ast/src/tokenstream.rs @@ -466,12 +466,6 @@ impl TokenStream { pub fn from_nonterminal_ast(nt: &Nonterminal) -> TokenStream { match nt { - Nonterminal::NtIdent(ident, is_raw) => { - TokenStream::token_alone(token::Ident(ident.name, *is_raw), ident.span) - } - Nonterminal::NtLifetime(ident) => { - TokenStream::token_alone(token::Lifetime(ident.name), ident.span) - } Nonterminal::NtItem(item) => TokenStream::from_ast(item), Nonterminal::NtBlock(block) => TokenStream::from_ast(block), Nonterminal::NtStmt(stmt) if let StmtKind::Empty = stmt.kind => { @@ -489,11 +483,17 @@ impl TokenStream { } fn flatten_token(token: &Token, spacing: Spacing) -> TokenTree { - match &token.kind { - token::Interpolated(nt) if let token::NtIdent(ident, is_raw) = &**nt => { - TokenTree::Token(Token::new(token::Ident(ident.name, *is_raw), ident.span), spacing) + match token.kind { + token::NtIdent(ident, is_raw) => { + TokenTree::Token(Token::new(token::Ident(ident.name, is_raw), ident.span), spacing) } - token::Interpolated(nt) => TokenTree::Delimited( + token::NtLifetime(ident) => TokenTree::Delimited( + DelimSpan::from_single(token.span), + DelimSpacing::new(Spacing::JointHidden, spacing), + Delimiter::Invisible, + TokenStream::token_alone(token::Lifetime(ident.name), ident.span), + ), + token::Interpolated(ref nt) => TokenTree::Delimited( DelimSpan::from_single(token.span), DelimSpacing::new(Spacing::JointHidden, spacing), Delimiter::Invisible, @@ -516,7 +516,10 @@ impl TokenStream { pub fn flattened(&self) -> TokenStream { fn can_skip(stream: &TokenStream) -> bool { stream.trees().all(|tree| match tree { - TokenTree::Token(token, _) => !matches!(token.kind, token::Interpolated(_)), + TokenTree::Token(token, _) => !matches!( + token.kind, + token::NtIdent(..) | token::NtLifetime(..) | token::Interpolated(..) + ), TokenTree::Delimited(.., inner) => can_skip(inner), }) } diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index f438cee4cd2..e9cce4b3c34 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -852,8 +852,6 @@ pub trait PrintState<'a>: std::ops::Deref + std::ops::Dere token::NtBlock(e) => self.block_to_string(e), token::NtStmt(e) => self.stmt_to_string(e), token::NtPat(e) => self.pat_to_string(e), - &token::NtIdent(e, is_raw) => IdentPrinter::for_ast_ident(e, is_raw.into()).to_string(), - token::NtLifetime(e) => e.to_string(), token::NtLiteral(e) => self.expr_to_string(e), token::NtVis(e) => self.vis_to_string(e), } @@ -915,10 +913,14 @@ pub trait PrintState<'a>: std::ops::Deref + std::ops::Dere token::Literal(lit) => literal_to_string(lit).into(), /* Name components */ - token::Ident(s, is_raw) => { - IdentPrinter::new(s, is_raw.into(), convert_dollar_crate).to_string().into() + token::Ident(name, is_raw) => { + IdentPrinter::new(name, is_raw.into(), convert_dollar_crate).to_string().into() } - token::Lifetime(s) => s.to_string().into(), + token::NtIdent(ident, is_raw) => { + IdentPrinter::for_ast_ident(ident, is_raw.into()).to_string().into() + } + token::Lifetime(name) => name.to_string().into(), + token::NtLifetime(ident) => ident.name.to_string().into(), /* Other */ token::DocComment(comment_kind, attr_style, data) => { diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs index 897420a11cd..67a5a9128cc 100644 --- a/compiler/rustc_expand/src/config.rs +++ b/compiler/rustc_expand/src/config.rs @@ -201,10 +201,17 @@ impl<'a> StripUnconfigured<'a> { inner = self.configure_tokens(&inner); Some(AttrTokenTree::Delimited(sp, spacing, delim, inner)).into_iter() } - AttrTokenTree::Token(ref token, _) - if let TokenKind::Interpolated(nt) = &token.kind => - { - panic!("Nonterminal should have been flattened at {:?}: {:?}", token.span, nt); + AttrTokenTree::Token( + Token { + kind: + TokenKind::NtIdent(..) + | TokenKind::NtLifetime(..) + | TokenKind::Interpolated(..), + .. + }, + _, + ) => { + panic!("Nonterminal should have been flattened: {:?}", tree); } AttrTokenTree::Token(token, spacing) => { Some(AttrTokenTree::Token(token, spacing)).into_iter() diff --git a/compiler/rustc_expand/src/mbe/transcribe.rs b/compiler/rustc_expand/src/mbe/transcribe.rs index 011aa95c8a1..e35eba0f859 100644 --- a/compiler/rustc_expand/src/mbe/transcribe.rs +++ b/compiler/rustc_expand/src/mbe/transcribe.rs @@ -261,6 +261,16 @@ pub(super) fn transcribe<'a>( // without wrapping them into groups. maybe_use_metavar_location(cx, &stack, sp, tt, &mut marker) } + MatchedSingle(ParseNtResult::Ident(ident, is_raw)) => { + marker.visit_span(&mut sp); + let kind = token::NtIdent(*ident, *is_raw); + TokenTree::token_alone(kind, sp) + } + MatchedSingle(ParseNtResult::Lifetime(ident)) => { + marker.visit_span(&mut sp); + let kind = token::NtLifetime(*ident); + TokenTree::token_alone(kind, sp) + } MatchedSingle(ParseNtResult::Nt(nt)) => { // Other variables are emitted into the output stream as groups with // `Delimiter::Invisible` to maintain parsing priorities. diff --git a/compiler/rustc_expand/src/proc_macro_server.rs b/compiler/rustc_expand/src/proc_macro_server.rs index 8cf96580161..1f3547c841a 100644 --- a/compiler/rustc_expand/src/proc_macro_server.rs +++ b/compiler/rustc_expand/src/proc_macro_server.rs @@ -220,6 +220,12 @@ impl FromInternal<(TokenStream, &mut Rustc<'_, '_>)> for Vec { trees.push(TokenTree::Ident(Ident { sym, is_raw: is_raw.into(), span })) } + NtIdent(ident, is_raw) => trees.push(TokenTree::Ident(Ident { + sym: ident.name, + is_raw: is_raw.into(), + span: ident.span, + })), + Lifetime(name) => { let ident = symbol::Ident::new(name, span).without_first_quote(); trees.extend([ @@ -227,6 +233,15 @@ impl FromInternal<(TokenStream, &mut Rustc<'_, '_>)> for Vec { + let stream = TokenStream::token_alone(token::Lifetime(ident.name), ident.span); + trees.push(TokenTree::Group(Group { + delimiter: pm::Delimiter::None, + stream: Some(stream), + span: DelimSpan::from_single(span), + })) + } + Literal(token::Lit { kind, symbol, suffix }) => { trees.push(TokenTree::Literal(self::Literal { kind: FromInternal::from_internal(kind), @@ -259,14 +274,6 @@ impl FromInternal<(TokenStream, &mut Rustc<'_, '_>)> for Vec { - trees.push(TokenTree::Ident(Ident { - sym: ident.name, - is_raw: matches!(is_raw, IdentIsRaw::Yes), - span: ident.span, - })) - } - Interpolated(nt) => { let stream = TokenStream::from_nonterminal_ast(&nt); // A hack used to pass AST fragments to attribute and derive diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 8e3cd0f8ea9..cbc7ce9ef7c 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -724,7 +724,9 @@ impl<'a> Parser<'a> { /// Returns the span of expr if it was not interpolated, or the span of the interpolated token. fn interpolated_or_expr_span(&self, expr: &Expr) -> Span { match self.prev_token.kind { - TokenKind::Interpolated(..) => self.prev_token.span, + TokenKind::NtIdent(..) | TokenKind::NtLifetime(..) | TokenKind::Interpolated(..) => { + self.prev_token.span + } _ => expr.span, } } diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index cc40c8a7126..3e0a98a55ae 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -407,6 +407,8 @@ pub(super) fn token_descr(token: &Token) -> String { (Some(TokenDescription::Keyword), _) => Some("keyword"), (Some(TokenDescription::ReservedKeyword), _) => Some("reserved keyword"), (Some(TokenDescription::DocComment), _) => Some("doc comment"), + (None, TokenKind::NtIdent(..)) => Some("identifier"), + (None, TokenKind::NtLifetime(..)) => Some("lifetime"), (None, TokenKind::Interpolated(node)) => Some(node.descr()), (None, _) => None, }; @@ -1633,5 +1635,9 @@ pub enum FlatToken { #[derive(Clone, Debug)] pub enum ParseNtResult { Tt(TokenTree), + Ident(Ident, IdentIsRaw), + Lifetime(Ident), + + /// This case will eventually be removed, along with `Token::Interpolate`. Nt(Lrc), } diff --git a/compiler/rustc_parse/src/parser/nonterminal.rs b/compiler/rustc_parse/src/parser/nonterminal.rs index 3a560382d32..a6f0ab78b5c 100644 --- a/compiler/rustc_parse/src/parser/nonterminal.rs +++ b/compiler/rustc_parse/src/parser/nonterminal.rs @@ -25,15 +25,13 @@ impl<'a> Parser<'a> { | NtPat(_) | NtExpr(_) | NtTy(_) - | NtIdent(..) | NtLiteral(_) // `true`, `false` | NtMeta(_) | NtPath(_) => true, NtItem(_) | NtBlock(_) - | NtVis(_) - | NtLifetime(_) => false, + | NtVis(_) => false, } } @@ -50,25 +48,30 @@ impl<'a> Parser<'a> { NonterminalKind::Literal => token.can_begin_literal_maybe_minus(), NonterminalKind::Vis => match token.kind { // The follow-set of :vis + "priv" keyword + interpolated - token::Comma | token::Ident(..) | token::Interpolated(_) => true, + token::Comma + | token::Ident(..) + | token::NtIdent(..) + | token::NtLifetime(..) + | token::Interpolated(_) => true, _ => token.can_begin_type(), }, NonterminalKind::Block => match &token.kind { token::OpenDelim(Delimiter::Brace) => true, + token::NtLifetime(..) => true, token::Interpolated(nt) => match &**nt { - NtBlock(_) | NtLifetime(_) | NtStmt(_) | NtExpr(_) | NtLiteral(_) => true, - NtItem(_) | NtPat(_) | NtTy(_) | NtIdent(..) | NtMeta(_) | NtPath(_) - | NtVis(_) => false, + NtBlock(_) | NtStmt(_) | NtExpr(_) | NtLiteral(_) => true, + NtItem(_) | NtPat(_) | NtTy(_) | NtMeta(_) | NtPath(_) | NtVis(_) => false, }, _ => false, }, NonterminalKind::Path | NonterminalKind::Meta => match &token.kind { - token::PathSep | token::Ident(..) => true, + token::PathSep | token::Ident(..) | token::NtIdent(..) => true, token::Interpolated(nt) => may_be_ident(nt), _ => false, }, NonterminalKind::PatParam { .. } | NonterminalKind::PatWithOr => match &token.kind { - token::Ident(..) | // box, ref, mut, and other identifiers (can stricten) + // box, ref, mut, and other identifiers (can stricten) + token::Ident(..) | token::NtIdent(..) | token::OpenDelim(Delimiter::Parenthesis) | // tuple pattern token::OpenDelim(Delimiter::Bracket) | // slice pattern token::BinOp(token::And) | // reference @@ -86,10 +89,7 @@ impl<'a> Parser<'a> { _ => false, }, NonterminalKind::Lifetime => match &token.kind { - token::Lifetime(_) => true, - token::Interpolated(nt) => { - matches!(&**nt, NtLifetime(_)) - } + token::Lifetime(_) | token::NtLifetime(..) => true, _ => false, }, NonterminalKind::TT | NonterminalKind::Item | NonterminalKind::Stmt => { @@ -154,15 +154,16 @@ impl<'a> Parser<'a> { } // this could be handled like a token, since it is one - NonterminalKind::Ident if let Some((ident, is_raw)) = get_macro_ident(&self.token) => { - self.bump(); - NtIdent(ident, is_raw) - } NonterminalKind::Ident => { - return Err(self.dcx().create_err(UnexpectedNonterminal::Ident { - span: self.token.span, - token: self.token.clone(), - })); + return if let Some((ident, is_raw)) = get_macro_ident(&self.token) { + self.bump(); + Ok(ParseNtResult::Ident(ident, is_raw)) + } else { + Err(self.dcx().create_err(UnexpectedNonterminal::Ident { + span: self.token.span, + token: self.token.clone(), + })) + }; } NonterminalKind::Path => { NtPath(P(self.collect_tokens_no_attrs(|this| this.parse_path(PathStyle::Type))?)) @@ -173,14 +174,14 @@ impl<'a> Parser<'a> { .collect_tokens_no_attrs(|this| this.parse_visibility(FollowedByType::Yes))?)) } NonterminalKind::Lifetime => { - if self.check_lifetime() { - NtLifetime(self.expect_lifetime().ident) + return if self.check_lifetime() { + Ok(ParseNtResult::Lifetime(self.expect_lifetime().ident)) } else { - return Err(self.dcx().create_err(UnexpectedNonterminal::Lifetime { + Err(self.dcx().create_err(UnexpectedNonterminal::Lifetime { span: self.token.span, token: self.token.clone(), - })); - } + })) + }; } }; -- cgit 1.4.1-3-g733a5 From 1529c661e4f1f5294877a471e388d9097e742414 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 20 Apr 2024 11:22:06 -0400 Subject: Warn against redundant use<...> --- compiler/rustc_ast_lowering/src/lib.rs | 15 +- compiler/rustc_hir/src/hir.rs | 11 +- compiler/rustc_hir/src/intravisit.rs | 2 +- compiler/rustc_hir_analysis/src/check/check.rs | 2 +- compiler/rustc_lint/messages.ftl | 19 +- compiler/rustc_lint/src/impl_trait_overcaptures.rs | 207 ++++++++++++++------- compiler/rustc_parse/src/parser/ty.rs | 58 +++--- tests/ui/impl-trait/precise-capturing/apit.stderr | 2 +- tests/ui/impl-trait/precise-capturing/redundant.rs | 25 +++ .../impl-trait/precise-capturing/redundant.stderr | 45 +++++ 10 files changed, 272 insertions(+), 114 deletions(-) create mode 100644 tests/ui/impl-trait/precise-capturing/redundant.rs create mode 100644 tests/ui/impl-trait/precise-capturing/redundant.stderr (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index a76935761f0..c23da8aa01e 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -1407,7 +1407,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { bounds, fn_kind, itctx, - precise_capturing.as_deref().map(|(args, _)| args.as_slice()), + precise_capturing.as_deref().map(|(args, span)| (args.as_slice(), *span)), ), ImplTraitContext::Universal => { if let Some(&(_, span)) = precise_capturing.as_deref() { @@ -1523,7 +1523,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { bounds: &GenericBounds, fn_kind: Option, itctx: ImplTraitContext, - precise_capturing_args: Option<&[PreciseCapturingArg]>, + precise_capturing_args: Option<(&[PreciseCapturingArg], Span)>, ) -> hir::TyKind<'hir> { // Make sure we know that some funky desugaring has been going on here. // This is a first: there is code in other places like for loop @@ -1533,7 +1533,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::OpaqueTy, span, None); let captured_lifetimes_to_duplicate = - if let Some(precise_capturing) = precise_capturing_args { + if let Some((precise_capturing, _)) = precise_capturing_args { // We'll actually validate these later on; all we need is the list of // lifetimes to duplicate during this portion of lowering. precise_capturing @@ -1607,7 +1607,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { captured_lifetimes_to_duplicate: FxIndexSet, span: Span, opaque_ty_span: Span, - precise_capturing_args: Option<&[PreciseCapturingArg]>, + precise_capturing_args: Option<(&[PreciseCapturingArg], Span)>, lower_item_bounds: impl FnOnce(&mut Self) -> &'hir [hir::GenericBound<'hir>], ) -> hir::TyKind<'hir> { let opaque_ty_def_id = self.create_def( @@ -1698,8 +1698,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { this.with_remapping(captured_to_synthesized_mapping, |this| { ( lower_item_bounds(this), - precise_capturing_args.map(|precise_capturing| { - this.lower_precise_capturing_args(precise_capturing) + precise_capturing_args.map(|(precise_capturing, span)| { + ( + this.lower_precise_capturing_args(precise_capturing), + this.lower_span(span), + ) }), ) }); diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 7d991e21ff3..6e4cef068c5 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2631,7 +2631,7 @@ pub struct OpaqueTy<'hir> { /// lowered as an associated type. pub in_trait: bool, /// List of arguments captured via `impl use<'a, P, ...> Trait` syntax. - pub precise_capturing_args: Option<&'hir [PreciseCapturingArg<'hir>]>, + pub precise_capturing_args: Option<(&'hir [PreciseCapturingArg<'hir>], Span)>, } #[derive(Debug, Clone, Copy, HashStable_Generic)] @@ -2641,6 +2641,15 @@ pub enum PreciseCapturingArg<'hir> { Param(PreciseCapturingNonLifetimeArg), } +impl PreciseCapturingArg<'_> { + pub fn hir_id(self) -> HirId { + match self { + PreciseCapturingArg::Lifetime(lt) => lt.hir_id, + PreciseCapturingArg::Param(param) => param.hir_id, + } + } +} + /// We need to have a [`Node`] for the [`HirId`] that we attach the type/const param /// resolution to. Lifetimes don't have this problem, and for them, it's actually /// kind of detrimental to use a custom node type versus just using [`Lifetime`], diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 0b095db953b..664784cd2c6 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -533,7 +533,7 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item<'v>) -> V:: try_visit!(visitor.visit_id(item.hir_id())); try_visit!(walk_generics(visitor, generics)); walk_list!(visitor, visit_param_bound, bounds); - if let Some(precise_capturing_args) = precise_capturing_args { + if let Some((precise_capturing_args, _)) = precise_capturing_args { for arg in precise_capturing_args { try_visit!(visitor.visit_precise_capturing_arg(arg)); } diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 652c1885073..b5c06751405 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -486,7 +486,7 @@ fn sanity_check_found_hidden_type<'tcx>( fn check_opaque_precise_captures<'tcx>(tcx: TyCtxt<'tcx>, opaque_def_id: LocalDefId) { let hir::OpaqueTy { precise_capturing_args, .. } = *tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty(); - let Some(precise_capturing_args) = precise_capturing_args else { + let Some((precise_capturing_args, _)) = precise_capturing_args else { // No precise capturing args; nothing to validate return; }; diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index d54e9a3c6eb..5180fce2eb3 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -9,14 +9,6 @@ lint_array_into_iter = .use_explicit_into_iter_suggestion = or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value -lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than possibly intended in edition 2024 - .note = specifically, {$num_captured -> - [one] this lifetime is - *[other] these lifetimes are - } in scope but not mentioned in the type's bounds - .note2 = all lifetimes in scope will be captured by `impl Trait`s in edition 2024 - .suggestion = use the precise capturing `use<...>` syntax to make the captures explicit - lint_async_fn_in_trait = use of `async fn` in public traits is discouraged as auto trait bounds cannot be specified .note = you can suppress this lint if you plan to use the trait only in your own code, or do not care about auto traits like `Send` on the `Future` .suggestion = you can alternatively desugar to a normal `fn` that returns `impl Future` and add any desired bounds such as `Send`, but these cannot be relaxed without a breaking API change @@ -277,6 +269,17 @@ lint_identifier_uncommon_codepoints = identifier contains {$codepoints_len -> lint_ignored_unless_crate_specified = {$level}({$name}) is ignored unless specified at crate level +lint_impl_trait_overcaptures = `{$self_ty}` will capture more lifetimes than possibly intended in edition 2024 + .note = specifically, {$num_captured -> + [one] this lifetime is + *[other] these lifetimes are + } in scope but not mentioned in the type's bounds + .note2 = all lifetimes in scope will be captured by `impl Trait`s in edition 2024 + .suggestion = use the precise capturing `use<...>` syntax to make the captures explicit + +lint_impl_trait_redundant_captures = all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + .suggestion = remove the `use<...>` syntax + lint_improper_ctypes = `extern` {$desc} uses type `{$ty}`, which is not FFI-safe .label = not FFI-safe .note = the type is defined here diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 7659b6da181..a239f38006b 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -1,9 +1,11 @@ use rustc_data_structures::fx::FxIndexSet; +use rustc_data_structures::unord::UnordSet; use rustc_errors::{Applicability, LintDiagnostic}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; -use rustc_hir::intravisit; +use rustc_macros::LintDiagnostic; +use rustc_middle::middle::resolve_bound_vars::ResolvedArg; use rustc_middle::ty::{ self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, }; @@ -14,10 +16,12 @@ use rustc_span::{BytePos, Span}; use crate::fluent_generated as fluent; use crate::{LateContext, LateLintPass}; +// TODO: feature gate these too + declare_lint! { /// UwU pub IMPL_TRAIT_OVERCAPTURES, - Warn, + Allow, "will capture more lifetimes than possibly intended in edition 2024", @future_incompatible = FutureIncompatibleInfo { reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024), @@ -25,66 +29,64 @@ declare_lint! { }; } +declare_lint! { + /// UwU + pub IMPL_TRAIT_REDUNDANT_CAPTURES, + Warn, + "uwu 2" +} + declare_lint_pass!( /// Lint for opaque types that will begin capturing in-scope but unmentioned lifetimes /// in edition 2024. - ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES] + ImplTraitOvercaptures => [IMPL_TRAIT_OVERCAPTURES, IMPL_TRAIT_REDUNDANT_CAPTURES] ); impl<'tcx> LateLintPass<'tcx> for ImplTraitOvercaptures { - fn check_fn( - &mut self, - cx: &LateContext<'tcx>, - _: intravisit::FnKind<'tcx>, - _: &'tcx hir::FnDecl<'tcx>, - _: &'tcx hir::Body<'tcx>, - _: Span, - parent_def_id: LocalDefId, - ) { - match cx.tcx.def_kind(parent_def_id) { - DefKind::AssocFn => { - // RPITITs already capture all lifetimes in scope, so skip them. - if matches!( - cx.tcx.def_kind(cx.tcx.local_parent(parent_def_id)), - DefKind::Trait | DefKind::Impl { of_trait: true } - ) { - return; - } - } - DefKind::Fn => { - // All free functions need to check for overcaptures. - } - DefKind::Closure => return, - kind => { - unreachable!( - "expected function item, found {}", - kind.descr(parent_def_id.to_def_id()) - ) - } + fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::Item<'tcx>) { + match &it.kind { + hir::ItemKind::Fn(..) => check_fn(cx.tcx, it.owner_id.def_id), + _ => {} } + } - let sig = cx.tcx.fn_sig(parent_def_id).instantiate_identity(); + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::ImplItem<'tcx>) { + match &it.kind { + hir::ImplItemKind::Fn(_, _) => check_fn(cx.tcx, it.owner_id.def_id), + _ => {} + } + } - let mut in_scope_parameters = FxIndexSet::default(); - // Populate the in_scope_parameters list first with all of the generics in scope - let mut current_def_id = Some(parent_def_id.to_def_id()); - while let Some(def_id) = current_def_id { - let generics = cx.tcx.generics_of(def_id); - for param in &generics.params { - in_scope_parameters.insert(param.def_id); - } - current_def_id = generics.parent; + fn check_trait_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx hir::TraitItem<'tcx>) { + match &it.kind { + hir::TraitItemKind::Fn(_, _) => check_fn(cx.tcx, it.owner_id.def_id), + _ => {} } + } +} - // Then visit the signature to walk through all the binders (incl. the late-bound - // vars on the function itself, which we need to count too). - sig.visit_with(&mut VisitOpaqueTypes { - tcx: cx.tcx, - parent_def_id, - in_scope_parameters, - seen: Default::default(), - }); +fn check_fn(tcx: TyCtxt<'_>, parent_def_id: LocalDefId) { + let sig = tcx.fn_sig(parent_def_id).instantiate_identity(); + + let mut in_scope_parameters = FxIndexSet::default(); + // Populate the in_scope_parameters list first with all of the generics in scope + let mut current_def_id = Some(parent_def_id.to_def_id()); + while let Some(def_id) = current_def_id { + let generics = tcx.generics_of(def_id); + for param in &generics.own_params { + in_scope_parameters.insert(param.def_id); + } + current_def_id = generics.parent; } + + // Then visit the signature to walk through all the binders (incl. the late-bound + // vars on the function itself, which we need to count too). + sig.visit_with(&mut VisitOpaqueTypes { + tcx, + parent_def_id, + in_scope_parameters, + seen: Default::default(), + }); } struct VisitOpaqueTypes<'tcx> { @@ -109,14 +111,11 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { let unique = self.in_scope_parameters.insert(def_id); assert!(unique); } - ty::BoundVariableKind::Ty(_) => { - todo!("we don't support late-bound type params in `impl Trait`") - } - ty::BoundVariableKind::Region(..) => { - unreachable!("all AST-derived bound regions should have a name") - } - ty::BoundVariableKind::Const => { - unreachable!("non-lifetime binder consts are not allowed") + _ => { + self.tcx.dcx().span_delayed_bug( + self.tcx.def_span(self.parent_def_id), + format!("unsupported bound variable kind: {arg:?}"), + ); } } } @@ -131,11 +130,19 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { } fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { - if !t.has_opaque_types() { + if !t.has_aliases() { return; } - if let ty::Alias(ty::Opaque, opaque_ty) = *t.kind() + if let ty::Alias(ty::Projection, opaque_ty) = *t.kind() + && self.tcx.is_impl_trait_in_trait(opaque_ty.def_id) + { + // visit the opaque of the RPITIT + self.tcx + .type_of(opaque_ty.def_id) + .instantiate(self.tcx, opaque_ty.args) + .visit_with(self) + } else if let ty::Alias(ty::Opaque, opaque_ty) = *t.kind() && let Some(opaque_def_id) = opaque_ty.def_id.as_local() // Don't recurse infinitely on an opaque && self.seen.insert(opaque_def_id) @@ -144,8 +151,6 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { self.tcx.hir_node_by_def_id(opaque_def_id).expect_item().expect_opaque_ty() && let hir::OpaqueTyOrigin::FnReturn(parent_def_id) = opaque.origin && parent_def_id == self.parent_def_id - // And if the opaque doesn't already have `use<>` syntax on it... - && opaque.precise_capturing_args.is_none() { // Compute the set of args that are captured by the opaque... let mut captured = FxIndexSet::default(); @@ -178,9 +183,16 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { .map(|def_id| self.tcx.def_span(def_id)) .collect(); - if !uncaptured_spans.is_empty() { - let opaque_span = self.tcx.def_span(opaque_def_id); + let opaque_span = self.tcx.def_span(opaque_def_id); + let new_capture_rules = + opaque_span.at_least_rust_2024() || self.tcx.features().lifetime_capture_rules_2024; + // If we have uncaptured args, and if the opaque doesn't already have + // `use<>` syntax on it, and we're < edition 2024, then warn the user. + if !new_capture_rules + && opaque.precise_capturing_args.is_none() + && !uncaptured_spans.is_empty() + { let suggestion = if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(opaque_span) && snippet.starts_with("impl ") @@ -207,11 +219,11 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { None }; - self.tcx.emit_node_lint( + self.tcx.emit_node_span_lint( IMPL_TRAIT_OVERCAPTURES, self.tcx.local_def_id_to_hir_id(opaque_def_id), + opaque_span, ImplTraitOvercapturesLint { - opaque_span, self_ty: t, num_captured: uncaptured_spans.len(), uncaptured_spans, @@ -219,6 +231,60 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { }, ); } + // Otherwise, if we are edition 2024, have `use<>` syntax, and + // have no uncaptured args, then we should warn to the user that + // it's redundant to capture all args explicitly. + else if new_capture_rules + && let Some((captured_args, capturing_span)) = opaque.precise_capturing_args + { + let mut explicitly_captured = UnordSet::default(); + for arg in captured_args { + match self.tcx.named_bound_var(arg.hir_id()) { + Some( + ResolvedArg::EarlyBound(def_id) | ResolvedArg::LateBound(_, _, def_id), + ) => { + if self.tcx.def_kind(self.tcx.parent(def_id)) == DefKind::OpaqueTy { + let (ty::ReEarlyParam(ty::EarlyParamRegion { def_id, .. }) + | ty::ReLateParam(ty::LateParamRegion { + bound_region: ty::BoundRegionKind::BrNamed(def_id, _), + .. + })) = self + .tcx + .map_opaque_lifetime_to_parent_lifetime(def_id.expect_local()) + .kind() + else { + span_bug!( + self.tcx.def_span(def_id), + "variable should have been duplicated from a parent" + ); + }; + explicitly_captured.insert(def_id); + } else { + explicitly_captured.insert(def_id); + } + } + _ => { + self.tcx.dcx().span_delayed_bug( + self.tcx().hir().span(arg.hir_id()), + "no valid for captured arg", + ); + } + } + } + + if self + .in_scope_parameters + .iter() + .all(|def_id| explicitly_captured.contains(def_id)) + { + self.tcx.emit_node_span_lint( + IMPL_TRAIT_REDUNDANT_CAPTURES, + self.tcx.local_def_id_to_hir_id(opaque_def_id), + opaque_span, + ImplTraitRedundantCapturesLint { capturing_span }, + ); + } + } // Walk into the bounds of the opaque, too, since we want to get nested opaques // in this lint as well. Interestingly, one place that I expect this lint to fire @@ -236,7 +302,6 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { } struct ImplTraitOvercapturesLint<'tcx> { - opaque_span: Span, uncaptured_spans: Vec, self_ty: Ty<'tcx>, num_captured: usize, @@ -247,7 +312,6 @@ impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> { fn decorate_lint<'b>(self, diag: &'b mut rustc_errors::Diag<'a, ()>) { diag.arg("self_ty", self.self_ty.to_string()) .arg("num_captured", self.num_captured) - .span(self.opaque_span) .span_note(self.uncaptured_spans, fluent::lint_note) .note(fluent::lint_note2); if let Some((suggestion, span)) = self.suggestion { @@ -265,6 +329,13 @@ impl<'a> LintDiagnostic<'a, ()> for ImplTraitOvercapturesLint<'_> { } } +#[derive(LintDiagnostic)] +#[diag(lint_impl_trait_redundant_captures)] +struct ImplTraitRedundantCapturesLint { + #[suggestion(lint_suggestion, code = "", applicability = "machine-applicable")] + capturing_span: Span, +} + fn extract_def_id_from_arg<'tcx>( tcx: TyCtxt<'tcx>, generics: &'tcx ty::Generics, diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 7096b201f84..2f08a48c7bc 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -675,8 +675,8 @@ impl<'a> Parser<'a> { let precise_capturing = if self.eat_keyword(kw::Use) { let use_span = self.prev_token.span; self.psess.gated_spans.gate(sym::precise_capturing, use_span); - let args = self.parse_precise_capturing_args()?; - Some(P((args, use_span))) + let (args, args_span) = self.parse_precise_capturing_args()?; + Some(P((args, use_span.to(args_span)))) } else { None }; @@ -689,32 +689,34 @@ impl<'a> Parser<'a> { Ok(TyKind::ImplTrait(ast::DUMMY_NODE_ID, bounds, precise_capturing)) } - fn parse_precise_capturing_args(&mut self) -> PResult<'a, ThinVec> { - Ok(self - .parse_unspanned_seq( - &TokenKind::Lt, - &TokenKind::Gt, - SeqSep::trailing_allowed(token::Comma), - |self_| { - if self_.check_keyword(kw::SelfUpper) { - self_.bump(); - Ok(PreciseCapturingArg::Arg( - ast::Path::from_ident(self_.prev_token.ident().unwrap().0), - DUMMY_NODE_ID, - )) - } else if self_.check_ident() { - Ok(PreciseCapturingArg::Arg( - ast::Path::from_ident(self_.parse_ident()?), - DUMMY_NODE_ID, - )) - } else if self_.check_lifetime() { - Ok(PreciseCapturingArg::Lifetime(self_.expect_lifetime())) - } else { - self_.unexpected_any() - } - }, - )? - .0) + fn parse_precise_capturing_args( + &mut self, + ) -> PResult<'a, (ThinVec, Span)> { + let lo = self.token.span; + let (args, _) = self.parse_unspanned_seq( + &TokenKind::Lt, + &TokenKind::Gt, + SeqSep::trailing_allowed(token::Comma), + |self_| { + if self_.check_keyword(kw::SelfUpper) { + self_.bump(); + Ok(PreciseCapturingArg::Arg( + ast::Path::from_ident(self_.prev_token.ident().unwrap().0), + DUMMY_NODE_ID, + )) + } else if self_.check_ident() { + Ok(PreciseCapturingArg::Arg( + ast::Path::from_ident(self_.parse_ident()?), + DUMMY_NODE_ID, + )) + } else if self_.check_lifetime() { + Ok(PreciseCapturingArg::Lifetime(self_.expect_lifetime())) + } else { + self_.unexpected_any() + } + }, + )?; + Ok((args, lo.to(self.prev_token.span))) } /// Is a `dyn B0 + ... + Bn` type allowed here? diff --git a/tests/ui/impl-trait/precise-capturing/apit.stderr b/tests/ui/impl-trait/precise-capturing/apit.stderr index 36bf80d9e2f..96548f5732f 100644 --- a/tests/ui/impl-trait/precise-capturing/apit.stderr +++ b/tests/ui/impl-trait/precise-capturing/apit.stderr @@ -11,7 +11,7 @@ error: `use<...>` precise capturing syntax not allowed on argument-position `imp --> $DIR/apit.rs:4:18 | LL | fn hello(_: impl use<> Sized) {} - | ^^^ + | ^^^^^ error: aborting due to 1 previous error; 1 warning emitted diff --git a/tests/ui/impl-trait/precise-capturing/redundant.rs b/tests/ui/impl-trait/precise-capturing/redundant.rs new file mode 100644 index 00000000000..108a4cb64aa --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/redundant.rs @@ -0,0 +1,25 @@ +//@ compile-flags: -Zunstable-options --edition=2024 +//@ check-pass + +#![feature(precise_capturing)] +//~^ WARN the feature `precise_capturing` is incomplete + +fn hello<'a>() -> impl use<'a> Sized {} +//~^ WARN all possible in-scope parameters are already captured + +struct Inherent; +impl Inherent { + fn inherent(&self) -> impl use<'_> Sized {} + //~^ WARN all possible in-scope parameters are already captured +} + +trait Test<'a> { + fn in_trait() -> impl use<'a, Self> Sized; + //~^ WARN all possible in-scope parameters are already captured +} +impl<'a> Test<'a> for () { + fn in_trait() -> impl use<'a> Sized {} + //~^ WARN all possible in-scope parameters are already captured +} + +fn main() {} diff --git a/tests/ui/impl-trait/precise-capturing/redundant.stderr b/tests/ui/impl-trait/precise-capturing/redundant.stderr new file mode 100644 index 00000000000..325f04d3536 --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/redundant.stderr @@ -0,0 +1,45 @@ +warning: the feature `precise_capturing` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/redundant.rs:4:12 + | +LL | #![feature(precise_capturing)] + | ^^^^^^^^^^^^^^^^^ + | + = note: see issue #123432 for more information + = note: `#[warn(incomplete_features)]` on by default + +warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + --> $DIR/redundant.rs:7:19 + | +LL | fn hello<'a>() -> impl use<'a> Sized {} + | ^^^^^-------^^^^^^ + | | + | help: remove the `use<...>` syntax + | + = note: `#[warn(impl_trait_redundant_captures)]` on by default + +warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + --> $DIR/redundant.rs:12:27 + | +LL | fn inherent(&self) -> impl use<'_> Sized {} + | ^^^^^-------^^^^^^ + | | + | help: remove the `use<...>` syntax + +warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + --> $DIR/redundant.rs:17:22 + | +LL | fn in_trait() -> impl use<'a, Self> Sized; + | ^^^^^-------------^^^^^^ + | | + | help: remove the `use<...>` syntax + +warning: all possible in-scope parameters are already captured, so `use<...>` syntax is redundant + --> $DIR/redundant.rs:21:22 + | +LL | fn in_trait() -> impl use<'a> Sized {} + | ^^^^^-------^^^^^^ + | | + | help: remove the `use<...>` syntax + +warning: 5 warnings emitted + -- cgit 1.4.1-3-g733a5 From 8dc6a5d145af86955402ec02907dda77dcb4a122 Mon Sep 17 00:00:00 2001 From: ardi Date: Tue, 14 May 2024 12:23:33 +0200 Subject: improve maybe_consume_incorrect_semicolon --- compiler/rustc_ast/src/ast.rs | 1 + compiler/rustc_parse/src/errors.rs | 2 +- compiler/rustc_parse/src/parser/diagnostics.rs | 43 ++++++++++------------ compiler/rustc_parse/src/parser/item.rs | 4 +- src/librustdoc/doctest.rs | 4 +- .../recover-from-semicolon-trailing-item.stderr | 4 ++ 6 files changed, 30 insertions(+), 28 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 7951b7e7b75..56d4f53f9ee 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -3250,6 +3250,7 @@ pub enum ItemKind { } impl ItemKind { + /// "a" or "an" pub fn article(&self) -> &'static str { use ItemKind::*; match self { diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 2f68a299f26..3f08a830b0c 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -83,7 +83,7 @@ pub(crate) struct IncorrectSemicolon<'a> { #[suggestion(style = "short", code = "", applicability = "machine-applicable")] pub span: Span, #[help] - pub opt_help: Option<()>, + pub show_help: bool, pub name: &'a str, } diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 63762f64be9..46809847fdb 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1817,34 +1817,31 @@ impl<'a> Parser<'a> { Ok(P(T::recovered(Some(P(QSelf { ty, path_span, position: 0 })), path))) } - pub fn maybe_consume_incorrect_semicolon(&mut self, items: &[P]) -> bool { - if self.token.kind == TokenKind::Semi { - self.bump(); - - let mut err = - IncorrectSemicolon { span: self.prev_token.span, opt_help: None, name: "" }; + /// This function gets called in places where a semicolon is NOT expected and if there's a + /// semicolon it emits the appropriate error and returns true. + pub fn maybe_consume_incorrect_semicolon(&mut self, previous_item: Option<&Item>) -> bool { + if self.token.kind != TokenKind::Semi { + return false; + } - if !items.is_empty() { - let previous_item = &items[items.len() - 1]; - let previous_item_kind_name = match previous_item.kind { + // Check previous item to add it to the diagnostic, for example to say + // `enum declarations are not followed by a semicolon` + let err = match previous_item { + Some(previous_item) => { + let name = match previous_item.kind { // Say "braced struct" because tuple-structs and // braceless-empty-struct declarations do take a semicolon. - ItemKind::Struct(..) => Some("braced struct"), - ItemKind::Enum(..) => Some("enum"), - ItemKind::Trait(..) => Some("trait"), - ItemKind::Union(..) => Some("union"), - _ => None, + ItemKind::Struct(..) => "braced struct", + _ => previous_item.kind.descr(), }; - if let Some(name) = previous_item_kind_name { - err.opt_help = Some(()); - err.name = name; - } + IncorrectSemicolon { span: self.token.span, name, show_help: true } } - self.dcx().emit_err(err); - true - } else { - false - } + None => IncorrectSemicolon { span: self.token.span, name: "", show_help: false }, + }; + self.dcx().emit_err(err); + + self.bump(); + true } /// Creates a `Diag` for an unexpected token `t` and tries to recover if it is a diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 29957bd2f71..2a82a95dd07 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -59,13 +59,13 @@ impl<'a> Parser<'a> { let post_attr_lo = self.token.span; let mut items = ThinVec::new(); while let Some(item) = self.parse_item(ForceCollect::No)? { + self.maybe_consume_incorrect_semicolon(Some(&item)); items.push(item); - self.maybe_consume_incorrect_semicolon(&items); } if !self.eat(term) { let token_str = super::token_descr(&self.token); - if !self.maybe_consume_incorrect_semicolon(&items) { + if !self.maybe_consume_incorrect_semicolon(items.last().map(|x| &**x)) { let msg = format!("expected item, found {token_str}"); let mut err = self.dcx().struct_span_err(self.token.span, msg); let span = self.token.span; diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 0ad4c9c2346..19aa5ef949e 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -686,9 +686,9 @@ pub(crate) fn make_test( } } - // The supplied slice is only used for diagnostics, + // The supplied item is only used for diagnostics, // which are swallowed here anyway. - parser.maybe_consume_incorrect_semicolon(&[]); + parser.maybe_consume_incorrect_semicolon(None); } // Reset errors so that they won't be reported as compiler bugs when dropping the diff --git a/tests/ui/suggestions/recover-from-semicolon-trailing-item.stderr b/tests/ui/suggestions/recover-from-semicolon-trailing-item.stderr index fee83eb5c18..e068fdb5aba 100644 --- a/tests/ui/suggestions/recover-from-semicolon-trailing-item.stderr +++ b/tests/ui/suggestions/recover-from-semicolon-trailing-item.stderr @@ -3,6 +3,8 @@ error: expected item, found `;` | LL | mod M {}; | ^ help: remove this semicolon + | + = help: module declarations are not followed by a semicolon error: expected item, found `;` --> $DIR/recover-from-semicolon-trailing-item.rs:4:12 @@ -17,6 +19,8 @@ error: expected item, found `;` | LL | fn foo(a: usize) {}; | ^ help: remove this semicolon + | + = help: function declarations are not followed by a semicolon error[E0308]: mismatched types --> $DIR/recover-from-semicolon-trailing-item.rs:10:20 -- cgit 1.4.1-3-g733a5 From 1f6d271527cd2ae25b586277d02ab71c1332f57c Mon Sep 17 00:00:00 2001 From: ardi Date: Wed, 8 May 2024 14:22:09 +0200 Subject: Clarify that the diff_marker is talking about version control system conflicts specifically and a few more improvements. --- compiler/rustc_parse/src/lexer/tokentrees.rs | 2 +- compiler/rustc_parse/src/parser/diagnostics.rs | 33 +++++++++++++------ compiler/rustc_parse/src/parser/expr.rs | 2 +- compiler/rustc_parse/src/parser/item.rs | 44 ++++++++++++-------------- compiler/rustc_parse/src/parser/stmt.rs | 4 +-- 5 files changed, 47 insertions(+), 38 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index b5a5a2a90ee..eabe0226b2f 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -241,7 +241,7 @@ impl<'psess, 'src> TokenTreesReader<'psess, 'src> { // we have no way of tracking this in the lexer itself, so we piggyback on the parser let mut in_cond = false; while parser.token != token::Eof { - if let Err(diff_err) = parser.err_diff_marker() { + if let Err(diff_err) = parser.err_vcs_conflict_marker() { diff_errs.push(diff_err); } else if parser.is_keyword_ahead(0, &[kw::If, kw::While]) { in_cond = true; diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 46809847fdb..ac12787f2ef 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -2954,13 +2954,23 @@ impl<'a> Parser<'a> { err } - pub fn is_diff_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> bool { + /// This checks if this is a conflict marker, depending of the parameter passed. + /// + /// * `>>>>>` + /// * `=====` + /// * `<<<<<` + /// + pub fn is_vcs_conflict_marker( + &mut self, + long_kind: &TokenKind, + short_kind: &TokenKind, + ) -> bool { (0..3).all(|i| self.look_ahead(i, |tok| tok == long_kind)) && self.look_ahead(3, |tok| tok == short_kind) } - fn diff_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> Option { - if self.is_diff_marker(long_kind, short_kind) { + fn conflict_marker(&mut self, long_kind: &TokenKind, short_kind: &TokenKind) -> Option { + if self.is_vcs_conflict_marker(long_kind, short_kind) { let lo = self.token.span; for _ in 0..4 { self.bump(); @@ -2970,15 +2980,16 @@ impl<'a> Parser<'a> { None } - pub fn recover_diff_marker(&mut self) { - if let Err(err) = self.err_diff_marker() { + pub fn recover_vcs_conflict_marker(&mut self) { + if let Err(err) = self.err_vcs_conflict_marker() { err.emit(); FatalError.raise(); } } - pub fn err_diff_marker(&mut self) -> PResult<'a, ()> { - let Some(start) = self.diff_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) else { + pub fn err_vcs_conflict_marker(&mut self) -> PResult<'a, ()> { + let Some(start) = self.conflict_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) + else { return Ok(()); }; let mut spans = Vec::with_capacity(3); @@ -2990,13 +3001,15 @@ impl<'a> Parser<'a> { if self.token.kind == TokenKind::Eof { break; } - if let Some(span) = self.diff_marker(&TokenKind::OrOr, &TokenKind::BinOp(token::Or)) { + if let Some(span) = self.conflict_marker(&TokenKind::OrOr, &TokenKind::BinOp(token::Or)) + { middlediff3 = Some(span); } - if let Some(span) = self.diff_marker(&TokenKind::EqEq, &TokenKind::Eq) { + if let Some(span) = self.conflict_marker(&TokenKind::EqEq, &TokenKind::Eq) { middle = Some(span); } - if let Some(span) = self.diff_marker(&TokenKind::BinOp(token::Shr), &TokenKind::Gt) { + if let Some(span) = self.conflict_marker(&TokenKind::BinOp(token::Shr), &TokenKind::Gt) + { spans.push(span); end = Some(span); break; diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index cbc7ce9ef7c..d2d21624150 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -3734,7 +3734,7 @@ impl<'a> Parser<'a> { /// Parses `ident (COLON expr)?`. fn parse_expr_field(&mut self) -> PResult<'a, ExprField> { let attrs = self.parse_outer_attributes()?; - self.recover_diff_marker(); + self.recover_vcs_conflict_marker(); self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| { let lo = this.token.span; diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 2a82a95dd07..05b0e0fe52d 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -49,6 +49,7 @@ impl<'a> Parser<'a> { } /// Parses the contents of a module (inner attributes followed by module items). + /// We exit once we hit `term` pub fn parse_mod( &mut self, term: &TokenKind, @@ -101,9 +102,9 @@ impl<'a> Parser<'a> { fn_parse_mode: FnParseMode, force_collect: ForceCollect, ) -> PResult<'a, Option> { - self.recover_diff_marker(); + self.recover_vcs_conflict_marker(); let attrs = self.parse_outer_attributes()?; - self.recover_diff_marker(); + self.recover_vcs_conflict_marker(); self.parse_item_common(attrs, true, false, fn_parse_mode, force_collect) } @@ -723,7 +724,7 @@ impl<'a> Parser<'a> { if self.recover_doc_comment_before_brace() { continue; } - self.recover_diff_marker(); + self.recover_vcs_conflict_marker(); match parse_item(self) { Ok(None) => { let mut is_unnecessary_semicolon = !items.is_empty() @@ -1070,7 +1071,7 @@ impl<'a> Parser<'a> { /// ``` fn parse_use_tree_list(&mut self) -> PResult<'a, ThinVec<(UseTree, ast::NodeId)>> { self.parse_delim_comma_seq(Delimiter::Brace, |p| { - p.recover_diff_marker(); + p.recover_vcs_conflict_marker(); Ok((p.parse_use_tree()?, DUMMY_NODE_ID)) }) .map(|(r, _)| r) @@ -1497,9 +1498,9 @@ impl<'a> Parser<'a> { } fn parse_enum_variant(&mut self, span: Span) -> PResult<'a, Option> { - self.recover_diff_marker(); + self.recover_vcs_conflict_marker(); let variant_attrs = self.parse_outer_attributes()?; - self.recover_diff_marker(); + self.recover_vcs_conflict_marker(); let help = "enum variants can be `Variant`, `Variant = `, \ `Variant(Type, ..., TypeN)` or `Variant { fields: Types }`"; self.collect_tokens_trailing_token( @@ -1688,6 +1689,10 @@ impl<'a> Parser<'a> { Ok((class_name, ItemKind::Union(vdata, generics))) } + /// This function parses the fields of record structs: + /// + /// - `struct S { ... }` + /// - `enum E { Variant { ... } }` pub(crate) fn parse_record_struct_body( &mut self, adt_ty: &str, @@ -1714,19 +1719,10 @@ impl<'a> Parser<'a> { self.eat(&token::CloseDelim(Delimiter::Brace)); } else { let token_str = super::token_descr(&self.token); - let msg = format!( - "expected {}`{{` after struct name, found {}", - if parsed_where { "" } else { "`where`, or " }, - token_str - ); + let where_str = if parsed_where { "" } else { "`where`, or " }; + let msg = format!("expected {where_str}`{{` after struct name, found {token_str}"); let mut err = self.dcx().struct_span_err(self.token.span, msg); - err.span_label( - self.token.span, - format!( - "expected {}`{{` after struct name", - if parsed_where { "" } else { "`where`, or " } - ), - ); + err.span_label(self.token.span, format!("expected {where_str}`{{` after struct name",)); return Err(err); } @@ -1740,7 +1736,7 @@ impl<'a> Parser<'a> { let attrs = p.parse_outer_attributes()?; p.collect_tokens_trailing_token(attrs, ForceCollect::No, |p, attrs| { let mut snapshot = None; - if p.is_diff_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) { + if p.is_vcs_conflict_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) { // Account for `<<<<<<<` diff markers. We can't proactively error here because // that can be a valid type start, so we snapshot and reparse only we've // encountered another parse error. @@ -1751,7 +1747,7 @@ impl<'a> Parser<'a> { Ok(vis) => vis, Err(err) => { if let Some(ref mut snapshot) = snapshot { - snapshot.recover_diff_marker(); + snapshot.recover_vcs_conflict_marker(); } return Err(err); } @@ -1760,7 +1756,7 @@ impl<'a> Parser<'a> { Ok(ty) => ty, Err(err) => { if let Some(ref mut snapshot) = snapshot { - snapshot.recover_diff_marker(); + snapshot.recover_vcs_conflict_marker(); } return Err(err); } @@ -1785,9 +1781,9 @@ impl<'a> Parser<'a> { /// Parses an element of a struct declaration. fn parse_field_def(&mut self, adt_ty: &str) -> PResult<'a, FieldDef> { - self.recover_diff_marker(); + self.recover_vcs_conflict_marker(); let attrs = self.parse_outer_attributes()?; - self.recover_diff_marker(); + self.recover_vcs_conflict_marker(); self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| { let lo = this.token.span; let vis = this.parse_visibility(FollowedByType::No)?; @@ -2647,7 +2643,7 @@ impl<'a> Parser<'a> { } let (mut params, _) = self.parse_paren_comma_seq(|p| { - p.recover_diff_marker(); + p.recover_vcs_conflict_marker(); let snapshot = p.create_snapshot_for_diagnostic(); let param = p.parse_param_general(req_name, first_param).or_else(|e| { let guar = e.emit(); diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index d70afebf1b2..7424fbea9b0 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -567,7 +567,7 @@ impl<'a> Parser<'a> { if self.token == token::Eof { break; } - if self.is_diff_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) { + if self.is_vcs_conflict_marker(&TokenKind::BinOp(token::Shl), &TokenKind::Lt) { // Account for `<<<<<<<` diff markers. We can't proactively error here because // that can be a valid path start, so we snapshot and reparse only we've // encountered another parse error. @@ -576,7 +576,7 @@ impl<'a> Parser<'a> { let stmt = match self.parse_full_stmt(recover) { Err(mut err) if recover.yes() => { if let Some(ref mut snapshot) = snapshot { - snapshot.recover_diff_marker(); + snapshot.recover_vcs_conflict_marker(); } if self.token == token::Colon { // if a previous and next token of the current one is -- cgit 1.4.1-3-g733a5 From db5a616405df5956a76bf08ec3ad97258a2ccee8 Mon Sep 17 00:00:00 2001 From: ardi Date: Tue, 14 May 2024 16:35:00 +0200 Subject: s/(Ident, ItemKind)/ItemInfo/ --- compiler/rustc_parse/src/parser/item.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 05b0e0fe52d..8e085f963cd 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -311,7 +311,7 @@ impl<'a> Parser<'a> { Ok(Some(info)) } - fn recover_import_as_use(&mut self) -> PResult<'a, Option<(Ident, ItemKind)>> { + fn recover_import_as_use(&mut self) -> PResult<'a, Option> { let span = self.token.span; let token_name = super::token_descr(&self.token); let snapshot = self.create_snapshot_for_diagnostic(); @@ -329,7 +329,7 @@ impl<'a> Parser<'a> { } } - fn parse_use_item(&mut self) -> PResult<'a, (Ident, ItemKind)> { + fn parse_use_item(&mut self) -> PResult<'a, ItemInfo> { let tree = self.parse_use_tree()?; if let Err(mut e) = self.expect_semi() { match tree.kind { -- cgit 1.4.1-3-g733a5 From f8433a82b4a4120e34de236a68d0cfeaea6e60c3 Mon Sep 17 00:00:00 2001 From: ardi Date: Tue, 14 May 2024 16:38:01 +0200 Subject: use signature name for arg --- compiler/rustc_parse/src/parser/item.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'compiler/rustc_parse/src') diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 8e085f963cd..34cdf8a9b5f 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -195,12 +195,12 @@ impl<'a> Parser<'a> { fn_parse_mode: FnParseMode, case: Case, ) -> PResult<'a, Option> { - let def_final = def == &Defaultness::Final; + let check_pub = def == &Defaultness::Final; let mut def_ = || mem::replace(def, Defaultness::Final); let info = if self.eat_keyword_case(kw::Use, case) { self.parse_use_item()? - } else if self.check_fn_front_matter(def_final, case) { + } else if self.check_fn_front_matter(check_pub, case) { // FUNCTION ITEM let (ident, sig, generics, body) = self.parse_fn(attrs, fn_parse_mode, lo, vis, case)?; -- cgit 1.4.1-3-g733a5