Fix a bug of unnecessarily cloning nodes.

* chain/src/item/default/splone.rs: Previously when we split nodes, we
  always clone the parent if the labels differ.  This turns out to be
  incorrect if the new label is open whereas the old label is closed.
  In that case, the old parent should not contain the new node as a
  child, as a closed node should not contain an open node.

  I am not yet entirely sure this fix is correct, so more test await
  us.

1 files changed, 23 insertions, 3 deletions

diff --git a/chain/src/item/default/splone.rs b/chain/src/item/default/splone.rs
index 09fca0b..c73bd9f 100644
--- a/chain/src/item/default/splone.rs
+++ b/chain/src/item/default/splone.rs
@@ -290,7 +290,8 @@ impl DefaultForest<ForestLabel<GrammarLabel>> {
     ///
     /// 2. Preserve the old children as specified by `edge_index + 1`.
     ///
-    /// 3. For each parent, clone the parent.  Replace the original
+    /// 3. If the new node is closed or if the new node is not cloned,
+    /// then for each parent, clone the parent.  Replace the original
     /// child of the parent, which pointed to the old node, by this
     /// new node.  Other children are inherited from the old parent.
     ///
@@ -305,15 +306,16 @@ impl DefaultForest<ForestLabel<GrammarLabel>> {
         completingp: bool,
     ) -> Result<usize, Error> {
         let end = new_label.label().end();
+        let new_node_is_clone = new_label.clone_index().is_some();
 
         let mut builder = PLGBuilderMut::from_graph_mut(&mut self.graph);
 
         let new_node = builder.add_vertex(new_label);
 
-        let new_packed = if new_label.clone_index().is_some() {
+        let new_packed = if new_node_is_clone {
             let packed = builder
                 .query_label(ForestLabel::new(new_label.label(), ForestLabelType::Packed))
-                .unwrap();
+                .unwrap_or_else(|| panic!("A cloned node {new_node} without packed nodes"));
 
             builder.add_edge(packed, new_node, new_label)?;
 
@@ -328,6 +330,24 @@ impl DefaultForest<ForestLabel<GrammarLabel>> {
             builder.add_edge(new_node, child, new_label)?;
         }
 
+        // NOTE: If the label of the parent is closed while the node
+        // label is open ended, we need to use an opened parent
+        // instead.
+        //
+        // To be specific, we only need to split the parent if the new
+        // label is closed.  This can be seen from the fact that this
+        // function only deals with the situation that the new label
+        // and the old label are different.  In this situation, if the
+        // new label is closed, it means we are closing a node under
+        // an open node, and we need to clone that parent indeed.  On
+        // the other hand, if we are opening a node under a closed
+        // node, that parent does not need to connect to this new
+        // node.
+
+        if end.is_none() && new_node_is_clone {
+            return Ok(new_node);
+        }
+
         let parents: Vec<_> = {
             if let Some(label) = self.vertex_label(node)? {
                 if label.clone_index().is_some() {