Skip to content

Optimization for unused DataStack tags #40

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ehwan merged 7 commits into from
Oct 3, 2025
Merged

Optimization for unused DataStack tags #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fac8fb9
do not push any data for eof input
ehwan Oct 2, 2025
a842fca
do not emit terminal stack if it was not used
ehwan Oct 2, 2025
91b1ae1
do not emit empty tag if it was not used
ehwan Oct 3, 2025
1b53b83
do not emit tag stack if single tag was used
ehwan Oct 3, 2025
388b54b
bootstrap
ehwan Oct 3, 2025
b6811e0
fix for unique_tag not considered for push_terminal
ehwan Oct 3, 2025
4e82b28
fixed empty_tag determine statements
ehwan Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions rusty_lr_core/src/parser/deterministic/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ impl<Data: DataStack, StateIndex: Index + Copy> Context<Data, StateIndex> {
{
self.feed_eof(parser, userdata)?;

// data_stack must be <Start> <EOF> in this point
self.data_stack.pop(); // pop eof
// data_stack must be <Start> in this point
Ok(self.data_stack.pop_start().unwrap())
}

Expand Down Expand Up @@ -514,10 +513,14 @@ impl<Data: DataStack, StateIndex: Index + Copy> Context<Data, StateIndex> {
if next_state_id.push {
match term {
TerminalSymbol::Term(t) => self.data_stack.push_terminal(t),
TerminalSymbol::Error | TerminalSymbol::Eof => self.data_stack.push_empty(),
TerminalSymbol::Error => self.data_stack.push_empty(),
TerminalSymbol::Eof => {} // do not push anything for eof
}
} else {
self.data_stack.push_empty();
match term {
TerminalSymbol::Term(_) | TerminalSymbol::Error => self.data_stack.push_empty(),
TerminalSymbol::Eof => {} // do not push anything for eof
}
}
Comment on lines 513 to 524
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for handling term can be simplified to reduce code duplication and improve readability. The current implementation has separate match statements for the if next_state_id.push and else branches, with similar handling for TerminalSymbol::Eof.

Suggested change
if next_state_id.push {
match term {
TerminalSymbol::Term(t) => self.data_stack.push_terminal(t),
TerminalSymbol::Error | TerminalSymbol::Eof => self.data_stack.push_empty(),
TerminalSymbol::Error => self.data_stack.push_empty(),
TerminalSymbol::Eof => {} // do not push anything for eof
}
} else {
self.data_stack.push_empty();
match term {
TerminalSymbol::Term(_) | TerminalSymbol::Error => self.data_stack.push_empty(),
TerminalSymbol::Eof => {} // do not push anything for eof
}
}
match term {
TerminalSymbol::Eof => {} // do not push anything for eof
TerminalSymbol::Term(t) => {
if next_state_id.push {
self.data_stack.push_terminal(t);
} else {
self.data_stack.push_empty();
}
}
TerminalSymbol::Error => {
self.data_stack.push_empty();
}
}


// location is `Some` if it is not `Eof`
Expand Down
21 changes: 16 additions & 5 deletions rusty_lr_core/src/parser/nondeterministic/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,6 @@ impl<Data: DataStack, StateIndex: Index, const MAX_REDUCE_RULES: usize>
let nodes = std::mem::take(&mut self.current_nodes);
Ok(nodes.into_iter().map(move |node| {
// let node = self.pop(eof_node).unwrap();
self.nodes_pool[node].data_stack.pop(); // pop eof
self.nodes_pool[node].data_stack.pop_start().unwrap()
}))
}
Expand Down Expand Up @@ -1171,12 +1170,18 @@ impl<Data: DataStack, StateIndex: Index, const MAX_REDUCE_RULES: usize>
TerminalSymbol::Term(term) => {
node_.data_stack.push_terminal(term);
}
TerminalSymbol::Eof | TerminalSymbol::Error => {
TerminalSymbol::Error => {
node_.data_stack.push_empty();
}
TerminalSymbol::Eof => {} // do not push for eof
}
} else {
node_.data_stack.push_empty();
match term {
TerminalSymbol::Term(_) | TerminalSymbol::Error => {
node_.data_stack.push_empty();
}
TerminalSymbol::Eof => {} // do not push for eof
}
}

self.next_nodes.push(node);
Expand Down Expand Up @@ -1205,12 +1210,18 @@ impl<Data: DataStack, StateIndex: Index, const MAX_REDUCE_RULES: usize>
TerminalSymbol::Term(term) => {
node_.data_stack.push_terminal(term);
}
TerminalSymbol::Eof | TerminalSymbol::Error => {
TerminalSymbol::Error => {
node_.data_stack.push_empty();
}
TerminalSymbol::Eof => {} // no push for eof
}
} else {
node_.data_stack.push_empty();
match term {
TerminalSymbol::Term(_) | TerminalSymbol::Error => {
node_.data_stack.push_empty();
}
TerminalSymbol::Eof => {} // no push for eof
}
}

self.next_nodes.push(node);
Comment on lines 1210 to 1227
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This logic for handling TerminalSymbol variants is repeated from an earlier part of the file and can be simplified to improve code clarity and reduce duplication.

Suggested change
TerminalSymbol::Term(term) => {
node_.data_stack.push_terminal(term);
}
TerminalSymbol::Eof | TerminalSymbol::Error => {
TerminalSymbol::Error => {
node_.data_stack.push_empty();
}
TerminalSymbol::Eof => {} // no push for eof
}
} else {
node_.data_stack.push_empty();
match term {
TerminalSymbol::Term(_) | TerminalSymbol::Error => {
node_.data_stack.push_empty();
}
TerminalSymbol::Eof => {} // no push for eof
}
}
self.next_nodes.push(node);
match term {
TerminalSymbol::Eof => {} // no push for eof
TerminalSymbol::Term(term) => {
if shift.push {
node_.data_stack.push_terminal(term);
} else {
node_.data_stack.push_empty();
}
}
TerminalSymbol::Error => {
node_.data_stack.push_empty();
}
}

Expand Down
Loading