Fix inlining so that different appearances of the same symbol can be

inlined in multiple ways.

lalrpop-test/src/issue_55.lalrpop

@@ -0,0 +1,13 @@
+grammar;
+
+pub E: (Vec<&'input str>, &'input str, Vec<&'input str>) = {
+    "{" <AT*> <ET> <AT*> "}"
+};
+
+AT: &'input str = {
+    "type" <r"[a-zA-Z]+"> ";"
+};
+
+ET: &'input str = {
+    "enum" <r"[a-zA-Z]+"> "{" "}"
+};

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"]);
+}
+

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);
     }
 }

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" "{" <a:AT*> <e:ET> <b:AT*> "}" => ()
+};
+
+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);
+}