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
…#898)

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 24, 2024
1 parent a27c96c commit 8536144
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 8536144

Please sign in to comment.