Skip to content

Commit

Permalink
Insert an empty lookahead in the lane table in reduce conflict states
Browse files Browse the repository at this point in the history
We need to make sure the state in the conflict is in the lane table.
All the existing test cases happened to have a shift case in the
conflict state, even if the shift wasn't part of the actual conflict.
In the event of a conflicting state with only reduce actions, we'll need
the lookahead store, so we can check it as a successor of the
predecessors we found.

fixes #897
  • Loading branch information
dburgener committed May 17, 2024
1 parent 1eb87a0 commit 7bc39d3
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 1 deletion.
1 change: 1 addition & 0 deletions lalrpop/src/lr1/lane_table/construct/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ impl<'m> ContextSets<'m> {
}

fn union(&mut self, source: StateIndex, target: StateIndex) -> bool {
debug!("state_sets: {:?}", self.state_sets);
let set1 = self.state_sets[&source];
let set2 = self.state_sets[&target];
let result = self.unify.unify_var_var(set1, set2).is_ok();
Expand Down
2 changes: 2 additions & 0 deletions lalrpop/src/lr1/lane_table/lane/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl<'trace, 'grammar, L: Lookahead> LaneTracer<'trace, 'grammar, L> {

Action::Reduce(prod) => {
let item = Item::lr0(prod, prod.symbols.len());
self.table.add_lookahead(state, conflict, &TokenSet::new());
self.continue_trace(state, conflict, item, &mut visited_set);
}
}
Expand All @@ -72,6 +73,7 @@ impl<'trace, 'grammar, L: Lookahead> LaneTracer<'trace, 'grammar, L> {
item: Lr0Item<'grammar>,
visited: &mut Set<(StateIndex, Lr0Item<'grammar>)>,
) {
debug!("continue_trace: state={:?}, index={:?}", state, item.index);
if !visited.insert((state, item)) {
return;
}
Expand Down
47 changes: 46 additions & 1 deletion lalrpop/src/lr1/lane_table/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,30 @@ Y: () = {
)
}

/// A variation on G1 to omit the possibility of shifting
pub fn example_g2() -> Grammar {
normalized_grammar(
r#"
grammar;
pub G = {
"a" X "d",
"a" Y "c",
"b" X "c",
"b" Y "d",
};
X = {
"e"
};
Y = {
"e"
};
"#,
)
}

fn build_table<'grammar>(
grammar: &'grammar Grammar,
goal: &str,
Expand Down Expand Up @@ -294,6 +318,27 @@ P: () = {
)
}

// The G1 example has a non-conflicting shift in the state with the reduce/reduce conflict. This
// test exercises the case where the reduce/reduce is the only difference.
#[test]
fn example_g2_build() {
let _tls = Tls::test();
let grammar = example_g2();

let _lr1_tls = Lr1Tls::install(grammar.terminals.clone());
let lr0_err = build::build_lr0_states(&grammar, nt("__G")).unwrap_err();
let states = build_states(&grammar, nt("__G")).expect("failed to build lane table states");

// we require more *states* than LR(0), not just different lookahead
assert_eq!(states.len() - lr0_err.states.len(), 1);

let tree = interpret::interpret(&states, tokens!["a", "e", "d"]).unwrap();
expect_debug(&tree, r#"[__G: [G: "a", [X: "e"], "d"]]"#);

let tree = interpret::interpret(&states, tokens!["b", "e", "d"]).unwrap();
expect_debug(&tree, r#"[__G: [G: "b", [Y: "e"], "d"]]"#);
}

#[test]
fn large_conflict_1() {
let _tls = Tls::test();
Expand All @@ -320,7 +365,7 @@ fn large_conflict_1() {
| S5 | | | ["a"] | ["r"] | {S16} |
| S7 | | | ["c", "w"] | ["d"] | {S16} |
| S16 | | | | | {S27} |
| S27 | ["s"] | ["k"] | | | {S32} |
| S27 | ["s"] | ["k"] | [] | [] | {S32} |
| S32 | | | ["z"] | ["u"] | {S16} |
"#
.trim_start(),
Expand Down

0 comments on commit 7bc39d3

Please sign in to comment.