diff --git a/lalrpop-test/src/issue_55.lalrpop b/lalrpop-test/src/issue_55.lalrpop new file mode 100644 index 0000000..56034fd --- /dev/null +++ b/lalrpop-test/src/issue_55.lalrpop @@ -0,0 +1,13 @@ +grammar; + +pub E: (Vec<&'input str>, &'input str, Vec<&'input str>) = { + "{" "}" +}; + +AT: &'input str = { + "type" ";" +}; + +ET: &'input str = { + "enum" "{" "}" +}; diff --git a/lalrpop-test/src/main.rs b/lalrpop-test/src/main.rs index a8e130f..7624a8d 100644 --- a/lalrpop-test/src/main.rs +++ b/lalrpop-test/src/main.rs @@ -46,6 +46,9 @@ mod use_super; /// test that exercises locations and spans mod error; +/// test for inlining expansion issue #55 +mod issue_55; + mod util; /// This constant is here so that some of the generator parsers can @@ -236,3 +239,20 @@ fn error_test1() { } } +#[test] +fn issue_55_test1() { + // Issue 55 caused us to either accept NO assoc types or assoc + // types both before and after, so check that we can parse with + // assoc types on either side. + + let (a, b, c) = issue_55::parse_E("{ type X; type Y; enum Z { } }").unwrap(); + assert_eq!(a, vec!["X", "Y"]); + assert_eq!(b, "Z"); + assert!(c.is_empty()); + + let (a, b, c) = issue_55::parse_E("{ enum Z { } type X; type Y; }").unwrap(); + assert!(a.is_empty()); + assert_eq!(b, "Z"); + assert_eq!(c, vec!["X", "Y"]); +} + diff --git a/lalrpop/src/normalize/inline/mod.rs b/lalrpop/src/normalize/inline/mod.rs index c103406..2fe5c76 100644 --- a/lalrpop/src/normalize/inline/mod.rs +++ b/lalrpop/src/normalize/inline/mod.rs @@ -30,18 +30,18 @@ fn inline_nt(grammar: &mut Grammar, inline_nt: NonterminalString) { continue; } - for inline_production in &inline_productions { - let mut inliner = Inliner { - action_fn_defns: &grammar.action_fn_defns, - inline_production: inline_production, - into_production: into_production, - new_symbols: vec![], - new_productions: &mut new_productions, - new_action_fn_defns: &mut new_action_fn_defns, - }; + let mut inliner = Inliner { + action_fn_defns: &grammar.action_fn_defns, + inline_nonterminal: inline_nt, + into_production: into_production, + inline_fallible: 0, + inline_productions: &inline_productions, + new_symbols: vec![], + new_productions: &mut new_productions, + new_action_fn_defns: &mut new_action_fn_defns, + }; - inliner.inline(&into_production.symbols); - } + inliner.inline(&into_production.symbols); } data.productions = new_productions; @@ -53,8 +53,19 @@ struct Inliner<'a> { /// Action fn defns action_fn_defns: &'a [ActionFnDefn], - /// The production `A = B C D` being inlined - inline_production: &'a Production, + /// The nonterminal `A` being inlined + inline_nonterminal: NonterminalString, + + /// The full set of productions `A = B C D | E F G` for the + /// nonterminal `A` being inlined + inline_productions: &'a [Production], + + /// Number of actions that we have inlined for `A` so far which + /// have been fallible. IOW, if we are inlining `A` into `X = Y A + /// A Z`, and in the first instance of `A` we used a fallible + /// action, but the second we used an infallible one, count would + /// be 1. + inline_fallible: u32, /// The `X = Y A Z` being inlined into into_production: &'a Production, @@ -73,15 +84,12 @@ struct Inliner<'a> { impl<'a> Inliner<'a> { fn inline(&mut self, into_symbols: &[Symbol]) { - let new_symbols_len = self.new_symbols.len(); - if into_symbols.is_empty() { // create an action fn for the result of inlining let into_action = self.into_production.action; - let inline_action = self.inline_production.action; let into_fallible = self.action_fn_defns[into_action.index()].fallible; let into_ret_type = self.action_fn_defns[into_action.index()].ret_type.clone(); - let inline_fallible = self.action_fn_defns[inline_action.index()].fallible; + let inline_fallible = self.inline_fallible != 0; let index = self.action_fn_defns.len() + self.new_action_fn_defns.len(); let action_fn = ActionFn::new(index); let inline_defn = InlineActionFnDefn { @@ -109,19 +117,36 @@ impl<'a> Inliner<'a> { } else { let next_symbol = into_symbols[0]; match next_symbol { - Symbol::Nonterminal(n) if n == self.inline_production.nonterminal => { - self.new_symbols.push( - InlinedSymbol::Inlined( - self.inline_production.action, - self.inline_production.symbols.clone())); + Symbol::Nonterminal(n) if n == self.inline_nonterminal => { + // Replace the current symbol with each of the + // `inline_productions` in turn. + for inline_production in self.inline_productions { + // If this production is fallible, increment + // count of fallible actions. + let inline_action = inline_production.action; + let fallible = self.action_fn_defns[inline_action.index()].fallible; + self.inline_fallible += fallible as u32; + + // Push the symbols of the production inline. + self.new_symbols.push( + InlinedSymbol::Inlined( + inline_production.action, + inline_production.symbols.clone())); + + // Inline remaining symbols: + self.inline(&into_symbols[1..]); + + // Reset state after we have inlined remaining symbols: + self.new_symbols.pop(); + self.inline_fallible -= fallible as u32; + } } _ => { self.new_symbols.push(InlinedSymbol::Original(next_symbol)); + self.inline(&into_symbols[1..]); + self.new_symbols.pop(); } } - self.inline(&into_symbols[1..]); } - - self.new_symbols.truncate(new_symbols_len); } } diff --git a/lalrpop/src/normalize/inline/test.rs b/lalrpop/src/normalize/inline/test.rs index fe8c928..9a87bd6 100644 --- a/lalrpop/src/normalize/inline/test.rs +++ b/lalrpop/src/normalize/inline/test.rs @@ -67,3 +67,28 @@ fn sri() { assert_eq!(format!("{:?}", e_productions[1].symbols), r#"["&", E]"#); assert_eq!(format!("{:?}", e_productions[2].symbols), r#"["&", "L", E]"#); } + +#[test] +fn issue_55() { + let grammar = inlined_grammar(r#" +grammar; + +pub E: () = { + "X" "{" "}" => () +}; + +AT: () = { + "type" ";" => () +}; + +ET: () = { + "enum" "{" "}" => () +}; + "#).unwrap(); + let nt = NonterminalString(intern("E")); + + // The problem in issue #55 was that we would inline both `AT*` + // the same way, so we ended up with `E = X { ET }` and `E = X { + // AT+ ET AT+ }` but not `E = X { AT+ ET }` or `E = X { ET AT+ }`. + assert!(grammar.productions_for(nt).len() == 4); +}