From 190f0b29b69a387159264209c8eedc3efc891b10 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Tue, 21 Jul 2026 09:08:10 +0200 Subject: [PATCH 01/14] Remove `has_no_remaining_items_for_step()` `has_no_remaining_items_for_step()` is only meaningful for breadth-first traversal during parsing; it needs to be removed or adjusted to make depth-first traversal possible. This has a small effect on error-reporting behavior. In one case in the `assert-trailing-junk.rs` test, `$(,)` (at the end of the LHS) is matched against `blah`. This leads to three possible `MatcherLoc`s being investigated in the last step: the sequence start, the `,`, and EOF (in this order). Before this commit, the sequence start is reporting as `remaining_matcher`; when it is reached, there is nothing else in `cur_mps`. The `,` is ignored because the EOF is also in `cur_mps`, and the EOF is ignored because it is EOF. After this commit, the `,` is reported as `remaining_matcher`. This is the only change, and I think the new behavior is more useful; it will prioritize the contents of a sequence over a "sequence start" `MatcherLoc`. The `Display` impl for `MatcherLoc` incorrectly noted that "sequence start" is not used in diagnostics. With this change, it is less likely to be used, but I've left in a FIXME to investigate that thoroughly. --- compiler/rustc_expand/src/mbe/diagnostics.rs | 6 ++---- compiler/rustc_expand/src/mbe/macro_parser.rs | 10 ++++------ compiler/rustc_expand/src/mbe/macro_rules.rs | 4 ++-- .../assert-trailing-junk.with-generic-asset.stderr | 6 ++++-- .../assert-trailing-junk.without-generic-asset.stderr | 6 ++++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index cd0c54f293e44..b7623086a3217 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -209,10 +209,8 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match self.current = Some((which_matcher, matcher)); } - fn before_match_loc(&mut self, parser: &TtParser, matcher: &'matcher MatcherLoc) { - if self.remaining_matcher.is_none() - || (parser.has_no_remaining_items_for_step() && *matcher != MatcherLoc::Eof) - { + fn before_match_loc(&mut self, matcher: &'matcher MatcherLoc) { + if self.remaining_matcher.is_none() || *matcher != MatcherLoc::Eof { self.remaining_matcher = Some(matcher); } } diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index 95a4ebc63d38b..bbd4f53bcb5cc 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -155,7 +155,9 @@ impl Display for MatcherLoc { } MatcherLoc::Eof => f.write_str("end of macro"), - // These are not printed in the diagnostic + // FIXME: A prior comment noted that the following variants should not be printed in + // diagnostics. "while trying to match sequence end" appears in several stderrs in the + // ui tests. Other variants might be reachable too. MatcherLoc::Delimited => f.write_str("delimiter"), MatcherLoc::Sequence { .. } => f.write_str("sequence start"), MatcherLoc::SequenceKleeneOpNoSep { .. } => f.write_str("sequence end"), @@ -451,10 +453,6 @@ impl TtParser { } } - pub(super) fn has_no_remaining_items_for_step(&self) -> bool { - self.cur_mps.is_empty() - } - /// Process the matcher positions of `cur_mps` until it is empty. In the process, this will /// produce more mps in `next_mps` and `bb_mps`. /// @@ -504,7 +502,7 @@ impl TtParser { checking_for_ambiguity: bool, ) -> Option { let matcher_loc = &matcher[mp.idx]; - track.before_match_loc(self, matcher_loc); + track.before_match_loc(matcher_loc); let token = &parser.token; match matcher_loc { diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index cc4e08e2877b6..80adfcd2da17d 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -363,7 +363,7 @@ pub(super) trait Tracker<'matcher> { fn prepare(&mut self, which_matcher: WhichMatcher, matcher: &'matcher [MatcherLoc]); /// This is called before trying to match next MatcherLoc on the current token. - fn before_match_loc(&mut self, parser: &TtParser, matcher: &'matcher MatcherLoc); + fn before_match_loc(&mut self, matcher: &'matcher MatcherLoc); /// A [`MatcherLoc`] successfully consumed input from the parser. /// @@ -404,7 +404,7 @@ pub(super) struct NoopTracker; impl<'matcher> Tracker<'matcher> for NoopTracker { fn prepare(&mut self, _which_matcher: WhichMatcher, _matcher: &'matcher [MatcherLoc]) {} - fn before_match_loc(&mut self, _parser: &TtParser, _matcher: &'matcher MatcherLoc) {} + fn before_match_loc(&mut self, _matcher: &'matcher MatcherLoc) {} fn matched_one(&mut self, _parser: &Parser<'_>, _loc_index: usize) {} diff --git a/tests/ui/macros/assert-trailing-junk.with-generic-asset.stderr b/tests/ui/macros/assert-trailing-junk.with-generic-asset.stderr index 2af779a467a3f..6ff0e76afb2a5 100644 --- a/tests/ui/macros/assert-trailing-junk.with-generic-asset.stderr +++ b/tests/ui/macros/assert-trailing-junk.with-generic-asset.stderr @@ -18,7 +18,8 @@ LL | assert!(true, "whatever" blah); | | | help: missing comma here | - = note: while trying to match sequence start +note: while trying to match `,` + --> $SRC_DIR/std/src/panic.rs:LL:COL error: unexpected string literal --> $DIR/assert-trailing-junk.rs:19:18 @@ -36,7 +37,8 @@ LL | assert!(true "whatever" blah); | | | help: missing comma here | - = note: while trying to match sequence start +note: while trying to match `,` + --> $SRC_DIR/std/src/panic.rs:LL:COL error: macro requires an expression as an argument --> $DIR/assert-trailing-junk.rs:23:5 diff --git a/tests/ui/macros/assert-trailing-junk.without-generic-asset.stderr b/tests/ui/macros/assert-trailing-junk.without-generic-asset.stderr index 2af779a467a3f..6ff0e76afb2a5 100644 --- a/tests/ui/macros/assert-trailing-junk.without-generic-asset.stderr +++ b/tests/ui/macros/assert-trailing-junk.without-generic-asset.stderr @@ -18,7 +18,8 @@ LL | assert!(true, "whatever" blah); | | | help: missing comma here | - = note: while trying to match sequence start +note: while trying to match `,` + --> $SRC_DIR/std/src/panic.rs:LL:COL error: unexpected string literal --> $DIR/assert-trailing-junk.rs:19:18 @@ -36,7 +37,8 @@ LL | assert!(true "whatever" blah); | | | help: missing comma here | - = note: while trying to match sequence start +note: while trying to match `,` + --> $SRC_DIR/std/src/panic.rs:LL:COL error: macro requires an expression as an argument --> $DIR/assert-trailing-junk.rs:23:5 From 990616f998727a161d463a6de857b21b88963660 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Tue, 21 Jul 2026 09:08:10 +0200 Subject: [PATCH 02/14] Track seen tokens in `CollectTrackerAndEmitter` This commit reduces `Tracker`'s reliance on `Parser`. `Parser` can only be relied on for information about the furthest match; this is not a problem for BFS because all mps are at the same input position. But in DFS, mps will have varying input positions. Now the diagnostics tracker will collect every token observed from the parser. In the next commit, this will be used in `ambiguity()`. `tests/ui/macros` passes. --- compiler/rustc_expand/src/mbe/diagnostics.rs | 20 +++++++++++++++---- compiler/rustc_expand/src/mbe/macro_parser.rs | 10 +++++----- compiler/rustc_expand/src/mbe/macro_rules.rs | 12 +++++------ 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index b7623086a3217..71519dd4c5faa 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use rustc_ast::token::{self, Token}; use rustc_ast::tokenstream::TokenStream; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{Applicability, Diag, DiagCtxtHandle, DiagMessage, pluralize}; use rustc_hir::attrs::diagnostic::{CustomDiagnostic, Directive, FormatArgs}; use rustc_macros::Subdiagnostic; @@ -161,6 +161,9 @@ struct CollectTrackerAndEmitter<'dcx, 'matcher> { /// competing matches for ambiguity errors. matches: FxHashSet, + /// Tokens seen during parsing. + tokens: FxHashMap, + remaining_matcher: Option<&'matcher MatcherLoc>, /// Which arm's failure should we report? (the one furthest along) best_failure: Option, @@ -209,14 +212,21 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match self.current = Some((which_matcher, matcher)); } - fn before_match_loc(&mut self, matcher: &'matcher MatcherLoc) { + fn trying_match(&mut self, input_pos: u32, token: &Token, loc_index: usize) { + let Some((_, matcher)) = self.current else { + bug!("`Self::prepare()` was not called to initialize context"); + }; + let matcher = &matcher[loc_index]; + + let old_token = self.tokens.insert(input_pos, *token); + debug_assert!(old_token.is_none_or(|t| t == *token)); + if self.remaining_matcher.is_none() || *matcher != MatcherLoc::Eof { self.remaining_matcher = Some(matcher); } } - fn matched_one(&mut self, parser: &Parser<'_>, loc_index: usize) { - let input_pos = parser.approx_token_stream_pos(); + fn matched_one(&mut self, input_pos: u32, loc_index: usize) { let loc_index: u32 = loc_index.try_into().unwrap(); let m = SuccessfulMatch { input_pos, loc_index }; self.matches.insert(m); @@ -247,6 +257,7 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match self.current = None; self.matches.clear(); + self.tokens.clear(); } fn failure(&mut self, parser: &Parser<'_>) { @@ -356,6 +367,7 @@ impl<'dcx> CollectTrackerAndEmitter<'dcx, '_> { dcx, current: None, matches: FxHashSet::default(), + tokens: FxHashMap::default(), remaining_matcher: None, best_failure: None, root_span, diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index bbd4f53bcb5cc..42f9b966f230d 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -502,7 +502,7 @@ impl TtParser { checking_for_ambiguity: bool, ) -> Option { let matcher_loc = &matcher[mp.idx]; - track.before_match_loc(matcher_loc); + track.trying_match(parser.approx_token_stream_pos(), &parser.token, mp.idx); let token = &parser.token; match matcher_loc { @@ -519,7 +519,7 @@ impl TtParser { mp.idx += 1; self.cur_mps.push(mp); } else if token_name_eq(t, token) { - track.matched_one(parser, mp.idx); + track.matched_one(parser.approx_token_stream_pos(), mp.idx); mp.idx += 1; self.next_mps.push(mp); } @@ -579,7 +579,7 @@ impl TtParser { if token_name_eq(token, separator) { // The separator matches the current token. Advance past it. - track.matched_one(parser, mp.idx); + track.matched_one(parser.approx_token_stream_pos(), mp.idx); mp.idx += 1; self.next_mps.push(mp); } @@ -601,7 +601,7 @@ impl TtParser { // EOF tokens would cause unexpected processing in `match_one()`. debug_assert!(parser.token != token::Eof, "{kind:?} should not accept EOF tokens"); - track.matched_one(parser, mp.idx); + track.matched_one(parser.approx_token_stream_pos(), mp.idx); if let ControlFlow::Break(result) = self.check_for_ambiguity(parser, matcher, track, checking_for_ambiguity) @@ -628,7 +628,7 @@ impl TtParser { return None; } - track.matched_one(parser, mp.idx); + track.matched_one(parser.approx_token_stream_pos(), mp.idx); if let ControlFlow::Break(result) = self.check_for_ambiguity(parser, matcher, track, checking_for_ambiguity) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 80adfcd2da17d..4227c20526cc2 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -362,16 +362,16 @@ pub(super) trait Tracker<'matcher> { /// Provide context on the arm that's about to be matched. fn prepare(&mut self, which_matcher: WhichMatcher, matcher: &'matcher [MatcherLoc]); - /// This is called before trying to match next MatcherLoc on the current token. - fn before_match_loc(&mut self, matcher: &'matcher MatcherLoc); + /// A [`MatcherLoc`] is about to be matched against `token` at `input_pos`. + fn trying_match(&mut self, input_pos: u32, token: &Token, loc_index: usize); /// A [`MatcherLoc`] successfully consumed input from the parser. /// /// This is called for [`MatcherLoc::Token`] and [`MatcherLoc::SequenceSep`], which consume - /// single tokens, when they successfully match [`Parser::token`]. It is also called for + /// single tokens, when they successfully match the token at `input_pos`. It is also called for /// [`MatcherLoc::MetaVarDecl`] when non-terminal parsing is guaranteed to occur (i.e. after /// [`Parser::nonterminal_may_begin_with()`] returns `true`). - fn matched_one(&mut self, parser: &Parser<'_>, loc_index: usize); + fn matched_one(&mut self, input_pos: u32, loc_index: usize); /// This is called after an arm has been parsed, either successfully or unsuccessfully. When /// this is called, `before_match_loc` was called at least once (with a `MatcherLoc::Eof`). @@ -404,9 +404,9 @@ pub(super) struct NoopTracker; impl<'matcher> Tracker<'matcher> for NoopTracker { fn prepare(&mut self, _which_matcher: WhichMatcher, _matcher: &'matcher [MatcherLoc]) {} - fn before_match_loc(&mut self, _matcher: &'matcher MatcherLoc) {} + fn trying_match(&mut self, _input_pos: u32, _token: &Token, _loc_index: usize) {} - fn matched_one(&mut self, _parser: &Parser<'_>, _loc_index: usize) {} + fn matched_one(&mut self, _input_pos: u32, _loc_index: usize) {} fn ambiguity(&mut self, _parser: &Parser<'_>) {} From 4e5d5b50bdd47c5cac933e62554b286c2e1fe6fc Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Tue, 21 Jul 2026 09:08:10 +0200 Subject: [PATCH 03/14] Remove `Parser` param from `Tracker::ambiguity()` With this commit, `CollecTrackerAndEmitter::ambiguity()` relies on the `tokens` field (added in the last commit) instead of the `parser` parameter. It identifies ambiguity by finding the earliest position where ambiguity occurred. This nicely crosses the bridge from BFS to DFS -- in BFS, the position of ambiguity is obvious, but in DFS, there could be multiple ambiguities at different positions and the earliest one needs to be prioritized. --- compiler/rustc_expand/src/mbe/diagnostics.rs | 34 ++++++++++++++----- compiler/rustc_expand/src/mbe/macro_parser.rs | 2 +- compiler/rustc_expand/src/mbe/macro_rules.rs | 4 +-- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index 71519dd4c5faa..57b3e2d219d30 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -297,7 +297,7 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match } } - fn ambiguity(&mut self, parser: &Parser<'_>) { + fn ambiguity(&mut self) { let Some((_, matcher)) = self.current else { bug!("`Self::prepare()` was not called to initialize context"); }; @@ -306,22 +306,38 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match rustc::potential_query_instability, reason = "sorting the results deterministically afterwards" )] - let (mut bb_locs, mut next_locs) = self - .matches + let mut matches = self.matches.iter().collect::>(); + // Sort by input position, then `MatcherLoc` index. + matches.sort_unstable(); + + // Identify the earliest position where ambiguity occurred. + let input_pos = matches + .array_windows::<2>() + .find(|ms @ [a, b]| { + let mut locs = ms.iter().map(|x| &matcher[x.loc_index as usize]); + a.input_pos == b.input_pos + && locs + .any(|loc| matches!(loc, MatcherLoc::MetaVarDecl { .. } | MatcherLoc::Eof)) + }) + .map(|[a, _]| a.input_pos) + .unwrap_or_else(|| bug!("no ambiguity detected")); + + let (bb_locs, next_locs) = matches .iter() - .filter(|m| m.input_pos == parser.approx_token_stream_pos()) + .filter(|m| m.input_pos == input_pos) .partition::, _>(|m| { let loc = &matcher[m.loc_index as usize]; matches!(loc, MatcherLoc::MetaVarDecl { .. }) }); - // Use a reasonable and deterministic ordering for data in the error message. - bb_locs.sort_unstable_by_key(|m| m.loc_index); - next_locs.sort_unstable_by_key(|m| m.loc_index); + debug_assert!(bb_locs.iter().is_sorted()); + debug_assert!(next_locs.iter().is_sorted()); + + let token = *self.tokens.get(&input_pos).unwrap(); - let span = parser.token.span.substitute_dummy(self.root_span); + let span = token.span.substitute_dummy(self.root_span); - if parser.token == token::Eof { + if token == token::Eof { let msg = "ambiguity: multiple successful parses".to_string(); let guar = self.dcx.span_err(span, msg); self.result = Some((span, guar)); diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index 42f9b966f230d..ddf81fa993494 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -675,7 +675,7 @@ impl TtParser { } if std::mem::take(&mut self.found_ambiguity) || !self.next_mps.is_empty() { - track.ambiguity(parser); + track.ambiguity(); ControlFlow::Break(Some(Ambiguity)) } else { ControlFlow::Continue(()) diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 4227c20526cc2..3481d45cf9d74 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -389,7 +389,7 @@ pub(super) trait Tracker<'matcher> { /// An ambiguity error occurred. /// /// The parser will return [`NamedParseResult::Ambiguity`] after calling this. - fn ambiguity(&mut self, parser: &Parser<'_>); + fn ambiguity(&mut self); /// For tracing. fn description() -> &'static str; @@ -408,7 +408,7 @@ impl<'matcher> Tracker<'matcher> for NoopTracker { fn matched_one(&mut self, _input_pos: u32, _loc_index: usize) {} - fn ambiguity(&mut self, _parser: &Parser<'_>) {} + fn ambiguity(&mut self) {} fn after_arm(&mut self, _result: &NamedParseResult) {} From ad2000a7ec4ddb4e70034524488a263d8bb4b188 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Fri, 24 Jul 2026 03:37:57 +0200 Subject: [PATCH 04/14] Merge `check_for_ambiguity()` flow into `parse_tt()` `check_for_ambiguity()` repeated some of the work done by `parse_tt()` (specifically, processing mps from `cur_mps`). The previous flow for metavar/EOF matching was: - During `parse_tt_inner()`: - During `match_one()`: - Check for a match, e.g. with `nonterminal_may_begin_with()`. - If `checking_for_ambiguity`, fail. - Call `check_for_ambiguity()`: - Drain everything in `cur_mps`. - If anything matched successfully, fail. - Finish processing the mp, e.g. `Parser::parse_nonterminal()`. The new flow is: - During `parse_tt_inner()`: - During `match_one()`: - Check for a match, e.g. with `nonterminal_may_begin_with()`. - Store the mp in `maybe_ambig_mp`. - If something is already there, fail. - Drain everything in `cur_mps`. - If `maybe_ambig_mp` is set: - If anything matched successfully, fail. - Finish processing the mp, e.g. `Parser::parse_nonterminal()`. This is quite similar to the structure before I started making changes, e.g. via `bb_mps`. In the new structure, it also handles EOF. `tests/ui/macros` passes. --- compiler/rustc_expand/src/mbe/macro_parser.rs | 112 +++++++++--------- 1 file changed, 53 insertions(+), 59 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index ddf81fa993494..9b1071607c26b 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -72,7 +72,6 @@ use std::borrow::Cow; use std::fmt::Display; -use std::ops::ControlFlow; use std::rc::Rc; pub(crate) use NamedMatch::*; @@ -439,6 +438,14 @@ pub(crate) struct TtParser { /// that have no metavars. empty_matches: Rc>, + /// A potentially-ambiguous mp waiting to be parsed. + /// + /// This is an mp that has been successfully matched, and that is unambiguous iff no other mps + /// match at the same input position. It is stored here until all other mps have been exhausted. + /// If another mp conflicts with this, this is left untouched and [`Self::found_ambiguity`] is + /// set. + maybe_ambig_mp: Option, + /// Whether an ambiguity error has occurred. found_ambiguity: bool, } @@ -449,6 +456,7 @@ impl TtParser { cur_mps: vec![], next_mps: vec![], empty_matches: Rc::new(vec![]), + maybe_ambig_mp: None, found_ambiguity: false, } } @@ -467,11 +475,21 @@ impl TtParser { track: &mut T, ) -> Option { while let Some(mp) = self.cur_mps.pop() { - if let Some(result) = self.match_one(parser, matcher, mp, track, false) { + if let Some(result) = self.match_one(parser, matcher, mp, track) { return Some(result); } } + if let Some(mp) = self.maybe_ambig_mp.take() { + if self.found_ambiguity || !self.next_mps.is_empty() { + // Something successfully matched at the same position as `mp`. + track.ambiguity(); + return Some(Ambiguity); + } + + return self.process_special(parser, matcher, mp); + } + // FIXME: Error messages here could be improved with links to original rules. if self.next_mps.is_empty() { @@ -489,9 +507,6 @@ impl TtParser { } /// Match a single [`MatcherPos`]. - /// - /// If a meta-variable is encountered and `checking_for_ambiguity` is `false`, `cur_mps` will be - /// drained to eagerly check for ambiguity, and `parser` will be modified. #[inline(always)] // must be inlined in `parse_tt_inner()` fn match_one<'matcher, T: Tracker<'matcher>>( &mut self, @@ -499,7 +514,6 @@ impl TtParser { matcher: &'matcher [MatcherLoc], mut mp: MatcherPos, track: &mut T, - checking_for_ambiguity: bool, ) -> Option { let matcher_loc = &matcher[mp.idx]; track.trying_match(parser.approx_token_stream_pos(), &parser.token, mp.idx); @@ -590,7 +604,7 @@ impl TtParser { mp.idx = idx_first; self.cur_mps.push(mp); } - &MatcherLoc::MetaVarDecl { kind, next_metavar, seq_depth, .. } => { + &MatcherLoc::MetaVarDecl { kind, .. } => { // Built-in nonterminals never start with these tokens, so we can eliminate them // from consideration. We use the span of the metavariable declaration to determine // any edition-specific matching behavior for non-terminals. @@ -603,22 +617,11 @@ impl TtParser { track.matched_one(parser.approx_token_stream_pos(), mp.idx); - if let ControlFlow::Break(result) = - self.check_for_ambiguity(parser, matcher, track, checking_for_ambiguity) - { - return result; + if self.maybe_ambig_mp.is_some() { + self.found_ambiguity = true; + } else { + self.maybe_ambig_mp = Some(mp); } - - // We use the span of the metavariable declaration to determine any - // edition-specific matching behavior for non-terminals. - let nt = match parser.to_mut().parse_nonterminal(kind) { - Err(err) => return Some(self.nt_parsing_error(matcher_loc, err)), - Ok(nt) => nt, - }; - mp.push_match(next_metavar, seq_depth, MatchedSingle(nt)); - - mp.idx += 1; - self.cur_mps.push(mp); } MatcherLoc::Eof => { // We are past the matcher's end, and not in a sequence. Try to end things. @@ -630,55 +633,46 @@ impl TtParser { track.matched_one(parser.approx_token_stream_pos(), mp.idx); - if let ControlFlow::Break(result) = - self.check_for_ambiguity(parser, matcher, track, checking_for_ambiguity) - { - return result; + if self.maybe_ambig_mp.is_some() { + self.found_ambiguity = true; + } else { + self.maybe_ambig_mp = Some(mp); } - - let matches = Rc::unwrap_or_clone(mp.matches).into_iter(); - return Some(Success(self.nameize(matcher, matches))); } } None } - /// Look for ambiguity before parsing a non-terminal. - /// - /// - When `checking_for_ambiguity`: immediately an ambiguity error. - /// - Otherwise: eagerly consumes [`Self::cur_mps`] to check for ambiguity. - /// - /// If [`ControlFlow::Continue`] is returned, ambiguity has not been detected. - fn check_for_ambiguity<'matcher, R, T: Tracker<'matcher>>( + /// Finish processing a matched special [`MatcherPos`]. + fn process_special( &mut self, parser: &mut Cow<'_, Parser<'_>>, - matcher: &'matcher [MatcherLoc], - track: &mut T, - checking_for_ambiguity: bool, - ) -> ControlFlow>> { - if checking_for_ambiguity { - // This was called in the context of a _different_ `MatcherLoc` that was about to be - // matched. Prevent the caller from doing more work, but don't prepare the actual error - // yet; let the outer `check_for_ambiguity()` do that. - self.found_ambiguity = true; - return ControlFlow::Break(None); - } + matcher: &[MatcherLoc], + mut mp: MatcherPos, + ) -> Option { + let matcher_loc = &matcher[mp.idx as usize]; + match matcher_loc { + &MatcherLoc::MetaVarDecl { kind, next_metavar, seq_depth, .. } => { + // We use the span of the metavariable declaration to determine any + // edition-specific matching behavior for non-terminals. + let nt = match parser.to_mut().parse_nonterminal(kind) { + Err(err) => return Some(self.nt_parsing_error(matcher_loc, err)), + Ok(nt) => nt, + }; + mp.push_match(next_metavar, seq_depth, MatchedSingle(nt)); - assert!(!self.found_ambiguity); + mp.idx += 1; + self.cur_mps.push(mp); + None + } - // Consume all pending mps at the current input position. - while let Some(mp) = self.cur_mps.pop() { - let result = self.match_one(parser, matcher, mp, track, true); - // A result cannot be returned when `check_for_ambiguity` is `true`. - assert!(result.is_none()); - } + MatcherLoc::Eof => { + let matches = Rc::unwrap_or_clone(mp.matches).into_iter(); + Some(Success(self.nameize(matcher, matches))) + } - if std::mem::take(&mut self.found_ambiguity) || !self.next_mps.is_empty() { - track.ambiguity(); - ControlFlow::Break(Some(Ambiguity)) - } else { - ControlFlow::Continue(()) + _ => unreachable!(), } } From 888a320453e5a32e0025d4227b356e85921c3547 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Fri, 24 Jul 2026 04:08:05 +0200 Subject: [PATCH 05/14] Remove return value from `match_one()` --- compiler/rustc_expand/src/mbe/macro_parser.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index 9b1071607c26b..a40ed3fd2cd37 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -475,9 +475,7 @@ impl TtParser { track: &mut T, ) -> Option { while let Some(mp) = self.cur_mps.pop() { - if let Some(result) = self.match_one(parser, matcher, mp, track) { - return Some(result); - } + self.match_one(parser, matcher, mp, track); } if let Some(mp) = self.maybe_ambig_mp.take() { @@ -514,7 +512,7 @@ impl TtParser { matcher: &'matcher [MatcherLoc], mut mp: MatcherPos, track: &mut T, - ) -> Option { + ) { let matcher_loc = &matcher[mp.idx]; track.trying_match(parser.approx_token_stream_pos(), &parser.token, mp.idx); let token = &parser.token; @@ -609,7 +607,7 @@ impl TtParser { // from consideration. We use the span of the metavariable declaration to determine // any edition-specific matching behavior for non-terminals. if !Parser::nonterminal_may_begin_with(kind, token) { - return None; + return; } // EOF tokens would cause unexpected processing in `match_one()`. @@ -628,7 +626,7 @@ impl TtParser { debug_assert_eq!(mp.idx, matcher.len() - 1); if *token != token::Eof { - return None; + return; } track.matched_one(parser.approx_token_stream_pos(), mp.idx); @@ -640,8 +638,6 @@ impl TtParser { } } } - - None } /// Finish processing a matched special [`MatcherPos`]. From da78b53a1b64f52a6194c9a555c5b8fce111ab45 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Fri, 24 Jul 2026 07:13:08 +0200 Subject: [PATCH 06/14] Deterministically pick `remaining_matcher` Previously, `remaining_matcher` was picked based on implementation dependent ordering of mps. With this commit, it is deterministically selected as the furthest-along mp with the furthest-along loc. --- compiler/rustc_expand/src/mbe/diagnostics.rs | 80 +++++++++++++------- 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index 57b3e2d219d30..86724dacb5993 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use rustc_ast::token::{self, Token}; use rustc_ast::tokenstream::TokenStream; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::FxHashMap; use rustc_errors::{Applicability, Diag, DiagCtxtHandle, DiagMessage, pluralize}; use rustc_hir::attrs::diagnostic::{CustomDiagnostic, Directive, FormatArgs}; use rustc_macros::Subdiagnostic; @@ -155,16 +155,16 @@ struct CollectTrackerAndEmitter<'dcx, 'matcher> { // FIXME: Factor out a per-arm `Tracker` so that the `Option` is unnecessary. current: Option<(WhichMatcher, &'matcher [MatcherLoc])>, - /// Matches of [`MatcherLoc`]s that successfully consumed input from the parser. + /// Matches of [`MatcherLoc`]s. /// - /// This accumulates all calls to [`Tracker::matched_one()`]. It is used to identify all - /// competing matches for ambiguity errors. - matches: FxHashSet, + /// This accumulates all calls to [`Tracker::trying_match()`] and [`Tracker::matched_one()`]. It + /// is used to identify all competing matches for ambiguity errors, and relevant match attempts + /// for failures. + matches: FxHashMap, /// Tokens seen during parsing. tokens: FxHashMap, - remaining_matcher: Option<&'matcher MatcherLoc>, /// Which arm's failure should we report? (the one furthest along) best_failure: Option, root_span: Span, @@ -172,7 +172,7 @@ struct CollectTrackerAndEmitter<'dcx, 'matcher> { } #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -struct SuccessfulMatch { +struct Match { /// The position in the parser. /// /// As per [`Parser::approx_token_stream_pos()`]. @@ -182,6 +182,12 @@ struct SuccessfulMatch { loc_index: u32, } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum MatchResult { + Success, + Failure, +} + struct BestFailure { token: Token, @@ -213,23 +219,21 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match } fn trying_match(&mut self, input_pos: u32, token: &Token, loc_index: usize) { - let Some((_, matcher)) = self.current else { - bug!("`Self::prepare()` was not called to initialize context"); - }; - let matcher = &matcher[loc_index]; - let old_token = self.tokens.insert(input_pos, *token); debug_assert!(old_token.is_none_or(|t| t == *token)); - if self.remaining_matcher.is_none() || *matcher != MatcherLoc::Eof { - self.remaining_matcher = Some(matcher); - } + // Insert failure for now, will be updated in `matched_one()`. + let loc_index: u32 = loc_index.try_into().unwrap(); + let m = Match { input_pos, loc_index }; + self.matches.entry(m).or_insert(MatchResult::Failure); } fn matched_one(&mut self, input_pos: u32, loc_index: usize) { let loc_index: u32 = loc_index.try_into().unwrap(); - let m = SuccessfulMatch { input_pos, loc_index }; - self.matches.insert(m); + let m = Match { input_pos, loc_index }; + let match_result = + self.matches.get_mut(&m).unwrap_or_else(|| bug!("no corresponding `trying_match()`")); + *match_result = MatchResult::Success; } fn after_arm(&mut self, result: &NamedParseResult) { @@ -261,7 +265,7 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match } fn failure(&mut self, parser: &Parser<'_>) { - let Some((which_matcher, _)) = self.current else { + let Some((which_matcher, matcher)) = self.current else { bug!("`Self::prepare()` was not called to initialize context"); }; @@ -284,15 +288,32 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match .as_ref() .is_none_or(|failure| failure.is_better_position(which_matcher, approx_position)) { + // Use the furthest-along non-EOF matcher. If none exists, use an EOF matcher. + #[expect(rustc::potential_query_instability, reason = "finding a unique maximum value")] + let (&Match { input_pos, loc_index }, &result) = self + .matches + .iter() + .max_by_key(|&(m, _res)| { + // sort EOFs before others to deprioritize them + let eof = match matcher[m.loc_index as usize] { + MatcherLoc::Eof => 0, + _ => 1, + }; + (eof, m) + }) + .unwrap_or_else(|| bug!("failure without a `trying_match()` call")); + // NOTE: `input_pos` might differ from `approx_position` (it might be a little older, + // if there were no non-EOF candidates at the right position), and it might have been + // a successful match for the same reason. + let _ = (input_pos, result); + let matcher = matcher[loc_index as usize].clone(); + self.best_failure = Some(BestFailure { token, matcher: which_matcher, position: approx_position, msg, - remaining_matcher: self - .remaining_matcher - .expect("must have collected matcher already") - .clone(), + remaining_matcher: matcher, }) } } @@ -306,7 +327,11 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match rustc::potential_query_instability, reason = "sorting the results deterministically afterwards" )] - let mut matches = self.matches.iter().collect::>(); + let mut matches = self + .matches + .iter() + .filter_map(|(m, result)| matches!(result, MatchResult::Success).then_some(m)) + .collect::>(); // Sort by input position, then `MatcherLoc` index. matches.sort_unstable(); @@ -322,10 +347,8 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match .map(|[a, _]| a.input_pos) .unwrap_or_else(|| bug!("no ambiguity detected")); - let (bb_locs, next_locs) = matches - .iter() - .filter(|m| m.input_pos == input_pos) - .partition::, _>(|m| { + let (bb_locs, next_locs) = + matches.iter().filter(|m| m.input_pos == input_pos).partition::, _>(|m| { let loc = &matcher[m.loc_index as usize]; matches!(loc, MatcherLoc::MetaVarDecl { .. }) }); @@ -382,9 +405,8 @@ impl<'dcx> CollectTrackerAndEmitter<'dcx, '_> { macro_name, dcx, current: None, - matches: FxHashSet::default(), + matches: FxHashMap::default(), tokens: FxHashMap::default(), - remaining_matcher: None, best_failure: None, root_span, result: None, From 00be6b54a9c4e3e6e2b38e9cdeab2646079b6046 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Wed, 15 Jul 2026 18:01:06 +0200 Subject: [PATCH 07/14] Use `u32`s for indices into `[MatcherLoc]` Needed so I can add another field to `MatcherPos` without growing it. --- compiler/rustc_expand/src/mbe/diagnostics.rs | 5 ++--- compiler/rustc_expand/src/mbe/macro_parser.rs | 14 +++++++------- compiler/rustc_expand/src/mbe/macro_rules.rs | 8 ++++---- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index 86724dacb5993..f2cd175342a8f 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -218,7 +218,7 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match self.current = Some((which_matcher, matcher)); } - fn trying_match(&mut self, input_pos: u32, token: &Token, loc_index: usize) { + fn trying_match(&mut self, input_pos: u32, token: &Token, loc_index: u32) { let old_token = self.tokens.insert(input_pos, *token); debug_assert!(old_token.is_none_or(|t| t == *token)); @@ -228,8 +228,7 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match self.matches.entry(m).or_insert(MatchResult::Failure); } - fn matched_one(&mut self, input_pos: u32, loc_index: usize) { - let loc_index: u32 = loc_index.try_into().unwrap(); + fn matched_one(&mut self, input_pos: u32, loc_index: u32) { let m = Match { input_pos, loc_index }; let match_result = self.matches.get_mut(&m).unwrap_or_else(|| bug!("no corresponding `trying_match()`")); diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index a40ed3fd2cd37..62ee7c558ae61 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -245,7 +245,7 @@ pub(super) fn compute_locs(matcher: &[TokenTree]) -> Vec { #[derive(Debug)] struct MatcherPos { /// The index into `TtParser::locs`, which represents the "dot". - idx: usize, + idx: u32, /// The matches made against metavar decls so far. On a successful match, this vector ends up /// with one element per metavar decl in the matcher. Each element records token trees matched @@ -513,7 +513,7 @@ impl TtParser { mut mp: MatcherPos, track: &mut T, ) { - let matcher_loc = &matcher[mp.idx]; + let matcher_loc = &matcher[mp.idx as usize]; track.trying_match(parser.approx_token_stream_pos(), &parser.token, mp.idx); let token = &parser.token; @@ -555,8 +555,8 @@ impl TtParser { if matches!(op, KleeneOp::ZeroOrMore | KleeneOp::ZeroOrOne) { // Try zero matches of this sequence, by skipping over it. - self.cur_mps - .push(MatcherPos { idx: idx_first_after, matches: Rc::clone(&mp.matches) }); + let idx = idx_first_after.try_into().unwrap(); + self.cur_mps.push(MatcherPos { idx, matches: Rc::clone(&mp.matches) }); } // Try one or more matches of this sequence, by entering it. @@ -575,7 +575,7 @@ impl TtParser { if op != KleeneOp::ZeroOrOne { // Try another repetition. - mp.idx = idx_first; + mp.idx = idx_first.try_into().unwrap(); self.cur_mps.push(mp); } } @@ -599,7 +599,7 @@ impl TtParser { &MatcherLoc::SequenceKleeneOpAfterSep { idx_first } => { // We are past the sequence separator. This can't be a `?` Kleene op, because they // don't permit separators. Try another repetition. - mp.idx = idx_first; + mp.idx = idx_first.try_into().unwrap(); self.cur_mps.push(mp); } &MatcherLoc::MetaVarDecl { kind, .. } => { @@ -623,7 +623,7 @@ impl TtParser { } MatcherLoc::Eof => { // We are past the matcher's end, and not in a sequence. Try to end things. - debug_assert_eq!(mp.idx, matcher.len() - 1); + debug_assert_eq!(mp.idx as usize, matcher.len() - 1); if *token != token::Eof { return; diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 3481d45cf9d74..05cb4e49314d4 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -363,7 +363,7 @@ pub(super) trait Tracker<'matcher> { fn prepare(&mut self, which_matcher: WhichMatcher, matcher: &'matcher [MatcherLoc]); /// A [`MatcherLoc`] is about to be matched against `token` at `input_pos`. - fn trying_match(&mut self, input_pos: u32, token: &Token, loc_index: usize); + fn trying_match(&mut self, input_pos: u32, token: &Token, loc_index: u32); /// A [`MatcherLoc`] successfully consumed input from the parser. /// @@ -371,7 +371,7 @@ pub(super) trait Tracker<'matcher> { /// single tokens, when they successfully match the token at `input_pos`. It is also called for /// [`MatcherLoc::MetaVarDecl`] when non-terminal parsing is guaranteed to occur (i.e. after /// [`Parser::nonterminal_may_begin_with()`] returns `true`). - fn matched_one(&mut self, input_pos: u32, loc_index: usize); + fn matched_one(&mut self, input_pos: u32, loc_index: u32); /// This is called after an arm has been parsed, either successfully or unsuccessfully. When /// this is called, `before_match_loc` was called at least once (with a `MatcherLoc::Eof`). @@ -404,9 +404,9 @@ pub(super) struct NoopTracker; impl<'matcher> Tracker<'matcher> for NoopTracker { fn prepare(&mut self, _which_matcher: WhichMatcher, _matcher: &'matcher [MatcherLoc]) {} - fn trying_match(&mut self, _input_pos: u32, _token: &Token, _loc_index: usize) {} + fn trying_match(&mut self, _input_pos: u32, _token: &Token, _loc_index: u32) {} - fn matched_one(&mut self, _input_pos: u32, _loc_index: usize) {} + fn matched_one(&mut self, _input_pos: u32, _loc_index: u32) {} fn ambiguity(&mut self) {} From c4b71f409246b433ed72eaee279a9bec570b53f0 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Wed, 15 Jul 2026 17:50:13 +0200 Subject: [PATCH 08/14] Track tokens bumped from the parser This will form the basis for backtracking. Note that it is cleared after meta-variable parsing; since meta-variables do not admit ambiguity, and `cur_mps` is cleared by `check_for_ambiguity()` when a meta-variable is about to be parsed, no mps could possibly exist to refer to older tokens from the input. --- compiler/rustc_expand/src/mbe/macro_parser.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index 62ee7c558ae61..531a38814f06e 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -434,6 +434,13 @@ pub(crate) struct TtParser { /// `parse_tt`. next_mps: Vec, + /// Previously seen tokens from the parser. + /// + /// Tokens after the latest meta-variable that have been matched against a fixed token and + /// [`Parser::bump()`]-ed past are stored here. This list is cleared every time a meta-variable + /// is parsed. + seen_tokens: Vec, + /// Pre-allocate an empty match array, so it can be cloned cheaply for macros with many rules /// that have no metavars. empty_matches: Rc>, @@ -455,6 +462,7 @@ impl TtParser { TtParser { cur_mps: vec![], next_mps: vec![], + seen_tokens: vec![], empty_matches: Rc::new(vec![]), maybe_ambig_mp: None, found_ambiguity: false, @@ -499,6 +507,7 @@ impl TtParser { // Dump all possible `next_mps` into `cur_mps` for the next iteration. Then // process the next token. self.cur_mps.append(&mut self.next_mps); + self.seen_tokens.push(parser.token); parser.to_mut().bump(); None @@ -659,11 +668,13 @@ impl TtParser { mp.push_match(next_metavar, seq_depth, MatchedSingle(nt)); mp.idx += 1; + self.seen_tokens.clear(); self.cur_mps.push(mp); None } MatcherLoc::Eof => { + self.seen_tokens.clear(); let matches = Rc::unwrap_or_clone(mp.matches).into_iter(); Some(Success(self.nameize(matcher, matches))) } @@ -685,6 +696,7 @@ impl TtParser { // possible next positions into `next_mps`. After some post-processing, the contents of // `next_mps` replenish `cur_mps` and we start over again. self.cur_mps.clear(); + self.seen_tokens.clear(); self.cur_mps.push(MatcherPos { idx: 0, matches: Rc::clone(&self.empty_matches) }); loop { From 436e129220dfbb63b629bc938f2a566c1026517e Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Wed, 15 Jul 2026 18:01:06 +0200 Subject: [PATCH 09/14] Track input position in `MatcherPos` This will be used for backtracking. At the moment, all mps in `cur_mps` have the same input position, and all mps in `next_mps` have the same input position (exactly one more than that in `cur_mps`). --- compiler/rustc_expand/src/mbe/macro_parser.rs | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index 531a38814f06e..e357914c3ff49 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -247,6 +247,12 @@ struct MatcherPos { /// The index into `TtParser::locs`, which represents the "dot". idx: u32, + /// The input position being targeted. + /// + /// This is an index into or to the end of `seen_tokens`; in the latter case, it then refers + /// to the latest token from `parser`. + input_pos: u32, + /// The matches made against metavar decls so far. On a successful match, this vector ends up /// with one element per metavar decl in the matcher. Each element records token trees matched /// against the relevant metavar by the black box parser. An element will be a `MatchedSeq` if @@ -523,8 +529,14 @@ impl TtParser { track: &mut T, ) { let matcher_loc = &matcher[mp.idx as usize]; - track.trying_match(parser.approx_token_stream_pos(), &parser.token, mp.idx); - let token = &parser.token; + // How far from the latest token are we. + let age = parser.approx_token_stream_pos() - mp.input_pos; + let token = if age == 0 { + &parser.token + } else { + &self.seen_tokens[self.seen_tokens.len() - age as usize] + }; + track.trying_match(mp.input_pos, token, mp.idx); match matcher_loc { MatcherLoc::Token { token: t } => { @@ -540,8 +552,9 @@ impl TtParser { mp.idx += 1; self.cur_mps.push(mp); } else if token_name_eq(t, token) { - track.matched_one(parser.approx_token_stream_pos(), mp.idx); + track.matched_one(mp.input_pos, mp.idx); mp.idx += 1; + mp.input_pos += 1; self.next_mps.push(mp); } } @@ -565,7 +578,11 @@ impl TtParser { if matches!(op, KleeneOp::ZeroOrMore | KleeneOp::ZeroOrOne) { // Try zero matches of this sequence, by skipping over it. let idx = idx_first_after.try_into().unwrap(); - self.cur_mps.push(MatcherPos { idx, matches: Rc::clone(&mp.matches) }); + self.cur_mps.push(MatcherPos { + idx, + input_pos: mp.input_pos, + matches: Rc::clone(&mp.matches), + }); } // Try one or more matches of this sequence, by entering it. @@ -578,6 +595,7 @@ impl TtParser { // around the loop. let ending_mp = MatcherPos { idx: mp.idx + 1, // +1 skips the Kleene op + input_pos: mp.input_pos, matches: Rc::clone(&mp.matches), }; self.cur_mps.push(ending_mp); @@ -594,14 +612,16 @@ impl TtParser { // fail quietly when it is processed next time around the loop. let ending_mp = MatcherPos { idx: mp.idx + 2, // +2 skips the separator and the Kleene op + input_pos: mp.input_pos, matches: Rc::clone(&mp.matches), }; self.cur_mps.push(ending_mp); if token_name_eq(token, separator) { // The separator matches the current token. Advance past it. - track.matched_one(parser.approx_token_stream_pos(), mp.idx); + track.matched_one(mp.input_pos, mp.idx); mp.idx += 1; + mp.input_pos += 1; self.next_mps.push(mp); } } @@ -622,7 +642,7 @@ impl TtParser { // EOF tokens would cause unexpected processing in `match_one()`. debug_assert!(parser.token != token::Eof, "{kind:?} should not accept EOF tokens"); - track.matched_one(parser.approx_token_stream_pos(), mp.idx); + track.matched_one(mp.input_pos, mp.idx); if self.maybe_ambig_mp.is_some() { self.found_ambiguity = true; @@ -638,7 +658,7 @@ impl TtParser { return; } - track.matched_one(parser.approx_token_stream_pos(), mp.idx); + track.matched_one(mp.input_pos, mp.idx); if self.maybe_ambig_mp.is_some() { self.found_ambiguity = true; @@ -668,6 +688,7 @@ impl TtParser { mp.push_match(next_metavar, seq_depth, MatchedSingle(nt)); mp.idx += 1; + mp.input_pos = parser.approx_token_stream_pos(); self.seen_tokens.clear(); self.cur_mps.push(mp); None @@ -697,7 +718,11 @@ impl TtParser { // `next_mps` replenish `cur_mps` and we start over again. self.cur_mps.clear(); self.seen_tokens.clear(); - self.cur_mps.push(MatcherPos { idx: 0, matches: Rc::clone(&self.empty_matches) }); + self.cur_mps.push(MatcherPos { + idx: 0, + input_pos: parser.approx_token_stream_pos(), + matches: Rc::clone(&self.empty_matches), + }); loop { assert!(!self.cur_mps.is_empty()); From 66831972c660bf2b8b78fb061cc039036589acd9 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Fri, 24 Jul 2026 03:03:42 +0200 Subject: [PATCH 10/14] Use depth-first traversal for macro parsing `tests/ui/macros` passes!!! --- compiler/rustc_expand/src/mbe/macro_parser.rs | 90 +++++++++---------- 1 file changed, 44 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index e357914c3ff49..22a14cde9c37e 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -432,13 +432,10 @@ fn token_name_eq(t1: &Token, t2: &Token) -> bool { // Note: the vectors could be created and dropped within `parse_tt`, but to avoid excess // allocations we have a single vector for each kind that is cleared and reused repeatedly. pub(crate) struct TtParser { - /// The set of current mps to be processed. This should be empty by the end of a successful - /// execution of `parse_tt_inner`. - cur_mps: Vec, - - /// The set of newly generated mps. These are used to replenish `cur_mps` in the function - /// `parse_tt`. - next_mps: Vec, + /// mps at older input positions that are yet to be explored. + /// + /// Invariant: `backtrack.iter().is_sorted_by_key(|mp| mp.input_pos)`. + backtrack: Vec, /// Previously seen tokens from the parser. /// @@ -466,8 +463,7 @@ pub(crate) struct TtParser { impl TtParser { pub(super) fn new() -> TtParser { TtParser { - cur_mps: vec![], - next_mps: vec![], + backtrack: vec![], seen_tokens: vec![], empty_matches: Rc::new(vec![]), maybe_ambig_mp: None, @@ -488,13 +484,18 @@ impl TtParser { matcher: &'matcher [MatcherLoc], track: &mut T, ) -> Option { - while let Some(mp) = self.cur_mps.pop() { + while let Some(mp) = self.backtrack.pop() { self.match_one(parser, matcher, mp, track); + + debug_assert!(self.backtrack.iter().is_sorted_by_key(|mp| mp.input_pos)); } if let Some(mp) = self.maybe_ambig_mp.take() { - if self.found_ambiguity || !self.next_mps.is_empty() { - // Something successfully matched at the same position as `mp`. + if self.found_ambiguity || parser.approx_token_stream_pos() > mp.input_pos { + // Either: + // - A second maybe-ambig mp was found, setting `found_ambiguity` + // - Something else was parsed successfully, advancing `parser` past `mp` + // - `mp` was matched while backtracking track.ambiguity(); return Some(Ambiguity); } @@ -502,21 +503,9 @@ impl TtParser { return self.process_special(parser, matcher, mp); } - // FIXME: Error messages here could be improved with links to original rules. - - if self.next_mps.is_empty() { - // There are no possible next positions: syntax error. - track.failure(parser); - return Some(Failure); - } - - // Dump all possible `next_mps` into `cur_mps` for the next iteration. Then - // process the next token. - self.cur_mps.append(&mut self.next_mps); - self.seen_tokens.push(parser.token); - parser.to_mut().bump(); - - None + // There are no possible next positions: syntax error. + track.failure(parser); + Some(Failure) } /// Match a single [`MatcherPos`]. @@ -549,19 +538,25 @@ impl TtParser { // Otherwise, this match has failed, there is nothing to do, and hopefully another // mp in `cur_mps` will match. if matches!(t, Token { kind: DocComment(..), .. }) { - mp.idx += 1; - self.cur_mps.push(mp); + // skip } else if token_name_eq(t, token) { track.matched_one(mp.input_pos, mp.idx); - mp.idx += 1; mp.input_pos += 1; - self.next_mps.push(mp); + if mp.input_pos > parser.approx_token_stream_pos() { + self.seen_tokens.push(parser.token); + parser.to_mut().bump(); + debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); + } + } else { + return; } + mp.idx += 1; + self.backtrack.push(mp); } MatcherLoc::Delimited => { // Entering the delimiter is trivial. mp.idx += 1; - self.cur_mps.push(mp); + self.backtrack.push(mp); } &MatcherLoc::Sequence { op, @@ -578,7 +573,7 @@ impl TtParser { if matches!(op, KleeneOp::ZeroOrMore | KleeneOp::ZeroOrOne) { // Try zero matches of this sequence, by skipping over it. let idx = idx_first_after.try_into().unwrap(); - self.cur_mps.push(MatcherPos { + self.backtrack.push(MatcherPos { idx, input_pos: mp.input_pos, matches: Rc::clone(&mp.matches), @@ -587,7 +582,7 @@ impl TtParser { // Try one or more matches of this sequence, by entering it. mp.idx += 1; - self.cur_mps.push(mp); + self.backtrack.push(mp); } &MatcherLoc::SequenceKleeneOpNoSep { op, idx_first } => { // We are past the end of a sequence with no separator. Try ending the sequence. If @@ -598,12 +593,12 @@ impl TtParser { input_pos: mp.input_pos, matches: Rc::clone(&mp.matches), }; - self.cur_mps.push(ending_mp); + self.backtrack.push(ending_mp); if op != KleeneOp::ZeroOrOne { // Try another repetition. mp.idx = idx_first.try_into().unwrap(); - self.cur_mps.push(mp); + self.backtrack.push(mp); } } MatcherLoc::SequenceSep { separator } => { @@ -615,21 +610,26 @@ impl TtParser { input_pos: mp.input_pos, matches: Rc::clone(&mp.matches), }; - self.cur_mps.push(ending_mp); + self.backtrack.push(ending_mp); if token_name_eq(token, separator) { // The separator matches the current token. Advance past it. track.matched_one(mp.input_pos, mp.idx); mp.idx += 1; mp.input_pos += 1; - self.next_mps.push(mp); + if mp.input_pos > parser.approx_token_stream_pos() { + self.seen_tokens.push(parser.token); + parser.to_mut().bump(); + debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); + } + self.backtrack.push(mp); } } &MatcherLoc::SequenceKleeneOpAfterSep { idx_first } => { // We are past the sequence separator. This can't be a `?` Kleene op, because they // don't permit separators. Try another repetition. mp.idx = idx_first.try_into().unwrap(); - self.cur_mps.push(mp); + self.backtrack.push(mp); } &MatcherLoc::MetaVarDecl { kind, .. } => { // Built-in nonterminals never start with these tokens, so we can eliminate them @@ -658,6 +658,8 @@ impl TtParser { return; } + debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); + track.matched_one(mp.input_pos, mp.idx); if self.maybe_ambig_mp.is_some() { @@ -690,7 +692,7 @@ impl TtParser { mp.idx += 1; mp.input_pos = parser.approx_token_stream_pos(); self.seen_tokens.clear(); - self.cur_mps.push(mp); + self.backtrack.push(mp); None } @@ -716,19 +718,15 @@ impl TtParser { // `parse_tt_inner` then processes all of these possible matcher positions and produces // possible next positions into `next_mps`. After some post-processing, the contents of // `next_mps` replenish `cur_mps` and we start over again. - self.cur_mps.clear(); + self.backtrack.clear(); self.seen_tokens.clear(); - self.cur_mps.push(MatcherPos { + self.backtrack.push(MatcherPos { idx: 0, input_pos: parser.approx_token_stream_pos(), matches: Rc::clone(&self.empty_matches), }); loop { - assert!(!self.cur_mps.is_empty()); - self.next_mps.clear(); - - // Parse all mps at the current input position, then progress the parser. let res = self.parse_tt_inner(parser, matcher, track); if let Some(res) = res { From c07eb9fa6bfbcd42d6be58b4c5c2effdd417d8a5 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Fri, 24 Jul 2026 05:11:06 +0200 Subject: [PATCH 11/14] Store current mp in local variable Instead of pushing and popping `backtrack` all the time, try to return an mp for immediate use. --- compiler/rustc_expand/src/mbe/macro_parser.rs | 57 ++++++++++--------- 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index 22a14cde9c37e..87b3eff54a59a 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -484,8 +484,10 @@ impl TtParser { matcher: &'matcher [MatcherLoc], track: &mut T, ) -> Option { - while let Some(mp) = self.backtrack.pop() { - self.match_one(parser, matcher, mp, track); + while let Some(mut mp) = self.backtrack.pop() { + while let Some(next_mp) = self.match_one(parser, matcher, mp, track) { + mp = next_mp; + } debug_assert!(self.backtrack.iter().is_sorted_by_key(|mp| mp.input_pos)); } @@ -516,7 +518,7 @@ impl TtParser { matcher: &'matcher [MatcherLoc], mut mp: MatcherPos, track: &mut T, - ) { + ) -> Option { let matcher_loc = &matcher[mp.idx as usize]; // How far from the latest token are we. let age = parser.approx_token_stream_pos() - mp.input_pos; @@ -548,15 +550,15 @@ impl TtParser { debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); } } else { - return; + return None; } mp.idx += 1; - self.backtrack.push(mp); + Some(mp) } MatcherLoc::Delimited => { // Entering the delimiter is trivial. mp.idx += 1; - self.backtrack.push(mp); + Some(mp) } &MatcherLoc::Sequence { op, @@ -582,35 +584,31 @@ impl TtParser { // Try one or more matches of this sequence, by entering it. mp.idx += 1; - self.backtrack.push(mp); + Some(mp) } &MatcherLoc::SequenceKleeneOpNoSep { op, idx_first } => { - // We are past the end of a sequence with no separator. Try ending the sequence. If - // that's not possible, `ending_mp` will fail quietly when it is processed next time - // around the loop. - let ending_mp = MatcherPos { - idx: mp.idx + 1, // +1 skips the Kleene op - input_pos: mp.input_pos, - matches: Rc::clone(&mp.matches), - }; - self.backtrack.push(ending_mp); - if op != KleeneOp::ZeroOrOne { // Try another repetition. - mp.idx = idx_first.try_into().unwrap(); - self.backtrack.push(mp); + let repeating_mp = MatcherPos { + idx: idx_first.try_into().unwrap(), + input_pos: mp.input_pos, + matches: Rc::clone(&mp.matches), + }; + self.backtrack.push(repeating_mp); } + + // Try ending the sequence. + mp.idx += 1; + Some(mp) } MatcherLoc::SequenceSep { separator } => { // We are past the end of a sequence with a separator but we haven't seen the - // separator yet. Try ending the sequence. If that's not possible, `ending_mp` will - // fail quietly when it is processed next time around the loop. + // separator yet. Try ending the sequence. let ending_mp = MatcherPos { idx: mp.idx + 2, // +2 skips the separator and the Kleene op input_pos: mp.input_pos, matches: Rc::clone(&mp.matches), }; - self.backtrack.push(ending_mp); if token_name_eq(token, separator) { // The separator matches the current token. Advance past it. @@ -622,21 +620,24 @@ impl TtParser { parser.to_mut().bump(); debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); } - self.backtrack.push(mp); + self.backtrack.push(ending_mp); + Some(mp) + } else { + Some(ending_mp) } } &MatcherLoc::SequenceKleeneOpAfterSep { idx_first } => { // We are past the sequence separator. This can't be a `?` Kleene op, because they // don't permit separators. Try another repetition. mp.idx = idx_first.try_into().unwrap(); - self.backtrack.push(mp); + Some(mp) } &MatcherLoc::MetaVarDecl { kind, .. } => { // Built-in nonterminals never start with these tokens, so we can eliminate them // from consideration. We use the span of the metavariable declaration to determine // any edition-specific matching behavior for non-terminals. if !Parser::nonterminal_may_begin_with(kind, token) { - return; + return None; } // EOF tokens would cause unexpected processing in `match_one()`. @@ -649,13 +650,15 @@ impl TtParser { } else { self.maybe_ambig_mp = Some(mp); } + + None } MatcherLoc::Eof => { // We are past the matcher's end, and not in a sequence. Try to end things. debug_assert_eq!(mp.idx as usize, matcher.len() - 1); if *token != token::Eof { - return; + return None; } debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); @@ -667,6 +670,8 @@ impl TtParser { } else { self.maybe_ambig_mp = Some(mp); } + + None } } } From b5dbe2c43b25437332a91b1af44b38c847feb3bd Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Fri, 24 Jul 2026 06:24:51 +0200 Subject: [PATCH 12/14] Merge backtracking logic into `parse_tt()` --- compiler/rustc_expand/src/mbe/macro_parser.rs | 105 ++++++++---------- 1 file changed, 45 insertions(+), 60 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index 87b3eff54a59a..aa4c71340c8d9 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -72,6 +72,7 @@ use std::borrow::Cow; use std::fmt::Display; +use std::ops::ControlFlow; use std::rc::Rc; pub(crate) use NamedMatch::*; @@ -471,43 +472,57 @@ impl TtParser { } } - /// Process the matcher positions of `cur_mps` until it is empty. In the process, this will - /// produce more mps in `next_mps` and `bb_mps`. - /// - /// # Returns - /// - /// `Some(result)` if everything is finished, `None` otherwise. Note that matches are kept - /// track of through the mps generated. - fn parse_tt_inner<'matcher, T: Tracker<'matcher>>( + /// Match the token stream from `parser` against `matcher`. + pub(super) fn parse_tt<'matcher, T: Tracker<'matcher>>( &mut self, parser: &mut Cow<'_, Parser<'_>>, matcher: &'matcher [MatcherLoc], track: &mut T, - ) -> Option { - while let Some(mut mp) = self.backtrack.pop() { - while let Some(next_mp) = self.match_one(parser, matcher, mp, track) { + ) -> NamedParseResult { + self.backtrack.clear(); + self.seen_tokens.clear(); + let mut mp = MatcherPos { + idx: 0, + input_pos: parser.approx_token_stream_pos(), + matches: Rc::clone(&self.empty_matches), + }; + + loop { + if let Some(next_mp) = self.match_one(parser, matcher, mp, track) { mp = next_mp; + continue; } - debug_assert!(self.backtrack.iter().is_sorted_by_key(|mp| mp.input_pos)); - } + // Try backtracking to an older mp. + if let Some(next_mp) = self.backtrack.pop() { + mp = next_mp; + continue; + } - if let Some(mp) = self.maybe_ambig_mp.take() { - if self.found_ambiguity || parser.approx_token_stream_pos() > mp.input_pos { - // Either: - // - A second maybe-ambig mp was found, setting `found_ambiguity` - // - Something else was parsed successfully, advancing `parser` past `mp` - // - `mp` was matched while backtracking - track.ambiguity(); - return Some(Ambiguity); + // Check for a matched meta-variable or EOF. + if let Some(mamp) = self.maybe_ambig_mp.take() { + if self.found_ambiguity || parser.approx_token_stream_pos() > mamp.input_pos { + // Either: + // - A second maybe-ambig mp was found, setting `found_ambiguity` + // - Something else was parsed successfully, advancing `parser` past `mp` + // - `mp` was matched while backtracking + track.ambiguity(); + return Ambiguity; + } + + match self.process_special(parser, matcher, mamp) { + ControlFlow::Break(result) => return result, + ControlFlow::Continue(next_mp) => { + mp = next_mp; + continue; + } + } } - return self.process_special(parser, matcher, mp); + // The invocation could not be matched. + track.failure(parser); + return Failure; } - - // There are no possible next positions: syntax error. - track.failure(parser); - Some(Failure) } /// Match a single [`MatcherPos`]. @@ -682,14 +697,14 @@ impl TtParser { parser: &mut Cow<'_, Parser<'_>>, matcher: &[MatcherLoc], mut mp: MatcherPos, - ) -> Option { + ) -> ControlFlow { let matcher_loc = &matcher[mp.idx as usize]; match matcher_loc { &MatcherLoc::MetaVarDecl { kind, next_metavar, seq_depth, .. } => { // We use the span of the metavariable declaration to determine any // edition-specific matching behavior for non-terminals. let nt = match parser.to_mut().parse_nonterminal(kind) { - Err(err) => return Some(self.nt_parsing_error(matcher_loc, err)), + Err(err) => return ControlFlow::Break(self.nt_parsing_error(matcher_loc, err)), Ok(nt) => nt, }; mp.push_match(next_metavar, seq_depth, MatchedSingle(nt)); @@ -697,49 +712,19 @@ impl TtParser { mp.idx += 1; mp.input_pos = parser.approx_token_stream_pos(); self.seen_tokens.clear(); - self.backtrack.push(mp); - None + ControlFlow::Continue(mp) } MatcherLoc::Eof => { self.seen_tokens.clear(); let matches = Rc::unwrap_or_clone(mp.matches).into_iter(); - Some(Success(self.nameize(matcher, matches))) + ControlFlow::Break(Success(self.nameize(matcher, matches))) } _ => unreachable!(), } } - /// Match the token stream from `parser` against `matcher`. - pub(super) fn parse_tt<'matcher, T: Tracker<'matcher>>( - &mut self, - parser: &mut Cow<'_, Parser<'_>>, - matcher: &'matcher [MatcherLoc], - track: &mut T, - ) -> NamedParseResult { - // A queue of possible matcher positions. We initialize it with the matcher position in - // which the "dot" is before the first token of the first token tree in `matcher`. - // `parse_tt_inner` then processes all of these possible matcher positions and produces - // possible next positions into `next_mps`. After some post-processing, the contents of - // `next_mps` replenish `cur_mps` and we start over again. - self.backtrack.clear(); - self.seen_tokens.clear(); - self.backtrack.push(MatcherPos { - idx: 0, - input_pos: parser.approx_token_stream_pos(), - matches: Rc::clone(&self.empty_matches), - }); - - loop { - let res = self.parse_tt_inner(parser, matcher, track); - - if let Some(res) = res { - return res; - } - } - } - fn nt_parsing_error(&self, loc: &MatcherLoc, err: Diag<'_>) -> ParseResult { let &MatcherLoc::MetaVarDecl { span, kind, .. } = loc else { unreachable!() }; let guarantee = err From 1859c23ee4a41e6e85b660ccc2592771317e521c Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Fri, 24 Jul 2026 05:41:59 +0200 Subject: [PATCH 13/14] Update `macro_parser.rs` docs for DFS --- compiler/rustc_expand/src/mbe/macro_parser.rs | 106 ++++++++---------- 1 file changed, 48 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index aa4c71340c8d9..3dc1fb9ba1843 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -1,74 +1,64 @@ -//! This is an NFA-based parser, which calls out to the main Rust parser for named non-terminals -//! (which it commits to fully when it hits one in a grammar). There's a set of current NFA threads -//! and a set of next ones. Instead of NTs, we have a special case for Kleene star. The big-O, in -//! pathological cases, is worse than traditional use of NFA or Earley parsing, but it's an easier -//! fit for Macro-by-Example-style rules. +//! Parsing macros-by-example invocations. //! -//! (In order to prevent the pathological case, we'd need to lazily construct the resulting -//! `NamedMatch`es at the very end. It'd be a pain, and require more memory to keep around old -//! matcher positions, but it would also save overhead) +//! The MBE macro matcher language allows for some limited ambiguity: //! -//! We don't say this parser uses the Earley algorithm, because it's unnecessarily inaccurate. -//! The macro parser restricts itself to the features of finite state automata. Earley parsers -//! can be described as an extension of NFAs with completion rules, prediction rules, and recursion. -//! -//! Quick intro to how the parser works: -//! -//! A "matcher position" (a.k.a. "position" or "mp") is a dot in the middle of a matcher, usually -//! written as a `·`. For example `· a $( a )* a b` is one, as is `a $( · a )* a b`. -//! -//! The parser walks through the input a token at a time, maintaining a list -//! of threads consistent with the current position in the input string: `cur_mps`. -//! -//! As it processes them, it fills up `eof_mps` with threads that would be valid if -//! the macro invocation is now over, `bb_mps` with threads that are waiting on -//! a Rust non-terminal like `$e:expr`, and `next_mps` with threads that are waiting -//! on a particular token. Most of the logic concerns moving the · through the -//! repetitions indicated by Kleene stars. The rules for moving the · without -//! consuming any input are called epsilon transitions. It only advances or calls -//! out to the real Rust parser when no `cur_mps` threads remain. -//! -//! Example: -//! -//! ```text, ignore -//! Start parsing a a a a b against [· a $( a )* a b]. +//! ``` +//! macro_rules! foo { +//! ($(,)? , $(,)?) => {}; +//! } //! -//! Remaining input: a a a a b -//! next: [· a $( a )* a b] +//! foo!(,); // can be parsed unambiguously +//! //foo!(,,); // fails to compile due to ambiguity +//! ``` //! -//! - - - Advance over an a. - - - +//! When a repetition or optional matcher is encountered, the macro parser will not prioritize one +//! possibility over another (as occurs with e.g. PEG); it will explore all possibilities. If there +//! are multiple ways to parse the macro invocation, an ambiguity error is raised. //! -//! Remaining input: a a a b -//! cur: [a · $( a )* a b] -//! Descend/Skip (first position). -//! next: [a $( · a )* a b] [a $( a )* · a b]. +//! The possible ways to parse an input can be visualized as a tree, where the root represents the +//! start of parsing, and the children of each node are the parsing steps that follow from it. For +//! the above macro, that would look like: //! -//! - - - Advance over an a. - - - +//! ```text +//! start - token ',' - token ',' - token ',' - eof +//! | \\ +//! | skip ------ eof +//! \\ +//! skip ------ token ',' - token ',' - eof +//! \\ +//! skip ------ eof +//! ``` //! -//! Remaining input: a a b -//! cur: [a $( a · )* a b] [a $( a )* a · b] -//! Follow epsilon transition: Finish/Repeat (first position) -//! next: [a $( a )* · a b] [a $( · a )* a b] [a $( a )* a · b] +//! This module implements a depth-first, backtracking traversal of that tree. It maintains a +//! stack of `MatcherPos`-es (i.e. mps), which represent paths taken through the tree. It will +//! continuously expand the latest `MatcherPos`, backtracking if the mp has no more children to +//! explore. //! -//! - - - Advance over an a. - - - (this looks exactly like the last step) +//! An important caveat is that meta-variables, e.g. `$e:expr`, require unambiguity to be parsed. No +//! other `MatcherPos`-es are allowed to match the same tokens as those consumed by a meta-variable; +//! doing so raises an ambiguity error. //! -//! Remaining input: a b -//! cur: [a $( a · )* a b] [a $( a )* a · b] -//! Follow epsilon transition: Finish/Repeat (first position) -//! next: [a $( a )* · a b] [a $( · a )* a b] [a $( a )* a · b] +//! ``` +//! macro_rules! foo { +//! ($(a)? $x:ident b) => {}; +//! } //! -//! - - - Advance over an a. - - - (this looks exactly like the last step) +//! foo!(b b); // can be parsed unambiguously +//! //foo!(a b); // fails to compile due to ambiguity +//! ``` //! -//! Remaining input: b -//! cur: [a $( a · )* a b] [a $( a )* a · b] -//! Follow epsilon transition: Finish/Repeat (first position) -//! next: [a $( a )* · a b] [a $( · a )* a b] [a $( a )* a · b] +//! In theory, the latter invocation could be parsed unambiguously. But, at the first input position +//! where a meta-variable needs to be matched (matching `$x` against `a`), another path through the +//! parse tree is valid (matching `a` in `$(a)?` against `a`), and this is not allowed. //! -//! - - - Advance over a b. - - - +//! # Pathological Behavior //! -//! Remaining input: '' -//! eof: [a $( a )* a b ·] -//! ``` +//! It is possible to construct macros which require an exponential runtime to parse. This is +//! because we don't deduplicate equivalent mps, or cache parsing results. Pathological macros are +//! very rare in the real world. While they could be handled in linear time like everything else, +//! doing so would add unnecessary overhead. We could retain the existing parsing algorithm and +//! switch to a guaranteed-linear-time alternative for a particular macro invocation if it takes +//! more than N parsing steps. use std::borrow::Cow; use std::fmt::Display; From 807fd416162aa3a6e993c03dbdbac534687cc117 Mon Sep 17 00:00:00 2001 From: arya dradjica Date: Fri, 24 Jul 2026 09:39:11 +0200 Subject: [PATCH 14/14] wip: wrangle perf, try 1 --- compiler/rustc_expand/src/mbe/diagnostics.rs | 10 ++ compiler/rustc_expand/src/mbe/macro_parser.rs | 156 ++++++++++-------- compiler/rustc_expand/src/mbe/macro_rules.rs | 11 ++ 3 files changed, 110 insertions(+), 67 deletions(-) diff --git a/compiler/rustc_expand/src/mbe/diagnostics.rs b/compiler/rustc_expand/src/mbe/diagnostics.rs index f2cd175342a8f..dba3f2b2da85d 100644 --- a/compiler/rustc_expand/src/mbe/diagnostics.rs +++ b/compiler/rustc_expand/src/mbe/diagnostics.rs @@ -165,6 +165,8 @@ struct CollectTrackerAndEmitter<'dcx, 'matcher> { /// Tokens seen during parsing. tokens: FxHashMap, + input_pos_offset: u32, + /// Which arm's failure should we report? (the one furthest along) best_failure: Option, root_span: Span, @@ -219,6 +221,7 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match } fn trying_match(&mut self, input_pos: u32, token: &Token, loc_index: u32) { + let input_pos = self.input_pos_offset + input_pos; let old_token = self.tokens.insert(input_pos, *token); debug_assert!(old_token.is_none_or(|t| t == *token)); @@ -229,12 +232,17 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match } fn matched_one(&mut self, input_pos: u32, loc_index: u32) { + let input_pos = self.input_pos_offset + input_pos; let m = Match { input_pos, loc_index }; let match_result = self.matches.get_mut(&m).unwrap_or_else(|| bug!("no corresponding `trying_match()`")); *match_result = MatchResult::Success; } + fn reset_input_pos(&mut self, parser: &Parser<'_>) { + self.input_pos_offset = parser.approx_token_stream_pos(); + } + fn after_arm(&mut self, result: &NamedParseResult) { match *result { Success(_) => { @@ -261,6 +269,7 @@ impl<'dcx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'dcx, 'match self.current = None; self.matches.clear(); self.tokens.clear(); + self.input_pos_offset = 0; } fn failure(&mut self, parser: &Parser<'_>) { @@ -406,6 +415,7 @@ impl<'dcx> CollectTrackerAndEmitter<'dcx, '_> { current: None, matches: FxHashMap::default(), tokens: FxHashMap::default(), + input_pos_offset: 0, best_failure: None, root_span, result: None, diff --git a/compiler/rustc_expand/src/mbe/macro_parser.rs b/compiler/rustc_expand/src/mbe/macro_parser.rs index 3dc1fb9ba1843..a8d9e02ca4723 100644 --- a/compiler/rustc_expand/src/mbe/macro_parser.rs +++ b/compiler/rustc_expand/src/mbe/macro_parser.rs @@ -471,67 +471,64 @@ impl TtParser { ) -> NamedParseResult { self.backtrack.clear(); self.seen_tokens.clear(); - let mut mp = MatcherPos { - idx: 0, - input_pos: parser.approx_token_stream_pos(), - matches: Rc::clone(&self.empty_matches), - }; + let mut mp = MatcherPos { idx: 0, input_pos: 0, matches: Rc::clone(&self.empty_matches) }; loop { - if let Some(next_mp) = self.match_one(parser, matcher, mp, track) { - mp = next_mp; - continue; + match self.match_one(parser, matcher, mp, track) { + ControlFlow::Continue(Some(next_mp)) => { + mp = next_mp; + continue; + } + ControlFlow::Continue(None) => {} + ControlFlow::Break(result) => { + std::hint::cold_path(); + return result; + } } - // Try backtracking to an older mp. - if let Some(next_mp) = self.backtrack.pop() { - mp = next_mp; - continue; + // Check for a matched meta-variable or EOF. + let Some(mamp) = self.maybe_ambig_mp.take() else { + // There was no valid way to parse the input. + std::hint::cold_path(); + track.failure(parser); + return Failure; + }; + + if self.found_ambiguity || self.seen_tokens.len() > mamp.input_pos as usize { + // Either: + // - A second maybe-ambig mp was found, setting `found_ambiguity` + // - Something else was parsed successfully, advancing `parser` past `mp` + // - `mp` was matched while backtracking + std::hint::cold_path(); + track.ambiguity(); + return Ambiguity; } - // Check for a matched meta-variable or EOF. - if let Some(mamp) = self.maybe_ambig_mp.take() { - if self.found_ambiguity || parser.approx_token_stream_pos() > mamp.input_pos { - // Either: - // - A second maybe-ambig mp was found, setting `found_ambiguity` - // - Something else was parsed successfully, advancing `parser` past `mp` - // - `mp` was matched while backtracking - track.ambiguity(); - return Ambiguity; + match self.process_special(parser, matcher, mamp, track) { + ControlFlow::Break(result) => { + std::hint::cold_path(); + return result; } - - match self.process_special(parser, matcher, mamp) { - ControlFlow::Break(result) => return result, - ControlFlow::Continue(next_mp) => { - mp = next_mp; - continue; - } + ControlFlow::Continue(next_mp) => { + mp = next_mp; + continue; } } - - // The invocation could not be matched. - track.failure(parser); - return Failure; } } /// Match a single [`MatcherPos`]. - #[inline(always)] // must be inlined in `parse_tt_inner()` + #[inline(always)] // must be inlined in `parse_tt()` fn match_one<'matcher, T: Tracker<'matcher>>( &mut self, parser: &mut Cow<'_, Parser<'_>>, matcher: &'matcher [MatcherLoc], mut mp: MatcherPos, track: &mut T, - ) -> Option { + ) -> ControlFlow> { let matcher_loc = &matcher[mp.idx as usize]; - // How far from the latest token are we. - let age = parser.approx_token_stream_pos() - mp.input_pos; - let token = if age == 0 { - &parser.token - } else { - &self.seen_tokens[self.seen_tokens.len() - age as usize] - }; + let input_pos = mp.input_pos as usize; + let token = self.seen_tokens.get(input_pos).unwrap_or(&parser.token); track.trying_match(mp.input_pos, token, mp.idx); match matcher_loc { @@ -545,25 +542,25 @@ impl TtParser { // Otherwise, this match has failed, there is nothing to do, and hopefully another // mp in `cur_mps` will match. if matches!(t, Token { kind: DocComment(..), .. }) { + std::hint::cold_path(); // skip } else if token_name_eq(t, token) { track.matched_one(mp.input_pos, mp.idx); mp.input_pos += 1; - if mp.input_pos > parser.approx_token_stream_pos() { + if mp.input_pos as usize > self.seen_tokens.len() { self.seen_tokens.push(parser.token); parser.to_mut().bump(); - debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); } } else { - return None; + return ControlFlow::Continue(self.backtrack.pop()); } mp.idx += 1; - Some(mp) + ControlFlow::Continue(Some(mp)) } MatcherLoc::Delimited => { // Entering the delimiter is trivial. mp.idx += 1; - Some(mp) + ControlFlow::Continue(Some(mp)) } &MatcherLoc::Sequence { op, @@ -589,7 +586,7 @@ impl TtParser { // Try one or more matches of this sequence, by entering it. mp.idx += 1; - Some(mp) + ControlFlow::Continue(Some(mp)) } &MatcherLoc::SequenceKleeneOpNoSep { op, idx_first } => { if op != KleeneOp::ZeroOrOne { @@ -604,7 +601,7 @@ impl TtParser { // Try ending the sequence. mp.idx += 1; - Some(mp) + ControlFlow::Continue(Some(mp)) } MatcherLoc::SequenceSep { separator } => { // We are past the end of a sequence with a separator but we haven't seen the @@ -620,29 +617,28 @@ impl TtParser { track.matched_one(mp.input_pos, mp.idx); mp.idx += 1; mp.input_pos += 1; - if mp.input_pos > parser.approx_token_stream_pos() { + if mp.input_pos as usize > self.seen_tokens.len() { self.seen_tokens.push(parser.token); parser.to_mut().bump(); - debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); } self.backtrack.push(ending_mp); - Some(mp) + ControlFlow::Continue(Some(mp)) } else { - Some(ending_mp) + ControlFlow::Continue(Some(ending_mp)) } } &MatcherLoc::SequenceKleeneOpAfterSep { idx_first } => { // We are past the sequence separator. This can't be a `?` Kleene op, because they // don't permit separators. Try another repetition. mp.idx = idx_first.try_into().unwrap(); - Some(mp) + ControlFlow::Continue(Some(mp)) } - &MatcherLoc::MetaVarDecl { kind, .. } => { + &MatcherLoc::MetaVarDecl { kind, next_metavar, seq_depth, .. } => { // Built-in nonterminals never start with these tokens, so we can eliminate them // from consideration. We use the span of the metavariable declaration to determine // any edition-specific matching behavior for non-terminals. if !Parser::nonterminal_may_begin_with(kind, token) { - return None; + return ControlFlow::Continue(self.backtrack.pop()); } // EOF tokens would cause unexpected processing in `match_one()`. @@ -650,43 +646,68 @@ impl TtParser { track.matched_one(mp.input_pos, mp.idx); - if self.maybe_ambig_mp.is_some() { + if self.maybe_ambig_mp.is_some() || input_pos < self.seen_tokens.len() { + std::hint::cold_path(); self.found_ambiguity = true; - } else { + return ControlFlow::Continue(self.backtrack.pop()); + } else if let Some(next_mp) = self.backtrack.pop() { + std::hint::cold_path(); self.maybe_ambig_mp = Some(mp); + return ControlFlow::Continue(Some(next_mp)); } - None + // We use the span of the metavariable declaration to determine any + // edition-specific matching behavior for non-terminals. + let nt = match parser.to_mut().parse_nonterminal(kind) { + Err(err) => { + std::hint::cold_path(); + return ControlFlow::Break(self.nt_parsing_error(matcher_loc, err)); + } + Ok(nt) => nt, + }; + mp.push_match(next_metavar, seq_depth, MatchedSingle(nt)); + + mp.idx += 1; + mp.input_pos = 0; + self.seen_tokens.clear(); + track.reset_input_pos(parser); + ControlFlow::Continue(Some(mp)) } MatcherLoc::Eof => { // We are past the matcher's end, and not in a sequence. Try to end things. debug_assert_eq!(mp.idx as usize, matcher.len() - 1); if *token != token::Eof { - return None; + return ControlFlow::Continue(self.backtrack.pop()); } - debug_assert_eq!(mp.input_pos, parser.approx_token_stream_pos()); - track.matched_one(mp.input_pos, mp.idx); - if self.maybe_ambig_mp.is_some() { + if self.maybe_ambig_mp.is_some() || input_pos < self.seen_tokens.len() { + std::hint::cold_path(); self.found_ambiguity = true; - } else { + return ControlFlow::Continue(self.backtrack.pop()); + } else if let Some(next_mp) = self.backtrack.pop() { + std::hint::cold_path(); self.maybe_ambig_mp = Some(mp); + return ControlFlow::Continue(Some(next_mp)); } - None + self.seen_tokens.clear(); + let matches = Rc::unwrap_or_clone(mp.matches).into_iter(); + ControlFlow::Break(Success(self.nameize(matcher, matches))) } } } /// Finish processing a matched special [`MatcherPos`]. - fn process_special( + #[cold] + fn process_special<'matcher, T: Tracker<'matcher>>( &mut self, parser: &mut Cow<'_, Parser<'_>>, - matcher: &[MatcherLoc], + matcher: &'matcher [MatcherLoc], mut mp: MatcherPos, + track: &mut T, ) -> ControlFlow { let matcher_loc = &matcher[mp.idx as usize]; match matcher_loc { @@ -700,8 +721,9 @@ impl TtParser { mp.push_match(next_metavar, seq_depth, MatchedSingle(nt)); mp.idx += 1; - mp.input_pos = parser.approx_token_stream_pos(); + mp.input_pos = 0; self.seen_tokens.clear(); + track.reset_input_pos(parser); ControlFlow::Continue(mp) } diff --git a/compiler/rustc_expand/src/mbe/macro_rules.rs b/compiler/rustc_expand/src/mbe/macro_rules.rs index 05cb4e49314d4..f2c35949a7d7f 100644 --- a/compiler/rustc_expand/src/mbe/macro_rules.rs +++ b/compiler/rustc_expand/src/mbe/macro_rules.rs @@ -373,6 +373,8 @@ pub(super) trait Tracker<'matcher> { /// [`Parser::nonterminal_may_begin_with()`] returns `true`). fn matched_one(&mut self, input_pos: u32, loc_index: u32); + fn reset_input_pos(&mut self, parser: &Parser<'_>); + /// This is called after an arm has been parsed, either successfully or unsuccessfully. When /// this is called, `before_match_loc` was called at least once (with a `MatcherLoc::Eof`). fn after_arm(&mut self, result: &NamedParseResult); @@ -402,16 +404,25 @@ pub(super) trait Tracker<'matcher> { pub(super) struct NoopTracker; impl<'matcher> Tracker<'matcher> for NoopTracker { + #[inline] fn prepare(&mut self, _which_matcher: WhichMatcher, _matcher: &'matcher [MatcherLoc]) {} + #[inline] fn trying_match(&mut self, _input_pos: u32, _token: &Token, _loc_index: u32) {} + #[inline] fn matched_one(&mut self, _input_pos: u32, _loc_index: u32) {} + #[inline] fn ambiguity(&mut self) {} + #[inline] + fn reset_input_pos(&mut self, _parser: &Parser<'_>) {} + + #[inline] fn after_arm(&mut self, _result: &NamedParseResult) {} + #[inline] fn failure(&mut self, _parser: &Parser<'_>) {} fn description() -> &'static str {