From 07afd63365fd3880aa63d5a198122ec2fd435205 Mon Sep 17 00:00:00 2001 From: Taehwan Kim Date: Sun, 5 Jul 2026 09:56:39 +0900 Subject: [PATCH 1/3] removed can_accept_error since can_feed does the same job --- rusty_lr_core/src/builder/grammar.rs | 15 ----- rusty_lr_core/src/builder/state.rs | 5 -- rusty_lr_core/src/lib.rs | 19 ------ .../src/parser/deterministic/context.rs | 63 +++++-------------- .../src/parser/nondeterministic/context.rs | 29 +++------ rusty_lr_core/src/parser/state.rs | 2 - rusty_lr_core/src/parser/table.rs | 26 +------- rusty_lr_parser/src/emit.rs | 23 ------- rusty_lr_parser/src/parser/parser_expanded.rs | 18 ------ scripts/diff/calculator.rs | 10 --- scripts/diff/calculator_u8.rs | 11 ---- scripts/diff/json.rs | 15 ----- 12 files changed, 27 insertions(+), 209 deletions(-) diff --git a/rusty_lr_core/src/builder/grammar.rs b/rusty_lr_core/src/builder/grammar.rs index d258028d..08506733 100644 --- a/rusty_lr_core/src/builder/grammar.rs +++ b/rusty_lr_core/src/builder/grammar.rs @@ -10,7 +10,6 @@ use super::DiagnosticCollector; use super::State; use super::States; use crate::TerminalSymbol; -use crate::TriState; use crate::hash::HashMap; use crate::production::*; use crate::symbol::Symbol; @@ -529,16 +528,6 @@ impl Grammar, NonTerm> { panic!("main state is not 0"); } - for state in &mut states { - if state - .shift_goto_map_term - .contains_key(&TerminalSymbol::Error) - || state.reduce_map.contains_key(&TerminalSymbol::Error) - { - state.can_accept_error = TriState::True; - } - } - Ok(States { states }) } @@ -676,8 +665,6 @@ impl Grammar, NonTerm> { .shift_goto_map_nonterm .append(&mut state_b.shift_goto_map_nonterm); state_a.reduce_map.append(&mut state_b.reduce_map); - state_a.can_accept_error = - state_a.can_accept_error | state_b.can_accept_error; merged = true; } } @@ -773,8 +760,6 @@ impl Grammar, NonTerm> { .or_default() .append(&mut reduce_rules); } - states[state_a].can_accept_error = - states[state_a].can_accept_error | state_b.can_accept_error; } } let mut new_states = Vec::with_capacity(states.len() - merge_into.len()); diff --git a/rusty_lr_core/src/builder/state.rs b/rusty_lr_core/src/builder/state.rs index 153ea190..727f96f3 100644 --- a/rusty_lr_core/src/builder/state.rs +++ b/rusty_lr_core/src/builder/state.rs @@ -1,8 +1,6 @@ use std::collections::BTreeMap; use std::collections::BTreeSet; -use crate::TriState; - /// state for internal usage during grammar building stage #[derive(Debug, Clone)] pub struct State { @@ -10,7 +8,6 @@ pub struct State { pub shift_goto_map_nonterm: BTreeMap, pub reduce_map: BTreeMap>, pub ruleset: BTreeSet, - pub can_accept_error: TriState, } impl State { pub fn new() -> Self { @@ -19,7 +16,6 @@ impl State { shift_goto_map_nonterm: Default::default(), reduce_map: Default::default(), ruleset: Default::default(), - can_accept_error: TriState::False, } } @@ -80,7 +76,6 @@ where .map(|(term, rules)| (term, rules.into_iter().collect())) .collect(), ruleset: state.ruleset.into_iter().collect(), - can_accept_error: state.can_accept_error, } } } diff --git a/rusty_lr_core/src/lib.rs b/rusty_lr_core/src/lib.rs index 870ae046..d46ffa57 100644 --- a/rusty_lr_core/src/lib.rs +++ b/rusty_lr_core/src/lib.rs @@ -51,22 +51,3 @@ impl std::error::Error for DefaultReduceActionError { "Default reduce action error" } } - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum TriState { - False, - Maybe, - True, -} -impl std::ops::BitOr for TriState { - type Output = Self; - fn bitor(self, rhs: Self) -> Self::Output { - match (self, rhs) { - (TriState::False, TriState::False) => TriState::False, - (TriState::False, _) => TriState::Maybe, - (_, TriState::False) => TriState::Maybe, - (TriState::True, TriState::True) => TriState::True, - _ => TriState::Maybe, - } - } -} diff --git a/rusty_lr_core/src/parser/deterministic/context.rs b/rusty_lr_core/src/parser/deterministic/context.rs index 8c78625c..fee2dc53 100644 --- a/rusty_lr_core/src/parser/deterministic/context.rs +++ b/rusty_lr_core/src/parser/deterministic/context.rs @@ -369,23 +369,17 @@ impl< return Err(ParseError::NoAction(err)); } - use crate::TriState; let mut pop_count = 0; let mut error_plan = None; - for &s in self.state_stack.iter().rev() { - match self.tables.can_accept_error(s.into_usize()) { - TriState::False => {} - TriState::Maybe | TriState::True => { - // Reuse the same CFG simulation used by normal feeds when deciding - // whether this stack prefix can actually shift the recovery token. - if let Ok(plan) = self.plan_feed_location_from_stack_len( - self.state_stack.len() - pop_count, - P::TermClass::ERROR, - ) { - error_plan = Some(plan); - break; - } - } + for _ in self.state_stack.iter().rev() { + // Reuse the same CFG simulation used by normal feeds when deciding whether + // this stack prefix can actually shift the recovery token. + if let Ok(plan) = self.plan_feed_location_from_stack_len( + self.state_stack.len() - pop_count, + P::TermClass::ERROR, + ) { + error_plan = Some(plan); + break; } pop_count += 1; @@ -428,7 +422,7 @@ impl< self.data_stack.push(Data::new_empty()); } - self.location_stack.push(err.location.clone()); + self.location_stack.push(err.location); self.state_stack.push(next_state.state); } else { // merge term with previous error @@ -627,12 +621,8 @@ impl< #[cfg(feature = "tree")] { // Mirror the same reduction in the optional syntax tree stack. - let mut children = Vec::with_capacity(reduction.tokens_len); - for _ in 0..reduction.tokens_len { - let tree = self.tree_stack.pop().unwrap(); - children.push(tree); - } - children.reverse(); + let l = self.tree_stack.len() - reduction.tokens_len; + let children = self.tree_stack.split_off(l); self.tree_stack .push(crate::tree::Tree::new_nonterminal(reduction.lhs, children)); @@ -656,12 +646,7 @@ impl< } } } else { - match term { - TerminalSymbol::Terminal(_) - | TerminalSymbol::Error - | TerminalSymbol::Eof - | TerminalSymbol::VirtualStart(_) => self.data_stack.push(Data::new_empty()), - } + self.data_stack.push(Data::new_empty()); } self.location_stack.push(location); @@ -690,12 +675,9 @@ impl< let mut stack_len = self.state_stack.len(); loop { - match self - .plan_feed_location_from_stack_len(stack_len, error) - .is_ok() - { - true => break true, // successfully shifted `error` - false => { + match self.plan_feed_location_from_stack_len(stack_len, error) { + Ok(_) => break true, // successfully shifted `error` + Err(_) => { if stack_len == 0 { break false; } else { @@ -785,7 +767,6 @@ where mod tests { use super::*; use crate::DefaultLocation; - use crate::TriState; use crate::parser::table::ShiftTarget; use crate::parser::table::TermActionRef; @@ -903,10 +884,6 @@ mod tests { std::iter::empty::() } - fn can_accept_error(&self, _state: usize) -> TriState { - TriState::False - } - fn rule(&self, rule: usize) -> &crate::parser::table::RuleInfo { static RULES: [crate::parser::table::RuleInfo; 1] = [crate::parser::table::RuleInfo { @@ -960,7 +937,6 @@ mod tests { class: Self::TermClass, ) -> Option> { // Recovery from state 2 must reduce before the synthetic `error` token can shift. - // Marking the state as `TriState::True` below ensures recovery still builds a plan. match (state, class) { (0, TestTermClass::Start) => Some(TermActionRef::Shift(ShiftTarget::new(1, false))), (1, TestTermClass::A) => Some(TermActionRef::Shift(ShiftTarget::new(2, true))), @@ -1001,13 +977,6 @@ mod tests { std::iter::empty::() } - fn can_accept_error(&self, state: usize) -> TriState { - match state { - 2 => TriState::True, - _ => TriState::False, - } - } - fn rule(&self, rule: usize) -> &crate::parser::table::RuleInfo { static RULES: [crate::parser::table::RuleInfo; 1] = [crate::parser::table::RuleInfo { diff --git a/rusty_lr_core/src/parser/nondeterministic/context.rs b/rusty_lr_core/src/parser/nondeterministic/context.rs index cb0e35a7..96e95e64 100644 --- a/rusty_lr_core/src/parser/nondeterministic/context.rs +++ b/rusty_lr_core/src/parser/nondeterministic/context.rs @@ -1098,8 +1098,6 @@ impl< P::NonTerm: std::fmt::Debug, { use crate::Location; - use crate::TriState; - let mut error_location_preserved = None; let pop_count = loop { @@ -1112,25 +1110,16 @@ impl< let mut pop_count = 0; let mut found = false; - for &s in node_.state_stack.iter().rev() { - match self.tables.can_accept_error(s.into_usize()) { - TriState::False => {} - TriState::Maybe => { - extra_state_stack.clear(); + for _ in node_.state_stack.iter().rev() { + extra_state_stack.clear(); - if self.can_feed_impl( - extra_state_stack, - Some((node, NonZeroUsize::new(node_.len() - pop_count).unwrap())), - P::TermClass::ERROR, - ) { - found = true; - break; - } - } - TriState::True => { - found = true; - break; - } + if self.can_feed_impl( + extra_state_stack, + Some((node, NonZeroUsize::new(node_.len() - pop_count).unwrap())), + P::TermClass::ERROR, + ) { + found = true; + break; } pop_count += 1; diff --git a/rusty_lr_core/src/parser/state.rs b/rusty_lr_core/src/parser/state.rs index c655d050..38d85933 100644 --- a/rusty_lr_core/src/parser/state.rs +++ b/rusty_lr_core/src/parser/state.rs @@ -1,4 +1,3 @@ -use crate::TriState; use crate::parser::table::ShiftTarget; /// One decoded parser-table row used by generated code and the grammar builder before converting @@ -8,5 +7,4 @@ pub struct IntermediateState { pub shift_goto_map_nonterm: Vec<(NonTerm, ShiftTarget)>, // must be sorted pub reduce_map: Vec<(TermClass, Vec)>, // must be sorted pub ruleset: Vec, - pub can_accept_error: TriState, } diff --git a/rusty_lr_core/src/parser/table.rs b/rusty_lr_core/src/parser/table.rs index 684d4413..a392d690 100644 --- a/rusty_lr_core/src/parser/table.rs +++ b/rusty_lr_core/src/parser/table.rs @@ -2,7 +2,6 @@ //! //! This module is the public home for parser table layouts used by generated parsers. -use crate::TriState; use crate::parser::nonterminal::NonTerminal; use crate::parser::state::IntermediateState; use crate::parser::terminalclass::TerminalClass; @@ -233,8 +232,6 @@ pub trait ParserTables { fn expected_reduce_rule(&self, state: usize) -> impl Iterator + '_; - fn can_accept_error(&self, state: usize) -> TriState; - /// Returns compact rule metadata used while reducing. fn rule(&self, rule: usize) -> &RuleInfo; @@ -285,8 +282,6 @@ pub struct SparseFlatTables { nonterm_offsets: Vec, /// Sorted `(nonterminal, goto)` pairs for all states concatenated together. nonterm_goto: Vec<(NonTerm, ShiftTarget)>, - /// Error-recovery capability for each state. - can_accept_error: Vec, /// Compact rule metadata indexed by reduce rule id. rules: Vec>, } @@ -305,7 +300,6 @@ where let mut term_actions = Vec::new(); let mut nonterm_offsets = Vec::with_capacity(intermediate.state_rows.len() + 1); let mut nonterm_goto = Vec::new(); - let mut can_accept_error = Vec::with_capacity(intermediate.state_rows.len()); term_offsets.push(0); nonterm_offsets.push(0); @@ -367,7 +361,6 @@ where .map(|(nonterm, target)| (nonterm, target.compact())), ); nonterm_offsets.push(nonterm_goto.len()); - can_accept_error.push(state.can_accept_error); } SparseFlatTables { @@ -375,7 +368,6 @@ where term_actions, nonterm_offsets, nonterm_goto, - can_accept_error, rules: intermediate.rules, } } @@ -458,16 +450,12 @@ where .flat_map(RuleContainer::to_iter) } - fn can_accept_error(&self, state: usize) -> TriState { - self.can_accept_error[state] - } - fn rule(&self, rule: usize) -> &RuleInfo { &self.rules[rule] } fn state_count(&self) -> usize { - self.can_accept_error.len() + self.term_offsets.len() - 1 } fn rule_count(&self) -> usize { @@ -500,8 +488,6 @@ pub struct DenseFlatTables { nonterm_keys_offsets: Vec, /// Nonterminal keys present in each goto row. nonterm_keys: Vec, - /// Error-recovery capability for each state. - can_accept_error: Vec, /// Compact rule metadata indexed by reduce rule id. rules: Vec>, } @@ -527,7 +513,6 @@ where let mut nonterm_goto = Vec::new(); let mut nonterm_keys_offsets = Vec::with_capacity(intermediate.state_rows.len() + 1); let mut nonterm_keys = Vec::new(); - let mut can_accept_error = Vec::with_capacity(intermediate.state_rows.len()); term_offsets.push(0); term_keys_offsets.push(0); @@ -627,8 +612,6 @@ where } nonterm_offsets.push(nonterm_goto.len()); nonterm_keys_offsets.push(nonterm_keys.len()); - - can_accept_error.push(state.can_accept_error); } DenseFlatTables { @@ -642,7 +625,6 @@ where nonterm_goto, nonterm_keys_offsets, nonterm_keys, - can_accept_error, rules: intermediate.rules, } } @@ -716,16 +698,12 @@ where .flat_map(RuleContainer::to_iter) } - fn can_accept_error(&self, state: usize) -> TriState { - self.can_accept_error[state] - } - fn rule(&self, rule: usize) -> &RuleInfo { &self.rules[rule] } fn state_count(&self) -> usize { - self.can_accept_error.len() + self.term_offsets.len() - 1 } fn rule_count(&self) -> usize { diff --git a/rusty_lr_parser/src/emit.rs b/rusty_lr_parser/src/emit.rs index b37b39cd..399bd906 100644 --- a/rusty_lr_parser/src/emit.rs +++ b/rusty_lr_parser/src/emit.rs @@ -522,7 +522,6 @@ impl Grammar { let mut shift_nonterm_offsets = Vec::new(); let mut reduce_data = Vec::new(); let mut reduce_offsets = Vec::new(); - let mut can_accept_error = Vec::new(); shift_term_offsets.push(0); shift_nonterm_offsets.push(0); @@ -601,14 +600,6 @@ impl Grammar { } } reduce_offsets.push(reduce_data.len() as u32); - - // 5. can_accept_error: TriState - let tri_val = match state.can_accept_error { - rusty_lr_core::TriState::False => 0u8, - rusty_lr_core::TriState::True => 1u8, - rusty_lr_core::TriState::Maybe => 2u8, - }; - can_accept_error.push(tri_val); } let num_rules = self.builder.rules.len(); @@ -642,10 +633,6 @@ impl Grammar { let reduce_offsets = reduce_offsets .into_iter() .map(proc_macro2::Literal::u32_unsuffixed); - let can_accept_error = can_accept_error - .into_iter() - .map(proc_macro2::Literal::u8_unsuffixed); - // range-compressed Vec based terminal-class_id map stream.extend(quote! { /// A lightweight parser struct that references the static parser tables and production rules. @@ -687,14 +674,12 @@ impl Grammar { // - SHIFT_TERM_OFFSETS & SHIFT_NONTERM_OFFSETS: Boundaries separating transitions for each state // - REDUCE_DATA: Variable-length reduce map encoding (term_class, len, rules...) // - REDUCE_OFFSETS: Boundaries separating reduce maps for each state - // - CAN_ACCEPT_ERROR: TriState (0 = False, 1 = True, 2 = Maybe) static SHIFT_TERM_DATA: &[u32] = &[ #(#shift_term_data),* ]; static SHIFT_TERM_OFFSETS: &[u32] = &[ #(#shift_term_offsets),* ]; static SHIFT_NONTERM_DATA: &[u32] = &[ #(#shift_nonterm_data),* ]; static SHIFT_NONTERM_OFFSETS: &[u32] = &[ #(#shift_nonterm_offsets),* ]; static REDUCE_DATA: &[u32] = &[ #(#reduce_data),* ]; static REDUCE_OFFSETS: &[u32] = &[ #(#reduce_offsets),* ]; - static CAN_ACCEPT_ERROR: &[u8] = &[ #(#can_accept_error),* ]; let num_rules = #num_rules; let mut rules = Vec::with_capacity(num_rules); @@ -751,19 +736,11 @@ impl Grammar { idx += 2 + len; } - let can_accept_error = match CAN_ACCEPT_ERROR[i] { - 0 => #module_prefix::TriState::False, - 1 => #module_prefix::TriState::True, - 2 => #module_prefix::TriState::Maybe, - _ => unreachable!(), - }; - let intermediate = #module_prefix::parser::state::IntermediateState { shift_goto_map_term, shift_goto_map_nonterm, reduce_map, ruleset: Vec::new(), - can_accept_error, }; state_rows.push(intermediate); } diff --git a/rusty_lr_parser/src/parser/parser_expanded.rs b/rusty_lr_parser/src/parser/parser_expanded.rs index 0f7c6fb3..da6d404c 100644 --- a/rusty_lr_parser/src/parser/parser_expanded.rs +++ b/rusty_lr_parser/src/parser/parser_expanded.rs @@ -7643,17 +7643,6 @@ impl ::rusty_lr_core::parser::Parser for Parser { 2709, 2709, 2718, 2721, 2721, 2721, 2730, 2739, 2739, 2748, 2748, 2757, 2766, 2769, 2772, 2772, 2772, ]; - static CAN_ACCEPT_ERROR: &[u8] = &[ - 0, 0, 0, 0, 0, 1, 1, 1, 2, 2, 0, 0, 0, 2, 2, 2, 2, 1, 1, 1, 1, 0, 1, - 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 0, 2, 2, 0, 0, 0, 2, 0, - 2, 2, 2, 2, 0, 2, 0, 0, 0, 0, 0, 0, 2, 1, 1, 0, 2, 0, 2, 0, 2, 0, 2, - 0, 2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2, 2, 2, 1, 1, 1, 1, 1, 1, - 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, - 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, - 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - ]; let num_rules = 163usize; let mut rules = Vec::with_capacity(num_rules); for i in 0..num_rules { @@ -7722,18 +7711,11 @@ impl ::rusty_lr_core::parser::Parser for Parser { reduce_map.push((term_class, rules)); idx += 2 + len; } - let can_accept_error = match CAN_ACCEPT_ERROR[i] { - 0 => ::rusty_lr_core::TriState::False, - 1 => ::rusty_lr_core::TriState::True, - 2 => ::rusty_lr_core::TriState::Maybe, - _ => unreachable!(), - }; let intermediate = ::rusty_lr_core::parser::state::IntermediateState { shift_goto_map_term, shift_goto_map_nonterm, reduce_map, ruleset: Vec::new(), - can_accept_error, }; state_rows.push(intermediate); } diff --git a/scripts/diff/calculator.rs b/scripts/diff/calculator.rs index d8eb67e8..dc3dab60 100644 --- a/scripts/diff/calculator.rs +++ b/scripts/diff/calculator.rs @@ -648,9 +648,6 @@ impl ::rusty_lr::parser::Parser for Parser { static REDUCE_OFFSETS: &[u32] = &[ 0, 0, 0, 12, 12, 18, 18, 27, 36, 36, 48, 48, 60, 60, 60, ]; - static CAN_ACCEPT_ERROR: &[u8] = &[ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - ]; let num_rules = 8usize; let mut rules = Vec::with_capacity(num_rules); for i in 0..num_rules { @@ -713,18 +710,11 @@ impl ::rusty_lr::parser::Parser for Parser { reduce_map.push((term_class, rules)); idx += 2 + len; } - let can_accept_error = match CAN_ACCEPT_ERROR[i] { - 0 => ::rusty_lr::TriState::False, - 1 => ::rusty_lr::TriState::True, - 2 => ::rusty_lr::TriState::Maybe, - _ => unreachable!(), - }; let intermediate = ::rusty_lr::parser::state::IntermediateState { shift_goto_map_term, shift_goto_map_nonterm, reduce_map, ruleset: Vec::new(), - can_accept_error, }; state_rows.push(intermediate); } diff --git a/scripts/diff/calculator_u8.rs b/scripts/diff/calculator_u8.rs index 369fa116..c16ce74e 100644 --- a/scripts/diff/calculator_u8.rs +++ b/scripts/diff/calculator_u8.rs @@ -939,10 +939,6 @@ impl ::rusty_lr::parser::Parser for Parser { 0, 0, 12, 24, 24, 36, 48, 72, 99, 99, 111, 111, 123, 135, 147, 147, 159, 165, 177, 189, 210, 231, 243, 264, 276, 288, 300, 306, 306, ]; - static CAN_ACCEPT_ERROR: &[u8] = &[ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, - ]; let num_rules = 16usize; let mut rules = Vec::with_capacity(num_rules); for i in 0..num_rules { @@ -1005,18 +1001,11 @@ impl ::rusty_lr::parser::Parser for Parser { reduce_map.push((term_class, rules)); idx += 2 + len; } - let can_accept_error = match CAN_ACCEPT_ERROR[i] { - 0 => ::rusty_lr::TriState::False, - 1 => ::rusty_lr::TriState::True, - 2 => ::rusty_lr::TriState::Maybe, - _ => unreachable!(), - }; let intermediate = ::rusty_lr::parser::state::IntermediateState { shift_goto_map_term, shift_goto_map_nonterm, reduce_map, ruleset: Vec::new(), - can_accept_error, }; state_rows.push(intermediate); } diff --git a/scripts/diff/json.rs b/scripts/diff/json.rs index 7b6eaa0c..54cf25e7 100644 --- a/scripts/diff/json.rs +++ b/scripts/diff/json.rs @@ -2096,14 +2096,6 @@ impl ::rusty_lr::parser::Parser for Parser { 1188, 1191, 1194, 1197, 1200, 1215, 1215, 1230, 1239, 1245, 1245, 1254, 1260, 1260, ]; - static CAN_ACCEPT_ERROR: &[u8] = &[ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, - ]; let num_rules = 99usize; let mut rules = Vec::with_capacity(num_rules); for i in 0..num_rules { @@ -2166,18 +2158,11 @@ impl ::rusty_lr::parser::Parser for Parser { reduce_map.push((term_class, rules)); idx += 2 + len; } - let can_accept_error = match CAN_ACCEPT_ERROR[i] { - 0 => ::rusty_lr::TriState::False, - 1 => ::rusty_lr::TriState::True, - 2 => ::rusty_lr::TriState::Maybe, - _ => unreachable!(), - }; let intermediate = ::rusty_lr::parser::state::IntermediateState { shift_goto_map_term, shift_goto_map_nonterm, reduce_map, ruleset: Vec::new(), - can_accept_error, }; state_rows.push(intermediate); } From 958f80b258f4738b0c364d65e0e0e3a187083ecf Mon Sep 17 00:00:00 2001 From: Taehwan Kim Date: Sun, 5 Jul 2026 11:04:09 +0900 Subject: [PATCH 2/3] use plan-play scheme for glr parser --- rusty_lr/tests/lr_glr_operation.rs | 109 +++ .../src/parser/deterministic/context.rs | 49 +- .../src/parser/nondeterministic/context.rs | 665 ++++++++++++++---- rusty_lr_parser/src/grammar.rs | 6 +- 4 files changed, 661 insertions(+), 168 deletions(-) diff --git a/rusty_lr/tests/lr_glr_operation.rs b/rusty_lr/tests/lr_glr_operation.rs index 2c4d6e3f..808973a7 100644 --- a/rusty_lr/tests/lr_glr_operation.rs +++ b/rusty_lr/tests/lr_glr_operation.rs @@ -193,6 +193,69 @@ mod token_userdata_and_recovery { } } +mod recovery_reduce_before_sync_token { + mod deterministic { + use rusty_lr::lr1; + + lr1! { + %nooptim; + %tokentype char; + %location std::ops::Range; + %userdata Vec>; + %start S; + + S(&'static str): error Tail { + data.push(@error.clone()); + "recovered" + }; + + Tail(()): Empty 's' { () }; + Empty(()): "" { () }; + } + + #[test] + fn deterministic_recovery_feeds_sync_token_through_reduce_chain() { + let mut ctx = SContext::new(Vec::new()); + ctx.feed_location('s', 0..1).unwrap(); + + let (value, recovered_spans) = ctx.accept().unwrap(); + assert_eq!(value, "recovered"); + assert_eq!(recovered_spans, [0..0]); + } + } + + mod glr { + use rusty_lr::lr1; + + lr1! { + %glr; + %nooptim; + %tokentype char; + %location std::ops::Range; + %userdata Vec>; + %start S; + + S(&'static str): error Tail { + data.push(@error.clone()); + "recovered" + }; + + Tail(()): Empty 's' { () }; + Empty(()): "" { () }; + } + + #[test] + fn glr_recovery_feeds_sync_token_through_reduce_chain() { + let mut ctx = SContext::new(Vec::new()); + ctx.feed_location('s', 0..1).unwrap(); + + let (value, recovered_spans) = ctx.accept().unwrap(); + assert_eq!(value, "recovered"); + assert_eq!(recovered_spans, [0..0]); + } + } +} + mod left_recursive_nullable_list { use rusty_lr::lr1; @@ -465,4 +528,50 @@ mod reduce_action_errors { assert_eq!(ctx.accept().unwrap(), ("shift path", ())); } } + + mod glr_recovery_partial_errors { + use rusty_lr::lr1; + + // Recovery can have GLR alternatives too. A semantic failure in one recovery branch should + // be reported without killing sibling recovery branches that successfully shifted `error`. + lr1! { + %glr; + %nooptim; + %tokentype char; + %error &'static str; + %start S; + + S(&'static str) + : Good error 's' { + "recovered" + } + | Bad error 's' { + "bad" + } + ; + + Good(()): "" { () }; + + Bad(()): "" { + if true { + return Err("bad recovery branch"); + } + () + }; + } + + #[test] + fn glr_recovery_success_reports_pruned_recovery_branch_errors() { + let mut ctx = SContext::with_default_userdata(); + + let success = ctx.feed('x').unwrap(); + let errors = success + .errors + .expect("failed recovery branch should be reported"); + assert_eq!(errors.reduce_action_errors, ["bad recovery branch"]); + + ctx.feed('s').unwrap(); + assert_eq!(ctx.accept().unwrap(), ("recovered", ())); + } + } } diff --git a/rusty_lr_core/src/parser/deterministic/context.rs b/rusty_lr_core/src/parser/deterministic/context.rs index fee2dc53..293d0a3e 100644 --- a/rusty_lr_core/src/parser/deterministic/context.rs +++ b/rusty_lr_core/src/parser/deterministic/context.rs @@ -403,44 +403,25 @@ impl< self.tree_stack.truncate(l); } - match self.apply_feed_plan(error_plan, TerminalSymbol::Error, error_location) { - Ok(()) => { - // try shift given term again - // to check if the given terminal should be merged with `error` token - // or it can be shift right after the error token - if let Some(next_state) = self.tables.shift_goto_class(self.state(), class) - { - #[cfg(feature = "tree")] - self.tree_stack - .push(crate::tree::Tree::new_terminal(err.term.clone())); - - // shift after `error` token - if next_state.push { - self.data_stack - .push(Data::new_terminal(err.term.into_term().unwrap())); - } else { - self.data_stack.push(Data::new_empty()); - } - - self.location_stack.push(err.location); - self.state_stack.push(next_state.state); + self.apply_feed_plan(error_plan, TerminalSymbol::Error, error_location)?; + // `error` was feed, now check with the original terminal symbol again. + // If the original terminal is still not accepted, it is part of the `error`. + match self.plan_feed_location(class) { + Ok(term_plan) => self.apply_feed_plan(term_plan, err.term, err.location), + Err(_) => { + // The terminal symbol is still not feedable, so it belongs to the `error` token. + // merge term with previous error + let error_location = Data::Location::new( + std::iter::once(&err.location).chain(self.location_stack.iter().rev()), + 2, // error node + ); + if let Some(err_loc) = self.location_stack.last_mut() { + *err_loc = error_location; } else { - // merge term with previous error - - let error_location = Data::Location::new( - std::iter::once(&err.location) - .chain(self.location_stack.iter().rev()), - 2, // error node - ); - if let Some(err_loc) = self.location_stack.last_mut() { - *err_loc = error_location; - } else { - unreachable!("location stack must have at least one element"); - } + unreachable!("location stack must have at least one element"); } Ok(()) } - Err(_) => Err(ParseError::NoAction(err)), // other errors } } Err(err) => Err(err), diff --git a/rusty_lr_core/src/parser/nondeterministic/context.rs b/rusty_lr_core/src/parser/nondeterministic/context.rs index 96e95e64..a2e09e18 100644 --- a/rusty_lr_core/src/parser/nondeterministic/context.rs +++ b/rusty_lr_core/src/parser/nondeterministic/context.rs @@ -1,3 +1,4 @@ +use std::cell::RefCell; use std::num::NonZeroUsize; use super::Node; @@ -101,6 +102,23 @@ struct UnshiftedBranch { userdata: UserData, } +#[derive(Clone, Copy)] +struct GlrFeedPlan { + shift: Option>, + first_reduction: Option, +} + +#[derive(Clone, Copy)] +struct PlannedGlrReduction { + rule_index: usize, + tokens_len: usize, + #[allow(dead_code)] + lhs: NonTerm, + next_nonterm_shift: crate::parser::table::ShiftTarget, + next_plan: usize, + next_sibling: Option, +} + impl Branch { fn new(node: NodeId, userdata: UserData) -> Self { Branch { node, userdata } @@ -132,8 +150,26 @@ pub struct Context< /// But we don't want to reallocate every `feed` call pub(crate) reduce_errors: Vec, - // Future optimization point: GLR can also reuse simulation state stacks, but branch-local - // recursion and cloned alternatives need a separate scratch ownership strategy. + /// Feed plans generated by the CFG-only GLR pre-feed simulation. + /// + /// Plans are stored in DFS pre-order, so replay usually visits child plans close to the + /// parent plan in memory. + feed_plans: Vec>, + + /// Flat sibling-linked reduction records used by `feed_plans`. + /// + /// Keeping reductions in one reusable buffer avoids allocating a nested `Vec` per plan node. + feed_plan_reductions: Vec>, + + /// Reusable simulated state stacks for GLR feed planning alternatives. + feed_plan_extra_state_stacks: Vec>, + + /// Reusable simulation stack for `can_feed`. + /// + /// `can_feed` only has `&self`, so this scratch buffer uses interior mutability to avoid + /// allocating a fresh top-level `Vec` for each query. + can_feed_extra_state_stack: RefCell>, + /// Decoded parser tables shared by every active GLR branch. /// /// Branches clone stack/userdata state, but they all read the same immutable runtime tables. @@ -160,6 +196,10 @@ impl< next_branches: Default::default(), reduce_errors: Default::default(), fallback_branches: Default::default(), + feed_plans: Default::default(), + feed_plan_reductions: Default::default(), + feed_plan_extra_state_stacks: Default::default(), + can_feed_extra_state_stack: Default::default(), tables: P::get_tables(), _phantom: std::marker::PhantomData, }; @@ -659,6 +699,56 @@ impl< } } + fn reduce_planned( + &mut self, + reduction: PlannedGlrReduction, + node: NodeId, + term: &crate::TerminalSymbol, + shift: &mut bool, + can_reuse_node_for_reduce: bool, + userdata: &mut Data::UserData, + ) -> Result + where + Data: Clone, + P::Term: Clone, + { + use crate::Location; + let count = reduction.tokens_len; + let mut new_location = Data::Location::new(self.location_iter(node), count); + + let node_to_shift = self.prepare_reduce_node(node, count, count, can_reuse_node_for_reduce); + + let node = self.node_mut(node_to_shift); + #[cfg(feature = "tree")] + let trees = node.tree_stack.split_off(node.tree_stack.len() - count); + + match Data::reduce_action( + &mut node.data_stack, + &mut node.location_stack, + reduction.next_nonterm_shift.push, + reduction.rule_index, + shift, + term, + userdata, + &mut new_location, + ) { + Ok(_) => { + node.state_stack.push(reduction.next_nonterm_shift.state); + node.location_stack.push(new_location); + #[cfg(feature = "tree")] + { + node.tree_stack + .push(crate::tree::Tree::new_nonterminal(reduction.lhs, trees)); + } + Ok(node_to_shift) + } + Err(err) => { + self.try_remove_node_recursive(node_to_shift); + Err(err) + } + } + } + /// Get number of diverged paths pub fn len_paths(&self) -> usize { self.current_branches.len() @@ -910,6 +1000,321 @@ impl< } } + fn branch_node_and_len(&self, node: NodeId) -> Option<(NodeId, NonZeroUsize)> { + let node_ = self.node(node); + if node_.len() == 0 { + None + } else { + Some((node, NonZeroUsize::new(node_.len()).unwrap())) + } + } + + fn simulated_state( + &self, + extra_state_stack: &[StateIndex], + node_and_len: Option<(NodeId, NonZeroUsize)>, + ) -> usize { + extra_state_stack + .last() + .copied() + .map(Index::into_usize) + .unwrap_or_else(|| { + node_and_len + .map(|(node, stack_len)| { + self.node(node).state_stack[stack_len.get() - 1].into_usize() + }) + .unwrap_or(0) + }) + } + + fn take_feed_plan_extra_state_stack(&mut self) -> Vec { + let mut stack = self.feed_plan_extra_state_stacks.pop().unwrap_or_default(); + stack.clear(); + stack + } + + fn put_feed_plan_extra_state_stack(&mut self, mut stack: Vec) { + stack.clear(); + self.feed_plan_extra_state_stacks.push(stack); + } + + fn simulate_planned_reduction( + &self, + extra_state_stack: &mut Vec, + mut node_and_len: Option<(NodeId, NonZeroUsize)>, + rule_index: usize, + ) -> ( + Option<(NodeId, NonZeroUsize)>, + P::NonTerm, + usize, + crate::parser::table::ShiftTarget, + ) { + let rule_info = *self.tables.rule(rule_index); + let tokens_len = rule_info.len; + + if tokens_len <= extra_state_stack.len() { + extra_state_stack.truncate(extra_state_stack.len() - tokens_len); + } else { + let left = tokens_len - extra_state_stack.len(); + extra_state_stack.clear(); + + let (node, len) = node_and_len.unwrap(); + let len = len.get(); + + if left < len { + node_and_len = Some((node, NonZeroUsize::new(len - left).unwrap())); + } else { + let parent = self.node(node).parent; + if let Some(parent) = parent { + node_and_len = self + .skip_last_n(parent, left - len) + .map(|(node, i)| (node, NonZeroUsize::new(i + 1).unwrap())); + } else { + node_and_len = None; + } + } + } + + let last_state = self.simulated_state(extra_state_stack, node_and_len); + let Some(next_nonterm_shift) = self.tables.shift_goto_nonterm(last_state, rule_info.lhs) + else { + unreachable!( + "unreachable: nonterminal shift should always succeed after reduce operation, but failed for nonterminal '{}' from state {:?}", + rule_info.lhs.as_str(), + last_state + ); + }; + extra_state_stack.push(next_nonterm_shift.state); + + (node_and_len, rule_info.lhs, tokens_len, next_nonterm_shift) + } + + fn plan_feed_impl( + &mut self, + extra_state_stack: &mut Vec, + node_and_len: Option<(NodeId, NonZeroUsize)>, + class: P::TermClass, + ) -> Option { + use crate::parser::table::ReduceRules; + + let last_state = self.simulated_state(extra_state_stack, node_and_len); + let (shift, reduce) = match self.tables.term_action(last_state, class) { + Some(action) => (action.shift(), action.reduce()), + None => (None, None), + }; + + let plan_id = self.feed_plans.len(); + self.feed_plans.push(GlrFeedPlan { + shift, + first_reduction: None, + }); + + let mut first_reduction: Option = None; + let mut previous_reduction: Option = None; + + if let Some(reduce_rules) = reduce { + for reduce_rule in reduce_rules.to_iter() { + let rule_index = reduce_rule.into_usize(); + let reduction_id = self.feed_plan_reductions.len(); + self.feed_plan_reductions.push(PlannedGlrReduction { + rule_index, + tokens_len: 0, + lhs: self.tables.rule(rule_index).lhs, + next_nonterm_shift: crate::parser::table::ShiftTarget { + state: StateIndex::from_usize_unchecked(0), + push: false, + }, + next_plan: usize::MAX, + next_sibling: None, + }); + + let plan_checkpoint = self.feed_plans.len(); + let mut branch_extra_state_stack = self.take_feed_plan_extra_state_stack(); + branch_extra_state_stack.extend_from_slice(extra_state_stack); + + let (next_node_and_len, lhs, tokens_len, next_nonterm_shift) = self + .simulate_planned_reduction( + &mut branch_extra_state_stack, + node_and_len, + rule_index, + ); + + let next_plan = + self.plan_feed_impl(&mut branch_extra_state_stack, next_node_and_len, class); + self.put_feed_plan_extra_state_stack(branch_extra_state_stack); + + if let Some(next_plan) = next_plan { + self.feed_plan_reductions[reduction_id] = PlannedGlrReduction { + rule_index, + tokens_len, + lhs, + next_nonterm_shift, + next_plan, + next_sibling: None, + }; + + if let Some(previous_reduction) = previous_reduction { + self.feed_plan_reductions[previous_reduction].next_sibling = + Some(reduction_id); + } else { + first_reduction = Some(reduction_id); + } + previous_reduction = Some(reduction_id); + } else { + self.feed_plans.truncate(plan_checkpoint); + self.feed_plan_reductions.truncate(reduction_id); + } + } + } + + if shift.is_none() && first_reduction.is_none() { + debug_assert_eq!(self.feed_plans.len(), plan_id + 1); + self.feed_plans.pop(); + None + } else { + self.feed_plans[plan_id] = GlrFeedPlan { + shift, + first_reduction, + }; + Some(plan_id) + } + } + + fn shift_terminal_for_plan( + &mut self, + node: NodeId, + shift: crate::parser::table::ShiftTarget, + term: TerminalSymbol, + location: Data::Location, + userdata: Data::UserData, + ) where + P::Term: Clone, + { + let shift_node = if self.node(node).is_leaf() { + node + } else { + let new_node_idx = self.new_node_with_capacity(1); + self.add_child(node, new_node_idx); + new_node_idx + }; + + let node_ = self.node_mut(shift_node); + node_.state_stack.push(shift.state); + node_.location_stack.push(location); + #[cfg(feature = "tree")] + node_ + .tree_stack + .push(crate::tree::Tree::new_terminal(term.clone())); + + if shift.push { + match term { + TerminalSymbol::Terminal(term) => { + node_.data_stack.push(Data::new_terminal(term)); + } + TerminalSymbol::Error | TerminalSymbol::Eof | TerminalSymbol::VirtualStart(_) => { + node_.data_stack.push(Data::new_empty()); + } + } + } else { + node_.data_stack.push(Data::new_empty()); + } + + self.next_branches.push(Branch::new(shift_node, userdata)); + } + + fn apply_feed_plan( + &mut self, + plan_id: usize, + node: NodeId, + term: TerminalSymbol, + location: Data::Location, + userdata: Data::UserData, + ) -> Result<(), UnshiftedBranch> + where + Data: Clone, + Data::UserData: Clone, + P::Term: Clone, + P::NonTerm: std::fmt::Debug, + { + let plan = self.feed_plans[plan_id]; + + if let Some(mut reduction_id) = plan.first_reduction { + let mut shifted = false; + let mut reduced_node = node; + let mut reduced_userdata = None; + let mut shift_allowed = false; + + loop { + let reduction = self.feed_plan_reductions[reduction_id]; + let mut pass = plan.shift.is_some(); + let mut branch_userdata = userdata.clone(); + let can_reuse_node_for_reduce = + reduction.next_sibling.is_none() && plan.shift.is_none(); + + match self.reduce_planned( + reduction, + node, + &term, + &mut pass, + can_reuse_node_for_reduce, + &mut branch_userdata, + ) { + Ok(next_node) => { + shift_allowed |= pass; + match self.apply_feed_plan( + reduction.next_plan, + next_node, + term.clone(), + location.clone(), + branch_userdata, + ) { + Ok(()) => shifted = true, + Err(unshifted) => { + reduced_node = unshifted.node; + reduced_userdata = Some(unshifted.userdata); + } + } + } + Err(err) => { + shift_allowed |= pass; + self.reduce_errors.push(err); + } + } + + if let Some(next_sibling) = reduction.next_sibling { + reduction_id = next_sibling; + } else { + break; + } + } + + let mut shift = plan.shift; + if !shift_allowed { + if shift.is_some() { + self.try_remove_node_recursive(node); + shift = None; + } + } + + if let Some(shift) = shift { + self.shift_terminal_for_plan(node, shift, term, location, userdata); + Ok(()) + } else if shifted { + Ok(()) + } else { + Err(UnshiftedBranch { + node: reduced_node, + userdata: reduced_userdata.unwrap_or(userdata), + }) + } + } else if let Some(shift) = plan.shift { + self.shift_terminal_for_plan(node, shift, term, location, userdata); + Ok(()) + } else { + unreachable!("GLR feed plan must contain at least one shift or reduction") + } + } + fn feed_location_impl( &mut self, node: NodeId, @@ -1100,32 +1505,36 @@ impl< use crate::Location; let mut error_location_preserved = None; - let pop_count = loop { - let node_ = self.node(node); - if !node_.is_leaf() { + let (pop_count, plan_id) = loop { + if !self.node(node).is_leaf() { self.try_remove_node(node); return; } + let node_len = self.node(node).len(); let mut pop_count = 0; - let mut found = false; + let mut found_plan = None; - for _ in node_.state_stack.iter().rev() { + for retained_len in (1..=node_len).rev() { extra_state_stack.clear(); + self.feed_plans.clear(); + self.feed_plan_reductions.clear(); - if self.can_feed_impl( + if let Some(plan_id) = self.plan_feed_impl( extra_state_stack, - Some((node, NonZeroUsize::new(node_.len() - pop_count).unwrap())), + Some((node, NonZeroUsize::new(retained_len).unwrap())), P::TermClass::ERROR, ) { - found = true; + found_plan = Some(plan_id); break; } pop_count += 1; } - if !found { + if let Some(plan_id) = found_plan { + break (pop_count, plan_id); + } else { error_location_preserved = Some(if let Some(prev) = error_location_preserved { Data::Location::new( std::iter::once(&prev).chain(self.location_iter(node)), @@ -1141,8 +1550,6 @@ impl< } else { return; } - } else { - break pop_count; } }; @@ -1164,17 +1571,17 @@ impl< let l = node_.tree_stack.len() - pop_count; node_.tree_stack.truncate(l); } - match self.feed_location_impl( + match self.apply_feed_plan( + plan_id, node, TerminalSymbol::Error, - P::TermClass::ERROR, error_location, userdata, ) { Ok(()) => {} Err(unshifted) => { self.try_remove_node(unshifted.node); - } // other errors + } } } /// Feed one terminal with location to parser, and update state stack. @@ -1206,62 +1613,52 @@ impl< let class = P::TermClass::from_term(&term); - let mut current_branches = std::mem::take(&mut self.current_branches); - let mut feedable_branch_seen = false; - for Branch { node, userdata } in current_branches.drain(..) { - // Classify this branch before mutating the graph-structured stack. Only branches that - // fail this CFG simulation are considered `NoAction` branches. - let mut extra_state_stack = Vec::new(); - let node_and_len = { - let node_ = self.node(node); - if node_.len() == 0 { - None - } else { - Some((node, NonZeroUsize::new(node_.len()).unwrap())) - } - }; - - if self.can_feed_impl(&mut extra_state_stack, node_and_len, class) { - feedable_branch_seen = true; + let mut feedable_branch_found = false; + let mut accepted_branch_found = false; + let mut extra_state_stack = self.take_feed_plan_extra_state_stack(); + while let Some(Branch { node, userdata }) = self.current_branches.pop() { + // Plan before mutating the graph-structured stack so syntax failures stay cleanly + // separated from semantic reduce-action failures. + self.feed_plans.clear(); + self.feed_plan_reductions.clear(); + extra_state_stack.clear(); - // The CFG admits this terminal on this branch. From here on, failures are caused - // by semantic reduce actions or runtime conflict decisions, so they must not - // trigger error recovery. - let _ = self.feed_location_impl( - node, - TerminalSymbol::Terminal(term.clone()), - class, - location.clone(), - userdata, - ); + if let Some(plan_id) = self.plan_feed_impl( + &mut extra_state_stack, + self.branch_node_and_len(node), + class, + ) { + feedable_branch_found = true; + + // Once the CFG admits this terminal, later failures are semantic/runtime branch + // pruning and must not be reclassified as syntax recovery input. + if self + .apply_feed_plan( + plan_id, + node, + TerminalSymbol::Terminal(term.clone()), + location.clone(), + userdata, + ) + .is_ok() + { + accepted_branch_found = true; + } } else { - // This branch cannot grammatically shift the lookahead. Keep it only as a - // recovery candidate in case every active branch is also `NoAction`. + // Keep grammatical `NoAction` branches only as recovery candidates; a surviving + // sibling branch makes recovery inappropriate for this lookahead. self.fallback_branches.push(Branch::new(node, userdata)); } } - // put back for reused allocated memory - self.current_branches = current_branches; + self.put_feed_plan_extra_state_stack(extra_state_stack); - // next_branches is empty; invalid terminal was given - // check for panic mode - // and restore nodes to original state from fallback_branches - if self.next_branches.is_empty() { - if feedable_branch_seen { - // A branch was grammatically able to shift the terminal, but failed while - // committing semantic/runtime actions. That is not syntax recovery input. - std::mem::swap(&mut self.current_branches, &mut self.fallback_branches); + if !accepted_branch_found { + debug_assert!(self.next_branches.is_empty()); - return Err(ParseError { - term: TerminalSymbol::Terminal(term), - location, - reduce_action_errors: std::mem::take(&mut self.reduce_errors), - states: self.states().collect(), - }); - } + if !P::ERROR_USED || feedable_branch_found { + // Recovery is reserved for pure syntax failure. If an admitted branch died while + // replaying its plan, report that semantic/runtime failure instead. - // early return if `error` token is not used in the grammar - if !P::ERROR_USED { std::mem::swap(&mut self.current_branches, &mut self.fallback_branches); return Err(ParseError { @@ -1272,30 +1669,14 @@ impl< }); } - let mut fallback_branches = std::mem::take(&mut self.fallback_branches); - let mut extra_state_stack = Vec::new(); - // try enter panic mode and store error nodes to next_branches - for Branch { node, userdata } in fallback_branches.drain(..) { - // GLR recovery only searches concrete branch stacks. The implicit root state 0 is - // before the virtual start symbol and cannot accept a synthetic `error` token. + let mut extra_state_stack = self.take_feed_plan_extra_state_stack(); + while let Some(Branch { node, userdata }) = self.fallback_branches.pop() { + // Recovery searches only concrete branch stacks; the implicit root state precedes + // the virtual start symbol and cannot accept a synthetic `error`. self.panic_mode(node, userdata, &mut extra_state_stack); } - // put back for reuse allocated memory - self.fallback_branches = fallback_branches; + self.put_feed_plan_extra_state_stack(extra_state_stack); - if !self.reduce_errors.is_empty() { - // Shifting the synthetic `error` token may execute reduce actions. If one fails, - // recovery itself failed semantically and must be reported. - return Err(ParseError { - term: TerminalSymbol::Terminal(term), - location, - reduce_action_errors: std::mem::take(&mut self.reduce_errors), - states: self.states().collect(), - }); - } - - // if next_node is still empty, then no panic mode was entered, this is an error - // restore current_branches to fallback_branches if self.next_branches.is_empty() { Err(ParseError { term: TerminalSymbol::Terminal(term), @@ -1304,37 +1685,37 @@ impl< states: self.states().collect(), }) } else { - // try shift term to error state - let mut next_branches = std::mem::take(&mut self.next_branches); - for Branch { + debug_assert!(self.current_branches.is_empty()); + std::mem::swap(&mut self.current_branches, &mut self.next_branches); + let mut extra_state_stack = self.take_feed_plan_extra_state_stack(); + // `error` was feed, now check with the original terminal symbol again. + // If the original terminal is still not accepted, it is part of the `error`. + while let Some(Branch { node: error_node, userdata, - } in next_branches.drain(..) + }) = self.current_branches.pop() { - let last_state = self.state(error_node); - if let Some(next_state) = self.tables.shift_goto_class(last_state, class) { - // A -> a . error b - // and b is fed, shift error and b - let node = self.node_mut(error_node); - node.state_stack.push(next_state.state); - node.location_stack.push(location.clone()); - #[cfg(feature = "tree")] - node.tree_stack.push(crate::tree::Tree::new_terminal( - TerminalSymbol::Terminal(term.clone()), - )); - - if next_state.push { - node.data_stack.push(Data::new_terminal(term.clone())); - } else { - node.data_stack.push(Data::new_empty()); - } + self.feed_plans.clear(); + self.feed_plan_reductions.clear(); + extra_state_stack.clear(); - self.current_branches - .push(Branch::new(error_node, userdata)); + if let Some(plan_id) = self.plan_feed_impl( + &mut extra_state_stack, + self.branch_node_and_len(error_node), + class, + ) { + // After shifting `error`, the original lookahead is a normal feed again: + // it may need nullable reductions before the terminal shift is visible. + let _ = self.apply_feed_plan( + plan_id, + error_node, + TerminalSymbol::Terminal(term.clone()), + location.clone(), + userdata, + ); } else { - // here, fed token is in `error` non-terminal - // so merge location with previous - + // Otherwise the lookahead is part of the `error`; merge it so + // diagnostics report the full discarded range. let new_location = Data::Location::new( std::iter::once(&location).chain(self.location_iter(error_node)), 2, // error node + fed token @@ -1342,18 +1723,42 @@ impl< let node = self.node_mut(error_node); *node.location_stack.last_mut().unwrap() = new_location; - self.current_branches - .push(Branch::new(error_node, userdata)); + self.next_branches.push(Branch::new(error_node, userdata)); } } - self.next_branches = next_branches; - // Recovery consumed or merged the lookahead. This is a successful feed, and the - // original `NoAction` branches have been transformed into recovery branches. - Ok(FeedSuccess { errors: None }) + self.put_feed_plan_extra_state_stack(extra_state_stack); + + if self.next_branches.is_empty() { + Err(ParseError { + term: TerminalSymbol::Terminal(term), + location, + reduce_action_errors: std::mem::take(&mut self.reduce_errors), + states: self.states().collect(), + }) + } else { + // A recovered branch keeps the feed successful. Semantic failures from sibling + // recovery branches are still surfaced, matching ordinary GLR branch pruning. + let errors = if self.reduce_errors.is_empty() { + None + } else { + Some(ParseError { + term: TerminalSymbol::Terminal(term), + location: location.clone(), + reduce_action_errors: std::mem::take(&mut self.reduce_errors), + states: Vec::new(), + }) + }; + + std::mem::swap(&mut self.current_branches, &mut self.next_branches); + // Recovery consumed or absorbed the lookahead, so the original syntax failure + // has been transformed into live recovery branches. + Ok(FeedSuccess { errors }) + } } } else { - // At least one original branch accepted the terminal. Report sibling branch failures - // to callers without treating this feed as fatal. + debug_assert!(!self.next_branches.is_empty()); + // Surviving original branches define feed success; sibling failures are diagnostics, + // not a fatal parse result. let errors = if self.fallback_branches.is_empty() && self.reduce_errors.is_empty() { None } else { @@ -1542,20 +1947,12 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars /// and will return `true` if `Err` variant is returned by `reduce_action`. pub fn can_feed(&self, term: &P::Term) -> bool { let class = P::TermClass::from_term(term); - let mut extra_state_stack = Vec::new(); - self.current_branches.iter().any(move |branch| { + let mut extra_state_stack = self.can_feed_extra_state_stack.borrow_mut(); + self.current_branches.iter().any(|branch| { let node = branch.node; extra_state_stack.clear(); - let node_and_len = { - let node_ = self.node(node); - if node_.len() == 0 { - // only root node can have 0 length stack - None - } else { - Some((node, NonZeroUsize::new(node_.len()).unwrap())) - } - }; + let node_and_len = self.branch_node_and_len(node); self.can_feed_impl(&mut extra_state_stack, node_and_len, class) }) } @@ -1624,8 +2021,7 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars Data::Location::new(std::iter::empty(), 0) }; - let mut current_branches = std::mem::take(&mut self.current_branches); - for Branch { node, userdata } in current_branches.drain(..) { + while let Some(Branch { node, userdata }) = self.current_branches.pop() { let node_eof_location = Data::Location::new(self.location_iter(node), 0); if let Err(unshifted) = self.feed_location_impl( node, @@ -1638,7 +2034,6 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars .push(Branch::new(unshifted.node, unshifted.userdata)); } } - self.current_branches = current_branches; // next_branches is empty; invalid terminal was given // check for panic mode @@ -1712,6 +2107,10 @@ where next_branches: Default::default(), reduce_errors: Default::default(), fallback_branches: Default::default(), + feed_plans: Default::default(), + feed_plan_reductions: Default::default(), + feed_plan_extra_state_stacks: Default::default(), + can_feed_extra_state_stack: Default::default(), tables: self.tables, _phantom: std::marker::PhantomData, } diff --git a/rusty_lr_parser/src/grammar.rs b/rusty_lr_parser/src/grammar.rs index 9597b1e5..463c3d15 100644 --- a/rusty_lr_parser/src/grammar.rs +++ b/rusty_lr_parser/src/grammar.rs @@ -4149,11 +4149,15 @@ mod tests { let grammar_args = Grammar::parse_args(input).expect("Should recover from malformed symbol binding"); - assert_eq!(grammar_args.error_recovered.len(), 1); + assert_eq!(grammar_args.error_recovered.len(), 2); assert_eq!( grammar_args.error_recovered[0].message, "Expected pattern after symbol binding" ); + assert_eq!( + grammar_args.error_recovered[1].message, + "Expected %prec or %dprec" + ); assert_eq!(grammar_args.rules.len(), 2); assert_eq!(grammar_args.rules[0].name.value(), "Expr"); assert_eq!(grammar_args.rules[1].name.value(), "Term"); From 246f89d70d86d8846be38b01d683932ba462b5c1 Mon Sep 17 00:00:00 2001 From: Taehwan Kim Date: Sun, 5 Jul 2026 12:11:02 +0900 Subject: [PATCH 3/3] gemini suggestions --- example/glr/src/main.rs | 2 +- rusty_lr/tests/lr_glr_operation.rs | 244 +++++++ .../src/parser/nondeterministic/context.rs | 690 ++++++------------ 3 files changed, 482 insertions(+), 454 deletions(-) diff --git a/example/glr/src/main.rs b/example/glr/src/main.rs index 59f378b2..ab720474 100644 --- a/example/glr/src/main.rs +++ b/example/glr/src/main.rs @@ -177,7 +177,7 @@ mod issue_89_optional_empty_branch { #[test] fn refself_no_panic() { - // `amp self_kw` — previously caused a panic inside feed_location_impl + // `amp self_kw` — previously caused a panic inside GLR feed planning // due to a non-leaf node being pushed to next_nodes after an empty // reduce produced a child of the original node. let mut ctx = ParamContext::with_default_userdata(); diff --git a/rusty_lr/tests/lr_glr_operation.rs b/rusty_lr/tests/lr_glr_operation.rs index 808973a7..95fc13e4 100644 --- a/rusty_lr/tests/lr_glr_operation.rs +++ b/rusty_lr/tests/lr_glr_operation.rs @@ -383,6 +383,43 @@ mod glr_ambiguous_concatenation { } } +mod glr_can_feed_planning { + use rusty_lr::lr1; + + lr1! { + %glr; + %nooptim; + %tokentype char; + %start S; + + S(&'static str) + : A 'x' { + "x" + } + | B 'y' { + "y" + } + | Empty { + "empty" + } + ; + + A(()): Empty { () }; + B(()): Empty { () }; + Empty(()): "" { () }; + } + + #[test] + fn glr_can_feed_and_can_accept_reuse_feed_planning_through_reduce_chains() { + let ctx = SContext::with_default_userdata(); + + assert!(ctx.can_feed(&'x')); + assert!(ctx.can_feed(&'y')); + assert!(!ctx.can_feed(&'z')); + assert!(ctx.can_accept()); + } +} + mod glr_dynamic_precedence { use rusty_lr::lr1; @@ -569,9 +606,216 @@ mod reduce_action_errors { .errors .expect("failed recovery branch should be reported"); assert_eq!(errors.reduce_action_errors, ["bad recovery branch"]); + ctx.debug_check(); ctx.feed('s').unwrap(); + ctx.debug_check(); assert_eq!(ctx.accept().unwrap(), ("recovered", ())); } } + + mod glr_failed_sibling_cleanup { + use rusty_lr::lr1; + + // One nullable sibling succeeds, while another reaches a deeper reduce-action failure. + // The failed branch must be detached from the GSS when the successful sibling survives. + lr1! { + %glr; + %nooptim; + %tokentype char; + %error &'static str; + %start S; + + S(&'static str) + : Good GoodTail { + "good" + } + | Bad BadTail { + "bad" + } + ; + + Good(()): "" { () }; + GoodTail(()): 'x' { () }; + + Bad(()): "" { () }; + BadTail(()): BadReduce 'x' { () }; + BadReduce(()): "" { + if true { + return Err("bad nested branch"); + } + () + }; + } + + #[test] + fn glr_feed_cleans_failed_sibling_nodes_when_another_sibling_survives() { + let mut ctx = SContext::with_default_userdata(); + let success = ctx.feed('x').unwrap(); + + let errors = success + .errors + .expect("failed sibling branch should be reported"); + assert_eq!(errors.reduce_action_errors, ["bad nested branch"]); + + ctx.debug_check(); + assert_eq!(ctx.accept().unwrap(), ("good", ())); + } + } + + mod glr_no_action_sibling_cleanup { + use rusty_lr::lr1; + + // Feeding `p` creates two live GLR branches. The following `q` keeps only the `A` + // branch, so the `B` branch is a grammatical NoAction sibling and must be removed. + lr1! { + %glr; + %nooptim; + %tokentype char; + %start S; + + S(&'static str) + : A 'q' { + "a" + } + | B 'r' { + "b" + } + ; + + A(()): X 'p' { () }; + B(()): Y 'p' { () }; + X(()): 'x' { () }; + Y(()): 'x' { () }; + } + + #[test] + fn glr_feed_cleans_no_action_sibling_nodes_when_another_branch_survives() { + let mut ctx = SContext::with_default_userdata(); + + ctx.feed('x').unwrap(); + ctx.feed('p').unwrap(); + ctx.debug_check(); + + let success = ctx.feed('q').unwrap(); + assert!(success.errors.is_some()); + + ctx.debug_check(); + assert_eq!(ctx.accept().unwrap(), ("a", ())); + } + } + + mod glr_panic_mode_node_cleanup { + use rusty_lr::lr1; + + // The `abc` branch lets `b` be shifted normally. When the next token is invalid, + // recovery must pop that shifted `b` node back to the state after `a`, where + // `error 's'` is available. The popped node must be removed from the GSS. + lr1! { + %glr; + %nooptim; + %tokentype char; + %start S; + + S(&'static str) + : 'a' error 's' { + "recovered" + } + | 'a' 'b' 'c' { + "abc" + } + ; + } + + #[test] + fn glr_panic_mode_cleans_nodes_popped_during_recovery() { + let mut ctx = SContext::with_default_userdata(); + + ctx.feed('a').unwrap(); + ctx.feed('b').unwrap(); + ctx.debug_check(); + assert!(ctx.can_panic()); + + ctx.feed('x').unwrap(); + ctx.debug_check(); + + ctx.feed('s').unwrap(); + ctx.debug_check(); + assert_eq!(ctx.accept().unwrap(), ("recovered", ())); + } + } + + mod glr_conflict_free_reduce_reuse { + use rusty_lr::{Location, lr1}; + use std::sync::atomic::{AtomicUsize, Ordering}; + + static DATA_CLONES: AtomicUsize = AtomicUsize::new(0); + static WATCHED_LOCATION_CLONES: AtomicUsize = AtomicUsize::new(0); + + #[derive(Debug, PartialEq, Eq)] + struct CloneWatchedValue(&'static str); + + impl Clone for CloneWatchedValue { + fn clone(&self) -> Self { + DATA_CLONES.fetch_add(1, Ordering::SeqCst); + Self(self.0) + } + } + + #[derive(Debug, PartialEq, Eq)] + struct CloneWatchedLocation { + reduced: bool, + } + + impl Clone for CloneWatchedLocation { + fn clone(&self) -> Self { + if self.reduced { + WATCHED_LOCATION_CLONES.fetch_add(1, Ordering::SeqCst); + } + Self { + reduced: self.reduced, + } + } + } + + impl Location for CloneWatchedLocation { + fn new<'a>(_stack: impl Iterator + Clone, len: usize) -> Self + where + Self: 'a, + { + Self { reduced: len > 0 } + } + } + + lr1! { + %glr; + %nooptim; + %tokentype char; + %location CloneWatchedLocation; + %start S; + + S(CloneWatchedValue): Pair 'c' { Pair }; + Pair(CloneWatchedValue): Item 'b' { Item }; + Item(CloneWatchedValue): 'a' { CloneWatchedValue("item") }; + } + + #[test] + fn glr_conflict_free_reduce_does_not_clone_semantic_or_location_stacks() { + DATA_CLONES.store(0, Ordering::SeqCst); + WATCHED_LOCATION_CLONES.store(0, Ordering::SeqCst); + + let mut ctx = SContext::with_default_userdata(); + ctx.feed_location('a', CloneWatchedLocation { reduced: false }) + .unwrap(); + ctx.feed_location('b', CloneWatchedLocation { reduced: false }) + .unwrap(); + ctx.feed_location('c', CloneWatchedLocation { reduced: false }) + .unwrap(); + + let (value, _) = ctx.accept().unwrap(); + assert_eq!(value, CloneWatchedValue("item")); + assert_eq!(DATA_CLONES.load(Ordering::SeqCst), 0); + assert_eq!(WATCHED_LOCATION_CLONES.load(Ordering::SeqCst), 0); + } + } } diff --git a/rusty_lr_core/src/parser/nondeterministic/context.rs b/rusty_lr_core/src/parser/nondeterministic/context.rs index a2e09e18..304a882f 100644 --- a/rusty_lr_core/src/parser/nondeterministic/context.rs +++ b/rusty_lr_core/src/parser/nondeterministic/context.rs @@ -95,11 +95,15 @@ struct Branch { /// Branch state returned when a GLR feed path reaches no terminal shift. /// -/// The caller only needs the graph node and user data to keep or discard that branch; the lookahead -/// terminal and location are owned by the failed attempt and do not need to be returned. -struct UnshiftedBranch { +/// The caller only needs the graph node to discard that branch; the lookahead terminal, location, +/// and user data are owned by the failed attempt and do not need to be returned. +struct UnshiftedBranch { node: NodeId, - userdata: UserData, +} + +enum FailedFeedPlan { + Unshifted(UnshiftedBranch), + Removed, } #[derive(Clone, Copy)] @@ -170,6 +174,14 @@ pub struct Context< /// allocating a fresh top-level `Vec` for each query. can_feed_extra_state_stack: RefCell>, + /// Reusable plan storage for `can_feed`/`can_accept`/`can_panic`. + /// + /// These queries share the same CFG-only planner as real feeds, but keep their buffers + /// separate because they only borrow the context immutably. + can_feed_plans: RefCell>>, + can_feed_plan_reductions: RefCell>>, + can_feed_plan_extra_state_stacks: RefCell>>, + /// Decoded parser tables shared by every active GLR branch. /// /// Branches clone stack/userdata state, but they all read the same immutable runtime tables. @@ -200,6 +212,9 @@ impl< feed_plan_reductions: Default::default(), feed_plan_extra_state_stacks: Default::default(), can_feed_extra_state_stack: Default::default(), + can_feed_plans: Default::default(), + can_feed_plan_reductions: Default::default(), + can_feed_plan_extra_state_stacks: Default::default(), tables: P::get_tables(), _phantom: std::marker::PhantomData, }; @@ -291,6 +306,9 @@ impl< &mut self.nodes_pool[node.index()] } + fn node_is_live(&self, node: NodeId) -> bool { + node.index() < self.nodes_pool.len() && !self.empty_node_indices.contains(&node) + } /// for debugging; checks for memory leak, not freed nodes, etc. pub fn debug_check(&self) { @@ -390,6 +408,31 @@ impl< break parent; } } + pub(crate) fn try_remove_node_subtree_recursive(&mut self, node: NodeId) -> Option { + let children = self + .nodes_pool + .iter() + .enumerate() + .filter_map(|(idx, child)| { + let child_id = NodeId::new(idx); + if !self.empty_node_indices.contains(&child_id) && child.parent == Some(node) { + Some(child_id) + } else { + None + } + }) + .collect::>(); + + for child in children { + self.try_remove_node_subtree_recursive(child); + } + + if self.node_is_live(node) { + self.try_remove_node_recursive(node) + } else { + None + } + } /// Get iterator for all nodes in the current context. fn node_iter( @@ -637,68 +680,6 @@ impl< } } - /// give lookahead token to parser, and check if there is any reduce action. - /// returns false if shift action is revoked - fn reduce( - &mut self, - reduce_rule: usize, - node: NodeId, - term: &crate::TerminalSymbol, - shift: &mut bool, - can_reuse_node_for_reduce: bool, - userdata: &mut Data::UserData, - ) -> Result - where - Data: Clone, - P::Term: Clone, - P::NonTerm: std::fmt::Debug, - { - use crate::Location; - let rule = *self.tables.rule(reduce_rule); - let count = rule.len; - let mut new_location = Data::Location::new(self.location_iter(node), count); - - let node_to_shift = self.prepare_reduce_node(node, count, count, can_reuse_node_for_reduce); - - let state = self.state(node_to_shift); - let Some(next_nonterm_shift) = self.tables.shift_goto_nonterm(state, rule.lhs) else { - unreachable!( - "Failed to shift non-terminal: {:?} in state {}", - rule.lhs, state - ); - }; - - let node = self.node_mut(node_to_shift); - #[cfg(feature = "tree")] - let trees = node.tree_stack.split_off(node.tree_stack.len() - count); - - match Data::reduce_action( - &mut node.data_stack, - &mut node.location_stack, - next_nonterm_shift.push, - reduce_rule, - shift, - term, - userdata, - &mut new_location, - ) { - Ok(_) => { - node.state_stack.push(next_nonterm_shift.state); - node.location_stack.push(new_location); - #[cfg(feature = "tree")] - { - node.tree_stack - .push(crate::tree::Tree::new_nonterminal(rule.lhs.clone(), trees)); - } - Ok(node_to_shift) - } - Err(err) => { - self.try_remove_node_recursive(node_to_shift); - Err(err) - } - } - } - fn reduce_planned( &mut self, reduction: PlannedGlrReduction, @@ -1038,6 +1019,17 @@ impl< self.feed_plan_extra_state_stacks.push(stack); } + fn take_extra_state_stack(scratch: &mut Vec>) -> Vec { + let mut stack = scratch.pop().unwrap_or_default(); + stack.clear(); + stack + } + + fn put_extra_state_stack(scratch: &mut Vec>, mut stack: Vec) { + stack.clear(); + scratch.push(stack); + } + fn simulate_planned_reduction( &self, extra_state_stack: &mut Vec, @@ -1094,6 +1086,36 @@ impl< extra_state_stack: &mut Vec, node_and_len: Option<(NodeId, NonZeroUsize)>, class: P::TermClass, + ) -> Option { + let mut feed_plans = std::mem::take(&mut self.feed_plans); + let mut feed_plan_reductions = std::mem::take(&mut self.feed_plan_reductions); + let mut feed_plan_extra_state_stacks = + std::mem::take(&mut self.feed_plan_extra_state_stacks); + + let plan_id = self.plan_feed_impl_with_storage( + extra_state_stack, + node_and_len, + class, + &mut feed_plans, + &mut feed_plan_reductions, + &mut feed_plan_extra_state_stacks, + ); + + self.feed_plans = feed_plans; + self.feed_plan_reductions = feed_plan_reductions; + self.feed_plan_extra_state_stacks = feed_plan_extra_state_stacks; + + plan_id + } + + fn plan_feed_impl_with_storage( + &self, + extra_state_stack: &mut Vec, + node_and_len: Option<(NodeId, NonZeroUsize)>, + class: P::TermClass, + feed_plans: &mut Vec>, + feed_plan_reductions: &mut Vec>, + feed_plan_extra_state_stacks: &mut Vec>, ) -> Option { use crate::parser::table::ReduceRules; @@ -1103,8 +1125,8 @@ impl< None => (None, None), }; - let plan_id = self.feed_plans.len(); - self.feed_plans.push(GlrFeedPlan { + let plan_id = feed_plans.len(); + feed_plans.push(GlrFeedPlan { shift, first_reduction: None, }); @@ -1115,8 +1137,8 @@ impl< if let Some(reduce_rules) = reduce { for reduce_rule in reduce_rules.to_iter() { let rule_index = reduce_rule.into_usize(); - let reduction_id = self.feed_plan_reductions.len(); - self.feed_plan_reductions.push(PlannedGlrReduction { + let reduction_id = feed_plan_reductions.len(); + feed_plan_reductions.push(PlannedGlrReduction { rule_index, tokens_len: 0, lhs: self.tables.rule(rule_index).lhs, @@ -1128,8 +1150,9 @@ impl< next_sibling: None, }); - let plan_checkpoint = self.feed_plans.len(); - let mut branch_extra_state_stack = self.take_feed_plan_extra_state_stack(); + let plan_checkpoint = feed_plans.len(); + let mut branch_extra_state_stack = + Self::take_extra_state_stack(feed_plan_extra_state_stacks); branch_extra_state_stack.extend_from_slice(extra_state_stack); let (next_node_and_len, lhs, tokens_len, next_nonterm_shift) = self @@ -1139,12 +1162,18 @@ impl< rule_index, ); - let next_plan = - self.plan_feed_impl(&mut branch_extra_state_stack, next_node_and_len, class); - self.put_feed_plan_extra_state_stack(branch_extra_state_stack); + let next_plan = self.plan_feed_impl_with_storage( + &mut branch_extra_state_stack, + next_node_and_len, + class, + feed_plans, + feed_plan_reductions, + feed_plan_extra_state_stacks, + ); + Self::put_extra_state_stack(feed_plan_extra_state_stacks, branch_extra_state_stack); if let Some(next_plan) = next_plan { - self.feed_plan_reductions[reduction_id] = PlannedGlrReduction { + feed_plan_reductions[reduction_id] = PlannedGlrReduction { rule_index, tokens_len, lhs, @@ -1154,25 +1183,24 @@ impl< }; if let Some(previous_reduction) = previous_reduction { - self.feed_plan_reductions[previous_reduction].next_sibling = - Some(reduction_id); + feed_plan_reductions[previous_reduction].next_sibling = Some(reduction_id); } else { first_reduction = Some(reduction_id); } previous_reduction = Some(reduction_id); } else { - self.feed_plans.truncate(plan_checkpoint); - self.feed_plan_reductions.truncate(reduction_id); + feed_plans.truncate(plan_checkpoint); + feed_plan_reductions.truncate(reduction_id); } } } if shift.is_none() && first_reduction.is_none() { - debug_assert_eq!(self.feed_plans.len(), plan_id + 1); - self.feed_plans.pop(); + debug_assert_eq!(feed_plans.len(), plan_id + 1); + feed_plans.pop(); None } else { - self.feed_plans[plan_id] = GlrFeedPlan { + feed_plans[plan_id] = GlrFeedPlan { shift, first_reduction, }; @@ -1229,7 +1257,7 @@ impl< term: TerminalSymbol, location: Data::Location, userdata: Data::UserData, - ) -> Result<(), UnshiftedBranch> + ) -> Result<(), FailedFeedPlan> where Data: Clone, Data::UserData: Clone, @@ -1240,9 +1268,10 @@ impl< if let Some(mut reduction_id) = plan.first_reduction { let mut shifted = false; - let mut reduced_node = node; - let mut reduced_userdata = None; + let mut failed_branch = None; let mut shift_allowed = false; + let mut node_removed = false; + let mut reused_node_kept = false; loop { let reduction = self.feed_plan_reductions[reduction_id]; @@ -1261,6 +1290,7 @@ impl< ) { Ok(next_node) => { shift_allowed |= pass; + let mut branch_accepted = false; match self.apply_feed_plan( reduction.next_plan, next_node, @@ -1268,15 +1298,26 @@ impl< location.clone(), branch_userdata, ) { - Ok(()) => shifted = true, - Err(unshifted) => { - reduced_node = unshifted.node; - reduced_userdata = Some(unshifted.userdata); + Ok(()) => { + shifted = true; + branch_accepted = true; } + Err(FailedFeedPlan::Unshifted(unshifted)) => { + if let Some(previous) = failed_branch.replace(unshifted) { + self.try_remove_node_subtree_recursive(previous.node); + } + } + Err(FailedFeedPlan::Removed) => {} + } + if can_reuse_node_for_reduce { + reused_node_kept = branch_accepted; } } Err(err) => { shift_allowed |= pass; + if can_reuse_node_for_reduce { + node_removed = true; + } self.reduce_errors.push(err); } } @@ -1292,20 +1333,35 @@ impl< if !shift_allowed { if shift.is_some() { self.try_remove_node_recursive(node); + node_removed = true; shift = None; } } if let Some(shift) = shift { + if let Some(unshifted) = failed_branch { + self.try_remove_node_subtree_recursive(unshifted.node); + } self.shift_terminal_for_plan(node, shift, term, location, userdata); Ok(()) } else if shifted { + if let Some(unshifted) = failed_branch { + self.try_remove_node_subtree_recursive(unshifted.node); + } + if !node_removed + && !reused_node_kept + && self.node_is_live(node) + && self.node(node).is_leaf() + { + self.try_remove_node_recursive(node); + } Ok(()) } else { - Err(UnshiftedBranch { - node: reduced_node, - userdata: reduced_userdata.unwrap_or(userdata), - }) + match failed_branch { + Some(failed_branch) => Err(FailedFeedPlan::Unshifted(failed_branch)), + None if node_removed => Err(FailedFeedPlan::Removed), + None => Err(FailedFeedPlan::Unshifted(UnshiftedBranch { node })), + } } } else if let Some(shift) = plan.shift { self.shift_terminal_for_plan(node, shift, term, location, userdata); @@ -1315,182 +1371,16 @@ impl< } } - fn feed_location_impl( - &mut self, - node: NodeId, - term: TerminalSymbol, - class: P::TermClass, - location: Data::Location, - userdata: Data::UserData, - ) -> Result<(), UnshiftedBranch> - where - Data: Clone, - Data::UserData: Clone, - P::Term: Clone, - P::NonTerm: std::fmt::Debug, - { - debug_assert!(self.node(node).is_leaf()); - use crate::parser::table::ReduceRules; - - let last_state = self.state(node); - let (shift_state, reduce) = match self.tables.term_action(last_state, class) { - Some(action) => (action.shift(), action.reduce()), - None => (None, None), - }; - if let Some(reduce_rules) = reduce { - let mut shift = shift_state; - - let mut shifted = false; - let mut reduced_node = node; - let mut reduced_userdata = None; - - // Each GLR branch receives its own user data copy before running reduce actions. - let mut shift_ = false; - let l = reduce_rules.to_iter().len(); - for (idx, reduce_rule) in reduce_rules.to_iter().enumerate() { - let mut pass = shift.is_some(); - let mut branch_userdata = userdata.clone(); - - // The current node may still be needed by later reduce alternatives - // or by the shift branch, so only the last reduce without a pending - // shift is allowed to reuse it in-place. - let can_reuse_node_for_reduce = idx == l - 1 && shift.is_none(); - match self.reduce( - reduce_rule.into_usize(), - node, - &term, - &mut pass, - can_reuse_node_for_reduce, - &mut branch_userdata, - ) { - Ok(next_node) => { - shift_ |= pass; - // reduce recursively - - match self.feed_location_impl( - next_node, - term.clone(), - class, - location.clone(), - branch_userdata, - ) { - Ok(_) => { - shifted = true; - } - Err(unshifted) => { - reduced_node = unshifted.node; - reduced_userdata = Some(unshifted.userdata); - } - } - } - Err(err) => { - shift_ |= pass; - self.reduce_errors.push(err); - } - } - } - // if every reduce action revoked shift, - // then reset shift to None - if !shift_ { - if shift.is_some() { - // remove node recursive - self.try_remove_node_recursive(node); - shift = None; - } - } - if let Some(shift) = shift { - // If `node` acquired children during the reduce loop (e.g. an - // empty rule created a child via `prepare_reduce_node`), it is - // no longer a leaf. Pushing it directly to `next_branches` would - // violate the `is_leaf()` precondition of the next - // `feed_location_impl` call. Instead, create a fresh leaf - // child that carries only the shift result. - let shift_node = if self.node(node).is_leaf() { - node - } else { - let new_node_idx = self.new_node_with_capacity(1); - self.add_child(node, new_node_idx); - new_node_idx - }; - - let node_ = self.node_mut(shift_node); - node_.state_stack.push(shift.state); - node_.location_stack.push(location.clone()); - #[cfg(feature = "tree")] - node_ - .tree_stack - .push(crate::tree::Tree::new_terminal(term.clone())); - - if shift.push { - match term { - TerminalSymbol::Terminal(term) => { - node_.data_stack.push(Data::new_terminal(term)); - } - TerminalSymbol::Error - | TerminalSymbol::Eof - | TerminalSymbol::VirtualStart(_) => { - node_.data_stack.push(Data::new_empty()); - } - } - } else { - match term { - TerminalSymbol::Terminal(_) - | TerminalSymbol::Error - | TerminalSymbol::Eof - | TerminalSymbol::VirtualStart(_) => { - node_.data_stack.push(Data::new_empty()); - } - } - } - - self.next_branches.push(Branch::new(shift_node, userdata)); - Ok(()) - } else if shifted { - Ok(()) - } else { - Err(UnshiftedBranch { - node: reduced_node, - userdata: reduced_userdata.unwrap_or(userdata), - }) - } - } else if let Some(shift) = shift_state { - let node_ = self.node_mut(node); - node_.state_stack.push(shift.state); - node_.location_stack.push(location); - #[cfg(feature = "tree")] - node_ - .tree_stack - .push(crate::tree::Tree::new_terminal(term.clone())); - - if shift.push { - match term { - TerminalSymbol::Terminal(term) => { - node_.data_stack.push(Data::new_terminal(term)); - } - TerminalSymbol::Error - | TerminalSymbol::Eof - | TerminalSymbol::VirtualStart(_) => { - node_.data_stack.push(Data::new_empty()); - } - } - } else { - match term { - TerminalSymbol::Terminal(_) - | TerminalSymbol::Error - | TerminalSymbol::Eof - | TerminalSymbol::VirtualStart(_) => { - node_.data_stack.push(Data::new_empty()); - } - } - } + fn discard_branch_node(&mut self, node: NodeId) { + self.try_remove_node_subtree_recursive(node); + } - self.next_branches.push(Branch::new(node, userdata)); - Ok(()) - } else { - // no reduce, no shift - Err(UnshiftedBranch { node, userdata }) + fn discard_fallback_branches(&mut self) { + while let Some(branch) = self.fallback_branches.pop() { + self.discard_branch_node(branch.node); } } + fn panic_mode( &mut self, mut node: NodeId, @@ -1579,9 +1469,10 @@ impl< userdata, ) { Ok(()) => {} - Err(unshifted) => { - self.try_remove_node(unshifted.node); + Err(FailedFeedPlan::Unshifted(unshifted)) => { + self.try_remove_node_subtree_recursive(unshifted.node); } + Err(FailedFeedPlan::Removed) => {} } } /// Feed one terminal with location to parser, and update state stack. @@ -1632,17 +1523,20 @@ impl< // Once the CFG admits this terminal, later failures are semantic/runtime branch // pruning and must not be reclassified as syntax recovery input. - if self - .apply_feed_plan( - plan_id, - node, - TerminalSymbol::Terminal(term.clone()), - location.clone(), - userdata, - ) - .is_ok() - { - accepted_branch_found = true; + match self.apply_feed_plan( + plan_id, + node, + TerminalSymbol::Terminal(term.clone()), + location.clone(), + userdata, + ) { + Ok(()) => { + accepted_branch_found = true; + } + Err(FailedFeedPlan::Unshifted(unshifted)) => { + self.try_remove_node_subtree_recursive(unshifted.node); + } + Err(FailedFeedPlan::Removed) => {} } } else { // Keep grammatical `NoAction` branches only as recovery candidates; a surviving @@ -1706,13 +1600,19 @@ impl< ) { // After shifting `error`, the original lookahead is a normal feed again: // it may need nullable reductions before the terminal shift is visible. - let _ = self.apply_feed_plan( + match self.apply_feed_plan( plan_id, error_node, TerminalSymbol::Terminal(term.clone()), location.clone(), userdata, - ); + ) { + Ok(()) => {} + Err(FailedFeedPlan::Unshifted(unshifted)) => { + self.try_remove_node_subtree_recursive(unshifted.node); + } + Err(FailedFeedPlan::Removed) => {} + } } else { // Otherwise the lookahead is part of the `error`; merge it so // diagnostics report the full discarded range. @@ -1767,7 +1667,7 @@ impl< .iter() .map(|branch| self.state(branch.node)) .collect(); - self.fallback_branches.clear(); + self.discard_fallback_branches(); Some(ParseError { term: TerminalSymbol::Terminal(term), location, @@ -1779,166 +1679,28 @@ impl< Ok(FeedSuccess { errors }) } } - /// Feed one terminal with location to parser, and update state stack. - fn can_feed_impl( + fn can_plan_feed( &self, extra_state_stack: &mut Vec, - mut node_and_len: Option<(NodeId, NonZeroUsize)>, + node_and_len: Option<(NodeId, NonZeroUsize)>, class: P::TermClass, ) -> bool { - let last_state = extra_state_stack - .last() - .copied() - .map(Index::into_usize) - .unwrap_or_else(|| { - node_and_len - .map(|(node, stack_len)| { - self.node(node).state_stack[stack_len.get() - 1].into_usize() - }) - .unwrap_or(0) - }); - let (shift_state, reduce) = match self.tables.term_action(last_state, class) { - Some(action) => (action.shift(), action.reduce()), - None => (None, None), - }; - if let Some(reduce_rules) = reduce { - let shift = shift_state; - - use crate::parser::table::ReduceRules; - - if shift.is_some() { - return true; - } - let mut reduces = reduce_rules.to_iter(); - match reduces.len() { - 0 => return false, // since reduce is `Some`, this should be unreachable - // if there is only one reduce rule, we can just reduce and shift with nonterminal - 1 => { - let rule_index = reduces.next().unwrap().into_usize(); - let rule_info = *self.tables.rule(rule_index); - let tokens_len = rule_info.len; - - // pop state stack - if tokens_len <= extra_state_stack.len() { - extra_state_stack.truncate(extra_state_stack.len() - tokens_len); - } else { - let left = tokens_len - extra_state_stack.len(); - extra_state_stack.clear(); - - let (node, len) = node_and_len.unwrap(); - let len = len.get(); - - if left < len { - node_and_len = Some((node, NonZeroUsize::new(len - left).unwrap())); - } else { - let parent = self.node(node).parent; - if let Some(parent) = parent { - node_and_len = self - .skip_last_n(parent, left - len) - .map(|(node, i)| (node, NonZeroUsize::new(i + 1).unwrap())); - } else { - // reached root node - node_and_len = None; - } - } - } - - // shift with reduced nonterminal - let last_state = extra_state_stack - .last() - .copied() - .map(Index::into_usize) - .unwrap_or_else(|| { - node_and_len - .map(|(node, stack_len)| { - self.node(node).state_stack[stack_len.get() - 1].into_usize() - }) - .unwrap_or(0) - }); - if let Some(next_state_id) = - self.tables.shift_goto_nonterm(last_state, rule_info.lhs) - { - extra_state_stack.push(next_state_id.state); - } else { - unreachable!( - "unreachable: nonterminal shift should always succeed after reduce operation. \ -Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a parser state machine bug.", - rule_info.lhs.as_str(), - rule_index - ); - } - - self.can_feed_impl(extra_state_stack, node_and_len, class) - } - // if there are multiple reduce rules, we need to check all of them, create new state stack for each reduce rule, and check if any of them can shift the terminal - _ => { - for reduce_rule in reduces { - let reduce_rule = *self.tables.rule(reduce_rule.into_usize()); - let tokens_len = reduce_rule.len; - - let mut extra_state_stack = extra_state_stack.clone(); - - // pop state stack - let new_node_and_len = if tokens_len <= extra_state_stack.len() { - extra_state_stack.truncate(extra_state_stack.len() - tokens_len); - node_and_len - } else { - let left = tokens_len - extra_state_stack.len(); - extra_state_stack.clear(); - - let (node, len) = node_and_len.unwrap(); - let len = len.get(); - - if left < len { - Some((node, NonZeroUsize::new(len - left).unwrap())) - } else { - let parent = self.node(node).parent; - if let Some(parent) = parent { - self.skip_last_n(parent, left - len) - .map(|(node, i)| (node, NonZeroUsize::new(i + 1).unwrap())) - } else { - // reached root node - None - } - } - }; - - // shift with reduced nonterminal - let last_state = extra_state_stack - .last() - .copied() - .map(Index::into_usize) - .unwrap_or_else(|| { - new_node_and_len - .map(|(node, stack_len)| { - self.node(node).state_stack[stack_len.get() - 1] - .into_usize() - }) - .unwrap_or(0) - }); - - if let Some(next_state_id) = - self.tables.shift_goto_nonterm(last_state, reduce_rule.lhs) - { - extra_state_stack.push(next_state_id.state); - } else { - unreachable!( - "unreachable: nonterminal shift should always succeed after reduce operation, but failed for nonterminal '{}' from state {:?}", - reduce_rule.lhs.as_str(), - last_state - ); - } - - if self.can_feed_impl(&mut extra_state_stack, new_node_and_len, class) { - return true; - } - } - false - } - } - } else { - shift_state.is_some() - } + let mut feed_plans = self.can_feed_plans.borrow_mut(); + let mut feed_plan_reductions = self.can_feed_plan_reductions.borrow_mut(); + let mut feed_plan_extra_state_stacks = self.can_feed_plan_extra_state_stacks.borrow_mut(); + + feed_plans.clear(); + feed_plan_reductions.clear(); + + self.plan_feed_impl_with_storage( + extra_state_stack, + node_and_len, + class, + &mut feed_plans, + &mut feed_plan_reductions, + &mut feed_plan_extra_state_stacks, + ) + .is_some() } /// Check if `term` can be feeded to current state. @@ -1953,7 +1715,7 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars extra_state_stack.clear(); let node_and_len = self.branch_node_and_len(node); - self.can_feed_impl(&mut extra_state_stack, node_and_len, class) + self.can_plan_feed(&mut extra_state_stack, node_and_len, class) }) } @@ -1964,9 +1726,9 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars return false; } - let mut extra_state_stack = Vec::new(); + let mut extra_state_stack = self.can_feed_extra_state_stack.borrow_mut(); - self.current_branches.iter().any(move |branch| { + self.current_branches.iter().any(|branch| { let mut node = branch.node; let mut len = self.node(node).len(); @@ -1974,7 +1736,7 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars loop { extra_state_stack.clear(); - if self.can_feed_impl( + if self.can_plan_feed( &mut extra_state_stack, Some((node, NonZeroUsize::new(len).unwrap())), P::TermClass::ERROR, @@ -2021,19 +1783,37 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars Data::Location::new(std::iter::empty(), 0) }; + let mut extra_state_stack = self.take_feed_plan_extra_state_stack(); while let Some(Branch { node, userdata }) = self.current_branches.pop() { let node_eof_location = Data::Location::new(self.location_iter(node), 0); - if let Err(unshifted) = self.feed_location_impl( - node, - TerminalSymbol::Eof, + + self.feed_plans.clear(); + self.feed_plan_reductions.clear(); + extra_state_stack.clear(); + + if let Some(plan_id) = self.plan_feed_impl( + &mut extra_state_stack, + self.branch_node_and_len(node), P::TermClass::EOF, - node_eof_location, - userdata, ) { - self.fallback_branches - .push(Branch::new(unshifted.node, unshifted.userdata)); + match self.apply_feed_plan( + plan_id, + node, + TerminalSymbol::Eof, + node_eof_location, + userdata, + ) { + Ok(()) => {} + Err(FailedFeedPlan::Unshifted(unshifted)) => { + self.try_remove_node_subtree_recursive(unshifted.node); + } + Err(FailedFeedPlan::Removed) => {} + } + } else { + self.fallback_branches.push(Branch::new(node, userdata)); } } + self.put_feed_plan_extra_state_stack(extra_state_stack); // next_branches is empty; invalid terminal was given // check for panic mode @@ -2048,14 +1828,15 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars states: self.states().collect(), }) } else { + self.discard_fallback_branches(); std::mem::swap(&mut self.current_branches, &mut self.next_branches); Ok(()) } } /// Check if current context can be terminated and get the start value. pub fn can_accept(&self) -> bool { - let mut extra_state_stack = Vec::new(); - self.current_branches.iter().any(move |branch| { + let mut extra_state_stack = self.can_feed_extra_state_stack.borrow_mut(); + self.current_branches.iter().any(|branch| { let node = branch.node; extra_state_stack.clear(); @@ -2068,7 +1849,7 @@ Failed to shift nonterminal '{}' after reducing rule '{}'. This indicates a pars Some((node, NonZeroUsize::new(node_.len()).unwrap())) } }; - self.can_feed_impl(&mut extra_state_stack, node_and_len, P::TermClass::EOF) + self.can_plan_feed(&mut extra_state_stack, node_and_len, P::TermClass::EOF) }) } } @@ -2111,6 +1892,9 @@ where feed_plan_reductions: Default::default(), feed_plan_extra_state_stacks: Default::default(), can_feed_extra_state_stack: Default::default(), + can_feed_plans: Default::default(), + can_feed_plan_reductions: Default::default(), + can_feed_plan_extra_state_stacks: Default::default(), tables: self.tables, _phantom: std::marker::PhantomData, }