From fad0a62a354fb634379952e4097c43708bb6af22 Mon Sep 17 00:00:00 2001 From: Will Lillis Date: Sun, 9 Aug 2026 11:51:45 -0500 Subject: [PATCH] perf(generate): pool parse-table action lists Every (state, terminal) parse-table entry stored its action list inline as a 32-byte `ParseTableEntry`, but across a grammar those lists are ~98-99% _duplicates_. The number of distinct lists is a few thousand regardless of grammar size, while total entries scale into the hundreds of thousands. Store each unique action list once in a shared `ActionListPool` (a flat arena of actions plus `(offset, len)` ranges) and replace the inline entry with a 4-byte `ActionListId` (a pool index with the `reusable` flag packed into the high bit). - intern_table converts the freshly built `ParseTable` into `ParseTable`. - minimize carries and operates on the 4-byte ids. The three global state renumberings rewrite Shift targets once at the pool level (`remap_terminal_references`), while the per-state unit-reduction redirects copy the changed list into a new slot (COW). `mark_fragile_tokens` becomes a free bit flip on the id. - `canonicalize` dedups and compacts the pool once before `render`, dropping the dead and duplicate slots the remaps and COW leave behind. `render` assigns the output action-list offsets directly. Yields ~7% wall time reduction, ~12% peak rss reduction. --- crates/generate/src/build_tables.rs | 14 +- .../src/build_tables/build_parse_table.rs | 11 +- .../src/build_tables/coincident_tokens.rs | 4 +- .../src/build_tables/minimize_parse_table.rs | 95 ++++-- crates/generate/src/render.rs | 44 +-- crates/generate/src/tables.rs | 280 +++++++++++++++--- 6 files changed, 359 insertions(+), 89 deletions(-) diff --git a/crates/generate/src/build_tables.rs b/crates/generate/src/build_tables.rs index 1c3c39ac0..2ad41de6c 100644 --- a/crates/generate/src/build_tables.rs +++ b/crates/generate/src/build_tables.rs @@ -30,7 +30,7 @@ use crate::{ node_types::VariableInfo, rules::{AliasMap, Symbol, SymbolType, TokenSet}, strpool::StrPool, - tables::{ActionList, LexTable, ParseAction, ParseTable, ParseTableEntry}, + tables::{ActionList, ActionListPool, LexTable, ParseAction, ParseTable, ParseTableEntry}, }; pub struct Tables { @@ -87,6 +87,7 @@ pub fn build_tables( str_pool, ); populate_used_symbols(&mut parse_table, syntax_grammar, lexical_grammar); + let mut parse_table = ActionListPool::intern_table(parse_table); minimize_parse_table( &mut parse_table, syntax_grammar, @@ -107,6 +108,9 @@ pub fn build_tables( ); populate_external_lex_states(&mut parse_table, syntax_grammar); mark_fragile_tokens(&mut parse_table, &token_conflict_map); + parse_table + .action_lists + .canonicalize(&mut parse_table.states); if let Some(report_symbol_name) = report_symbol_name { report_state_info( @@ -170,7 +174,7 @@ fn get_following_tokens( } fn populate_error_state( - parse_table: &mut ParseTable, + parse_table: &mut ParseTable, syntax_grammar: &SyntaxGrammar, lexical_grammar: &LexicalGrammar, coincident_token_index: &CoincidentTokenIndex, @@ -249,7 +253,7 @@ fn populate_error_state( } fn populate_used_symbols( - parse_table: &mut ParseTable, + parse_table: &mut ParseTable, syntax_grammar: &SyntaxGrammar, lexical_grammar: &LexicalGrammar, ) { @@ -446,11 +450,11 @@ fn mark_fragile_tokens(parse_table: &mut ParseTable, token_conflict_map: &TokenC valid_terminal_indices.push(token.index); } } - for (token, entry) in &mut state.terminal_entries { + for (token, id) in &mut state.terminal_entries { if token.is_terminal() { for &i in &valid_terminal_indices { if token_conflict_map.does_overlap(i as usize, token.index as usize) { - entry.reusable = false; + id.set_reusable(false); break; } } diff --git a/crates/generate/src/build_tables/build_parse_table.rs b/crates/generate/src/build_tables/build_parse_table.rs index 0aee5f6d9..ff59fd1c0 100644 --- a/crates/generate/src/build_tables/build_parse_table.rs +++ b/crates/generate/src/build_tables/build_parse_table.rs @@ -21,8 +21,8 @@ use crate::{ rules::{Associativity, Precedence, Symbol, SymbolType, TokenSet}, strpool::StrPool, tables::{ - ActionList, FieldLocation, GotoAction, ParseAction, ParseState, ParseStateId, ParseTable, - ParseTableEntry, ProductionInfo, ProductionInfoId, + ActionList, ActionListPool, FieldLocation, GotoAction, ParseAction, ParseState, + ParseStateId, ParseTable, ParseTableEntry, ProductionInfo, ProductionInfoId, }, }; @@ -77,7 +77,7 @@ struct ParseTableBuilder<'a> { parse_state_queue: VecDeque, non_terminal_extra_states: Vec<(Symbol, ParseStateId)>, actual_conflicts: FxHashSet>, - parse_table: ParseTable, + parse_table: ParseTable, str_pool: &'a StrPool, } @@ -276,6 +276,7 @@ impl<'a> ParseTableBuilder<'a> { actual_conflicts: syntax_grammar.expected_conflicts.iter().cloned().collect(), parse_table: ParseTable { states: Vec::new(), + action_lists: ActionListPool::default(), symbols: Vec::new(), external_lex_states: Vec::new(), production_infos: Vec::new(), @@ -288,7 +289,7 @@ impl<'a> ParseTableBuilder<'a> { fn build( mut self, diagnostics: &mut Vec, - ) -> BuildTableResult<(ParseTable, ParseStateInfo<'a>)> { + ) -> BuildTableResult<(ParseTable, ParseStateInfo<'a>)> { // Ensure that the empty alias sequence has index 0. self.parse_table .production_infos @@ -1252,7 +1253,7 @@ pub fn build_parse_table<'a>( variable_info: &'a [VariableInfo], str_pool: &'a StrPool, diagnostics: &mut Vec, -) -> BuildTableResult<(ParseTable, ParseStateInfo<'a>)> { +) -> BuildTableResult<(ParseTable, ParseStateInfo<'a>)> { ParseTableBuilder::new( syntax_grammar, lexical_grammar, diff --git a/crates/generate/src/build_tables/coincident_tokens.rs b/crates/generate/src/build_tables/coincident_tokens.rs index 5066fadb4..ff02a497b 100644 --- a/crates/generate/src/build_tables/coincident_tokens.rs +++ b/crates/generate/src/build_tables/coincident_tokens.rs @@ -18,8 +18,8 @@ pub struct CoincidentTokenIndex { impl<'a> CoincidentTokenIndex { #[must_use] - pub fn new( - table: &ParseTable, + pub fn new( + table: &ParseTable, lexical_grammar: &'a LexicalGrammar, word_token: Option, ) -> Self { diff --git a/crates/generate/src/build_tables/minimize_parse_table.rs b/crates/generate/src/build_tables/minimize_parse_table.rs index 28f24193c..7011e550c 100644 --- a/crates/generate/src/build_tables/minimize_parse_table.rs +++ b/crates/generate/src/build_tables/minimize_parse_table.rs @@ -11,7 +11,9 @@ use crate::{ grammars::{LexicalGrammar, SyntaxGrammar, VariableType}, rules::{AliasMap, Symbol, SymbolType, TokenSet}, strpool::StrPool, - tables::{GotoAction, ParseAction, ParseState, ParseStateId, ParseTable, ParseTableEntry}, + tables::{ + ActionList, ActionListId, GotoAction, ParseAction, ParseState, ParseStateId, ParseTable, + }, }; /// Index into [`SyntaxGrammar::variables`]. All nonterminal [`Symbol`]s share @@ -159,8 +161,8 @@ impl Minimizer<'_> { for (i, state) in self.parse_table.states.iter().enumerate() { let mut only_unit_reductions = true; let mut unit_reduction_symbol = None; - for (_, entry) in &state.terminal_entries { - for action in &entry.actions { + for (_, id) in &state.terminal_entries { + for action in self.parse_table.action_lists.get(*id) { match action { ParseAction::ShiftExtra => continue, ParseAction::Reduce { @@ -198,11 +200,23 @@ impl Minimizer<'_> { } } - for state in &mut self.parse_table.states { + if unit_reduction_symbols_by_state.is_empty() { + return; + } + + let mut action_list_ids = FxHashMap::default(); + for state_index in 0..self.parse_table.states.len() { let mut done = false; while !done { done = true; - state.update_referenced_states(|other_state_id, state| { + let ParseTable { + states, + action_lists, + .. + } = self.parse_table; + let state = &mut states[state_index]; + + state.update_nonterminal_references(|other_state_id, state| { unit_reduction_symbols_by_state.get(&other_state_id).map_or( other_state_id, |symbol| { @@ -214,6 +228,32 @@ impl Minimizer<'_> { }, ) }); + + for i in 0..state.terminal_entries.len() { + let old_id = state.terminal_entries.get_index(i).unwrap().1; + let mut actions = ActionList::from_slice(action_lists.get(*old_id)); + let mut changed = false; + for action in &mut *actions { + // A Shift onto a unit-reduction state (one whose only action reduces a + // single `symbol`) can skip it. Shift then reduce then goto is equivalent + // to shifting straight to the the goto target for `symbol` in this case. + if let ParseAction::Shift { state: target, .. } = action + && let Some(symbol) = unit_reduction_symbols_by_state.get(target) + && let Some(GotoAction::Goto(new_target)) = + state.nonterminal_entries.get(symbol) + && *new_target != *target + { + *target = *new_target; + changed = true; + done = false; + } + } + if changed { + let index = action_lists.intern(&mut action_list_ids, actions); + *state.terminal_entries.get_index_mut(i).unwrap().1 = + ActionListId::new(index, old_id.reusable()); + } + } } } } @@ -239,9 +279,8 @@ impl Minimizer<'_> { } // Precompute sorted terminal entry references for merge-join in states_conflict. - // entry_maps[state_id][i] = (symbol_key, &ParseTableEntry) - // Keys are packed u64s (symbol_key) for single-instruction comparison. - // Storing a reference avoids the IndexMap::get_index call in states_conflict. + // entry_maps[state_id][i] = (symbol_key, action_list_id). Keys are packed u64s + // (symbol_key) for easy comparison. let entry_maps = self .parse_table .states @@ -250,8 +289,8 @@ impl Minimizer<'_> { let mut entries = state .terminal_entries .iter() - .map(|(sym, entry)| (SymbolKey::new(*sym), entry)) - .collect::>(); + .map(|(sym, id)| (SymbolKey::new(*sym), *id)) + .collect::>(); entries.sort_unstable_by_key(|&(key, _)| key); entries }) @@ -330,7 +369,7 @@ impl Minimizer<'_> { .terminal_entries .iter() .filter_map(|(sym, entry)| { - let action = entry.actions.last()?; + let action = self.parse_table.action_lists.get(*entry).last()?; if let ParseAction::Shift { state: s, .. } = action { Some((SymbolKey::new(*sym), *s)) } else { @@ -407,12 +446,15 @@ impl Minimizer<'_> { } // Update the new state's outgoing references using the new grouping. - parse_state - .update_referenced_states(|state_id, _| group_ids_by_state_id[state_id as usize]); + parse_state.update_nonterminal_references(|state_id, _| { + group_ids_by_state_id[state_id as usize] + }); new_states.push(parse_state); } self.parse_table.states = new_states; + self.parse_table + .remap_terminal_references(|state_id| group_ids_by_state_id[state_id as usize]); } fn states_conflict( @@ -420,7 +462,7 @@ impl Minimizer<'_> { state1: &ParseState, state2: &ParseState, group_ids_by_state_id: &[ParseStateId], - entry_maps: &[Vec<(SymbolKey, &ParseTableEntry)>], + entry_maps: &[Vec<(SymbolKey, ActionListId)>], bits: &ConflictBits, ) -> bool { let entries1 = &entry_maps[state1.id as usize]; @@ -564,13 +606,16 @@ impl Minimizer<'_> { state_id1: ParseStateId, state_id2: ParseStateId, token: Symbol, - entry1: &ParseTableEntry, - entry2: &ParseTableEntry, + id1: ActionListId, + id2: ActionListId, group_ids_by_state_id: &[ParseStateId], ) -> bool { // To be compatible, entries need to have the same actions. - let actions1 = &entry1.actions; - let actions2 = &entry2.actions; + if id1.index() == id2.index() { + return false; + } + let actions1 = self.parse_table.action_lists.get(id1); + let actions2 = self.parse_table.action_lists.get(id2); if actions1.len() != actions2.len() { debug!( "split states {state_id1} {state_id2} - differing action counts for token {}", @@ -714,7 +759,7 @@ impl Minimizer<'_> { state_usage_map[1] = true; for state in &self.parse_table.states { - for referenced_state in state.referenced_states() { + for referenced_state in state.referenced_states(&self.parse_table.action_lists) { state_usage_map[referenced_state as usize] = true; } } @@ -730,15 +775,17 @@ impl Minimizer<'_> { let mut original_state_id = 0; while state_id < self.parse_table.states.len() { if state_usage_map[original_state_id] { - self.parse_table.states[state_id].update_referenced_states(|other_state_id, _| { - state_replacement_map[other_state_id as usize] - }); + self.parse_table.states[state_id].update_nonterminal_references( + |other_state_id, _| state_replacement_map[other_state_id as usize], + ); state_id += 1; } else { self.parse_table.states.remove(state_id); } original_state_id += 1; } + self.parse_table + .remap_terminal_references(|state_id| state_replacement_map[state_id as usize]); } fn reorder_states_by_descending_size(&mut self) { @@ -768,9 +815,11 @@ impl Minimizer<'_> { .map(|old_id| { let mut state = ParseState::default(); mem::swap(&mut state, &mut self.parse_table.states[*old_id]); - state.update_referenced_states(|id, _| new_ids_by_old_id[id as usize]); + state.update_nonterminal_references(|id, _| new_ids_by_old_id[id as usize]); state }) .collect(); + self.parse_table + .remap_terminal_references(|id| new_ids_by_old_id[id as usize]); } } diff --git a/crates/generate/src/render.rs b/crates/generate/src/render.rs index 6d03a717e..1d4e5b099 100644 --- a/crates/generate/src/render.rs +++ b/crates/generate/src/render.rs @@ -10,6 +10,8 @@ use rustc_hash::{FxHashMap, FxHashSet}; use serde::{Deserialize, Serialize}; use thiserror::Error; +use crate::tables::{ActionListId, ActionListPool}; + use super::{ LANGUAGE_VERSION, build_tables::Tables, @@ -20,8 +22,7 @@ use super::{ rules::{AliasMap, Symbol, SymbolType, TokenSet}, strpool::{StrId, StrPool}, tables::{ - ActionList, AdvanceAction, FieldLocation, GotoAction, LexState, LexTable, ParseAction, - ParseTable, ParseTableEntry, + AdvanceAction, FieldLocation, GotoAction, LexState, LexTable, ParseAction, ParseTable, }, }; @@ -1309,11 +1310,10 @@ impl Generator { let mut next_parse_action_list_index = 0u32; // Parse action lists zero is for the default value, when a symbol is not valid. + // `canonicalize` guarantees pool index 0 is the empty list. Self::get_parse_action_list_id( - &ParseTableEntry { - actions: ActionList::Empty, - reusable: false, - }, + ActionListId::new(0, false), + &self.parse_table.action_lists, &mut parse_table_entries, &mut next_parse_action_list_index, ); @@ -1358,9 +1358,10 @@ impl Generator { ); } - for (symbol, entry) in &terminal_entries { + for (symbol, id) in &terminal_entries { let entry_id = Self::get_parse_action_list_id( - entry, + **id, + &self.parse_table.action_lists, &mut parse_table_entries, &mut next_parse_action_list_index, ); @@ -1400,7 +1401,8 @@ impl Generator { // in order to avoid repeating the action. for (symbol, entry) in &terminal_entries { let entry_id = Self::get_parse_action_list_id( - entry, + **entry, + &self.parse_table.action_lists, &mut parse_table_entries, &mut next_parse_action_list_index, ); @@ -1482,7 +1484,7 @@ impl Generator { let mut parse_table_entries = parse_table_entries .into_iter() - .map(|(entry, i)| (i, entry)) + .map(|(id, i)| (i, id)) .collect::>(); parse_table_entries.sort_by_key(|(index, _)| *index); self.add_parse_action_list(parse_table_entries); @@ -1490,20 +1492,21 @@ impl Generator { Ok(()) } - fn add_parse_action_list(&mut self, parse_table_entries: Vec<(u32, ParseTableEntry)>) { + fn add_parse_action_list(&mut self, parse_table_entries: Vec<(u32, ActionListId)>) { add_line!( self, "static const TSParseActionEntry ts_parse_actions[] = {{" ); indent!(self); - for (i, entry) in parse_table_entries { + for (i, id) in parse_table_entries { + let actions = self.parse_table.action_lists.get(id); add!( self, " [{i}] = {{.entry = {{.count = {}, .reusable = {}}}}},", - entry.actions.len(), - entry.reusable + actions.len(), + id.reusable(), ); - for action in &entry.actions { + for action in actions { add!(self, " "); match *action { ParseAction::Accept => add!(self, " ACCEPT_INPUT()"), @@ -1700,16 +1703,17 @@ impl Generator { } fn get_parse_action_list_id( - entry: &ParseTableEntry, - parse_table_entries: &mut FxHashMap, + id: ActionListId, + pool: &ActionListPool, + parse_action_list_offsets: &mut FxHashMap, next_parse_action_list_index: &mut u32, ) -> u32 { - if let Some(&index) = parse_table_entries.get(entry) { + if let Some(&index) = parse_action_list_offsets.get(&id) { index } else { let result = *next_parse_action_list_index; - parse_table_entries.insert(entry.clone(), result); - *next_parse_action_list_index += 1 + entry.actions.len() as u32; + parse_action_list_offsets.insert(id, result); + *next_parse_action_list_index += 1 + pool.get(id).len() as u32; result } } diff --git a/crates/generate/src/tables.rs b/crates/generate/src/tables.rs index c2817b951..bd171cf30 100644 --- a/crates/generate/src/tables.rs +++ b/crates/generate/src/tables.rs @@ -14,7 +14,7 @@ pub type LexStateId = u32; use std::hash::BuildHasherDefault; use indexmap::IndexMap; -use rustc_hash::FxHasher; +use rustc_hash::{FxHashMap, FxHasher}; #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum ParseAction { @@ -80,6 +80,14 @@ impl ActionList { *self = Self::One(last); } } + + pub fn from_slice(actions: &[ParseAction]) -> Self { + match actions { + [] => Self::Empty, + [action] => Self::One(*action), + actions => Self::Many(actions.to_vec()), + } + } } impl std::ops::Deref for ActionList { @@ -123,16 +131,163 @@ impl<'a> IntoIterator for &'a ActionList { } } +/// Index into [`ActionListPool::ranges`], with the high bit indicating whether the +/// given [`ActionList`] is reusable. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] +pub struct ActionListId(u32); + +impl ActionListId { + /// [`ActionListPool`] holds one entry per unique action list. In practice, there + /// are only a few thousand of these, and `render` caps the table count at `u16::MAX`. + /// These ids never come close to 2^31, so the top bit is free to indicate `reusable`. + const REUSABLE_BIT: u32 = 1 << 31; + + pub const fn new(index: u32, reusable: bool) -> Self { + debug_assert!(index < Self::REUSABLE_BIT); + Self(index | if reusable { Self::REUSABLE_BIT } else { 0 }) + } + + pub const fn index(self) -> usize { + (self.0 & !Self::REUSABLE_BIT) as usize + } + + pub const fn reusable(self) -> bool { + self.0 & Self::REUSABLE_BIT != 0 + } + + pub const fn set_reusable(&mut self, reusable: bool) { + self.0 = (self.0 & !Self::REUSABLE_BIT) | if reusable { Self::REUSABLE_BIT } else { 0 }; + } +} + +#[derive(Clone, Copy, Debug, Default)] +struct ActionListRange { + offset: u32, + len: u32, +} + +/// De-duped storage for the parse table's terminal action lists. +/// +/// Every `(state, terminal)` entry has an action list, but across a grammar typically +/// 98-99% of these lists are duplicates. Each entry holds an [`ActionListId`] pointing +/// into [`Self::ranges`], which points to a representative slice into [`Self::actions`]. +/// +/// [`intern_table`] builds this pool immediately after the parse table is constructed +/// (every inline becomes an id). `minimize` then rewrites shift targets at the pool +/// level. `canonicalize` rebuilds the pool before `render` with each list stored exactly +/// once. Pool index 0 is always the empty list. +#[derive(Clone, Debug, Default)] +pub struct ActionListPool { + actions: Vec, + ranges: Vec, + remap_scratch: Vec, +} + +impl ActionListPool { + pub fn push(&mut self, list: &[ParseAction]) -> u32 { + let index = self.ranges.len() as u32; + let offset = self.actions.len() as u32; + self.actions.extend_from_slice(list); + self.ranges.push(ActionListRange { + offset, + len: list.len() as u32, + }); + index + } + + pub fn get(&self, id: ActionListId) -> &[ParseAction] { + let range = self.ranges[id.index()]; + &self.actions[range.offset as usize..(range.offset + range.len) as usize] + } + + pub const fn len(&self) -> usize { + self.ranges.len() + } + + pub fn intern(&mut self, dedup: &mut FxHashMap, list: ActionList) -> u32 { + if let Some(&index) = dedup.get(&list) { + index + } else { + let index = self.push(&list); + dedup.insert(list, index); + index + } + } + + pub fn intern_table(table: ParseTable) -> ParseTable { + let mut pool = Self::default(); + let mut ids = FxHashMap::default(); + let states = table + .states + .into_iter() + .map(|state| { + let terminal_entries = state + .terminal_entries + .into_iter() + .map(|(symbol, entry)| { + let index = pool.intern(&mut ids, entry.actions); + (symbol, ActionListId::new(index, entry.reusable)) + }) + .collect(); + + ParseState { + id: state.id, + terminal_entries, + nonterminal_entries: state.nonterminal_entries, + reserved_words: state.reserved_words, + lex_state_id: state.lex_state_id, + external_lex_state_id: state.external_lex_state_id, + core_id: state.core_id, + } + }) + .collect(); + + ParseTable { + states, + action_lists: pool, + symbols: table.symbols, + production_infos: table.production_infos, + max_aliased_production_length: table.max_aliased_production_length, + external_lex_states: table.external_lex_states, + } + } + + pub fn canonicalize(&mut self, states: &mut [ParseState]) { + let old_actions = std::mem::take(&mut self.actions); + let old_ranges = std::mem::take(&mut self.ranges); + let mut ids = FxHashMap::default(); + // render's "no action" default + self.intern(&mut ids, ActionList::Empty); + let mut old_to_new = vec![u32::MAX; old_ranges.len()]; + for state in states { + for id in state.terminal_entries.values_mut() { + let old_index = id.index(); + let mut new_index = old_to_new[old_index]; + // INVARIANT: [`ParseTableEntry::index`] can never return `u32::MAX` + if new_index == u32::MAX { + let range = old_ranges[old_index]; + let list = ActionList::from_slice( + &old_actions[range.offset as usize..(range.offset + range.len) as usize], + ); + new_index = self.intern(&mut ids, list); + old_to_new[old_index] = new_index; + } + *id = ActionListId::new(new_index, id.reusable()); + } + } + } +} + #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct ParseTableEntry { pub actions: ActionList, pub reusable: bool, } -#[derive(Clone, Debug, Default, PartialEq, Eq)] -pub struct ParseState { +#[derive(Clone, Debug, Default)] +pub struct ParseState { pub id: ParseStateId, - pub terminal_entries: IndexMap>, + pub terminal_entries: IndexMap>, pub nonterminal_entries: IndexMap>, pub reserved_words: TokenSet, pub lex_state_id: LexStateId, @@ -152,9 +307,10 @@ pub struct ProductionInfo { pub field_map: BTreeMap>, } -#[derive(Debug, Default, PartialEq, Eq)] -pub struct ParseTable { - pub states: Vec, +#[derive(Debug, Default)] +pub struct ParseTable { + pub states: Vec>, + pub action_lists: ActionListPool, pub symbols: Vec, pub production_infos: Vec, pub max_aliased_production_length: usize, @@ -190,18 +346,23 @@ impl ParseTableEntry { } } -impl ParseState { +impl ParseState { #[must_use] pub fn is_end_of_non_terminal_extra(&self) -> bool { self.terminal_entries .contains_key(&Symbol::end_of_nonterminal_extra()) } +} - pub fn referenced_states(&self) -> impl Iterator + '_ { +impl ParseState { + pub fn referenced_states<'a>( + &'a self, + pool: &'a ActionListPool, + ) -> impl Iterator + 'a { self.terminal_entries .iter() - .flat_map(|(_, entry)| { - entry.actions.iter().filter_map(|action| match action { + .flat_map(|(_, id)| { + pool.get(*id).iter().filter_map(|action| match action { ParseAction::Shift { state, .. } => Some(*state), _ => None, }) @@ -215,42 +376,93 @@ impl ParseState { })) } - pub fn update_referenced_states(&mut self, mut f: F) + pub fn update_nonterminal_references(&mut self, mut f: F) where F: FnMut(ParseStateId, &Self) -> ParseStateId, { let mut updates = Vec::new(); - for (symbol, entry) in &self.terminal_entries { - for (i, action) in entry.actions.iter().enumerate() { - if let ParseAction::Shift { state, .. } = action { - let result = f(*state, self); - if result != *state { - updates.push((*symbol, i, result)); - } - } - } - } for (symbol, action) in &self.nonterminal_entries { if let GotoAction::Goto(other_state) = action { let result = f(*other_state, self); if result != *other_state { - updates.push((*symbol, 0, result)); + updates.push((*symbol, result)); } } } - for (symbol, action_index, new_state) in updates { - if symbol.is_non_terminal() { - self.nonterminal_entries - .insert(symbol, GotoAction::Goto(new_state)); - } else { - let entry = self.terminal_entries.get_mut(&symbol).unwrap(); - if let ParseAction::Shift { is_repetition, .. } = entry.actions[action_index] { - entry.actions[action_index] = ParseAction::Shift { - state: new_state, - is_repetition, - }; + for (symbol, new_state) in updates { + self.nonterminal_entries + .insert(symbol, GotoAction::Goto(new_state)); + } + } +} + +impl ParseTable { + pub fn remap_terminal_references(&mut self, mut f: impl FnMut(ParseStateId) -> ParseStateId) { + self.action_lists + .remap_scratch + .resize(self.action_lists.len(), false); + self.action_lists.remap_scratch.fill(false); + for state in &self.states { + for id in state.terminal_entries.values() { + self.action_lists.remap_scratch[id.index()] = true; + } + } + for (index, range) in self.action_lists.ranges.iter().copied().enumerate() { + if !self.action_lists.remap_scratch[index] { + continue; + } + for action in &mut self.action_lists.actions + [range.offset as usize..(range.offset + range.len) as usize] + { + if let ParseAction::Shift { state, .. } = action { + *state = f(*state); } } } } } + +#[cfg(test)] +mod tests { + use super::*; + + // `remap_terminal_references` must remap only the pool slots some state references. + // Dead slots (e.g. left by remove_unit_reductions' COW) can hold Shift targets + // outside the remap's domain, so touching them would panic or corrupt. Here, slot 0 + // is live, slot 1 is dead, and the closure is defined only for state 0. A regressed + // skip would call `f` on slot 1 and index out of bounds here. + #[test] + fn terminal_remap_skips_unreferenced_pool_entries() { + let mut table = ParseTable::default(); + table + .action_lists + .push(&ActionList::One(ParseAction::Shift { + state: 0, + is_repetition: false, + })); + table + .action_lists + .push(&ActionList::One(ParseAction::Shift { + state: 1, + is_repetition: false, + })); + let mut state = ParseState::default(); + state + .terminal_entries + .insert(Symbol::end(), ActionListId::new(0, true)); + table.states.push(state); + + // Slot 1 is unreferenced, so `f` must never see state 1 (else this indexes out of bounds). + let replacement = [7]; + table.remap_terminal_references(|state| replacement[state as usize]); + + assert!(matches!( + table.action_lists.get(ActionListId::new(0, false))[0], + ParseAction::Shift { state: 7, .. } + )); + assert!(matches!( + table.action_lists.get(ActionListId::new(1, false))[0], + ParseAction::Shift { state: 1, .. } + )); + } +}