Optimized out unused states and production rules#27
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the state machine by identifying and eliminating single-token reduce states and unused production rules. The optimization removes states that only perform a single reduction of a single-token rule (A -> a), redirecting transitions to bypass these states.
- Added
is_usedfield to production rules to track usage after optimization - Implemented state machine optimization algorithm that bypasses single-token reduce states
- Updated parser state structures to support
ShiftTargetwith push/no-push semantics - Modified reduce action signatures to return boolean indicating whether data was pushed
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| rusty_lr_parser/src/pattern.rs | Adds is_used: true initialization to all newly created rules |
| rusty_lr_parser/src/nonterminal_info.rs | Adds is_used field to Rule struct for optimization tracking |
| rusty_lr_parser/src/grammar.rs | Implements main optimization algorithm and unused rule detection |
| rusty_lr_parser/src/emit.rs | Updates code generation to handle optimized states and unused rules |
| rusty_lr_core/src/parser/state.rs | Adds ShiftTarget struct and updates state interfaces |
| rusty_lr_core/src/parser/nondeterministic/context.rs | Updates shift operations to use ShiftTarget |
| rusty_lr_core/src/parser/deterministic/context.rs | Updates shift operations to use ShiftTarget |
| rusty_lr_core/src/parser/data_stack.rs | Changes reduce action return type to Result<bool, _> |
| rusty_lr_core/src/builder/state.rs | Updates state conversion to use ShiftTarget |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| let mut states: Vec< | ||
| rusty_lr_core::parser::state::IntermediateState<usize, usize, usize, usize>, | ||
| > = states.into_iter().map(Into::into).collect(); |
There was a problem hiding this comment.
[nitpick] The type annotation on line 1562-1564 is unnecessarily verbose and could be simplified using type inference, making the code more readable.
| let mut states: Vec< | |
| rusty_lr_core::parser::state::IntermediateState<usize, usize, usize, usize>, | |
| > = states.into_iter().map(Into::into).collect(); | |
| let mut states = states.into_iter().map(Into::into).collect(); |
| reduce_action_case_streams.extend(quote! { | ||
| #rule_index => unreachable!("{rule_index}: this production rule was optimized out"), |
There was a problem hiding this comment.
The error message uses string interpolation syntax {rule_index} but should use {rule_index} within the unreachable! macro. This will not interpolate the variable correctly.
| reduce_action_case_streams.extend(quote! { | |
| #rule_index => unreachable!("{rule_index}: this production rule was optimized out"), | |
| #rule_index => unreachable!("{}: this production rule was optimized out", #rule_index), |
| reduce_action_case_streams.extend(quote! { | ||
| #rule_index => unreachable!("{rule_index}: this production rule was optimized out"), |
There was a problem hiding this comment.
Same issue as above - the error message uses string interpolation syntax incorrectly within the unreachable! macro.
| reduce_action_case_streams.extend(quote! { | |
| #rule_index => unreachable!("{rule_index}: this production rule was optimized out"), | |
| #rule_index => unreachable!("{}: this production rule was optimized out", #rule_index), |
| reduce_action_case_streams.extend(quote! { | ||
| #rule_index => unreachable!("{rule_index}: this production rule was optimized out"), |
There was a problem hiding this comment.
Same issue as above - the error message uses string interpolation syntax incorrectly within the unreachable! macro.
| reduce_action_case_streams.extend(quote! { | |
| #rule_index => unreachable!("{rule_index}: this production rule was optimized out"), | |
| #rule_index => unreachable!("{}: this production rule was optimized out", #rule_index), |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Optimizes the state machine for single-token-reduce-state:
delete and redirect all of the state that has only single reduce action, and that reduce rule has single token
A -> a.This could reduce runtime by 30%