Fix join_into overwriting a destination that holds the source

src/dense_byte_node.rs

@@ -1335,8 +1335,12 @@ impl<V: Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> TrieNode<V, A>
                 self.pjoin(other_byte_node).map(|new_node| TrieNodeODRc::new_in(new_node, self.alloc.clone()))
             },
             TINY_REF_NODE_TAG => {
+                //Expand the tiny node and keep `self` on the left, so the identity mask stays ours
                 let tiny_node = unsafe{ other.as_tiny_unchecked() };
-                tiny_node.pjoin_dyn(self.as_tagged())
+                match tiny_node.into_full() {
+                    Some(full_node) => self.pjoin_dyn(full_node.as_tagged()),
+                    None => AlgebraicResult::Identity(SELF_IDENT),
+                }
             }
             EMPTY_NODE_TAG => {
                 AlgebraicResult::Identity(SELF_IDENT)

src/line_list_node.rs

@@ -1345,14 +1345,13 @@ fn merge_guts<'a, V: Clone + Lattice + Send + Sync, A: Allocator, const ASLOT: u
         unsafe{ intermediate_node.set_payload_owned::<0>(&a_key[overlap..], a_payload); }
         debug_assert!(validate_node(&intermediate_node));
         let intermediate_node = TrieNodeODRc::new_in(intermediate_node, a.alloc.clone());
-        let joined = b_child.pjoin(&intermediate_node).unwrap_or_else(|which_arg| {
-            match which_arg {
-                0 => b_child.clone(),
-                1 => intermediate_node,
-                _ => unreachable!()
-            }
-        }, || panic!());
-        return AlgebraicResult::Element((&a_key[0..overlap], ValOrChild::Child(joined)))
+        return match b_child.pjoin(&intermediate_node) {
+            AlgebraicResult::Element(joined) => AlgebraicResult::Element((&a_key[0..overlap], ValOrChild::Child(joined))),
+            //`b`'s child already held `a`'s payload, so `b`'s slot is the result
+            AlgebraicResult::Identity(mask) if mask & SELF_IDENT > 0 => AlgebraicResult::Identity(COUNTER_IDENT),
+            AlgebraicResult::Identity(_) => AlgebraicResult::Element((&a_key[0..overlap], ValOrChild::Child(intermediate_node))),
+            AlgebraicResult::None => unreachable!(), //`intermediate_node` is never empty
+        }
     }
     if a_key_len == overlap && a.is_child_ptr::<ASLOT>() && b_key_len > overlap {
         let a_child = unsafe{ a.child_in_slot::<ASLOT>() };
@@ -1361,14 +1360,13 @@ fn merge_guts<'a, V: Clone + Lattice + Send + Sync, A: Allocator, const ASLOT: u
         unsafe{ intermediate_node.set_payload_owned::<0>(&b_key[overlap..], b_payload); }
         debug_assert!(validate_node(&intermediate_node));
         let intermediate_node = TrieNodeODRc::new_in(intermediate_node, a.alloc.clone());
-        let joined = a_child.pjoin(&intermediate_node).unwrap_or_else(|which_arg| {
-            match which_arg {
-                0 => a_child.clone(),
-                1 => intermediate_node,
-                _ => unreachable!()
-            }
-        }, || panic!());
-        return AlgebraicResult::Element((&a_key[0..overlap], ValOrChild::Child(joined)))
+        return match a_child.pjoin(&intermediate_node) {
+            AlgebraicResult::Element(joined) => AlgebraicResult::Element((&a_key[0..overlap], ValOrChild::Child(joined))),
+            //Mirror of the case above: `a`'s slot is the result
+            AlgebraicResult::Identity(mask) if mask & SELF_IDENT > 0 => AlgebraicResult::Identity(SELF_IDENT),
+            AlgebraicResult::Identity(_) => AlgebraicResult::Element((&a_key[0..overlap], ValOrChild::Child(intermediate_node))),
+            AlgebraicResult::None => unreachable!(), //`intermediate_node` is never empty
+        }
     }
 
     //If we have overlapping initial bytes that can be joined together, make a new prefix node
@@ -2652,8 +2650,12 @@ impl<V: Clone + Send + Sync, A: Allocator> TrieNode<V, A> for LineListNode<V, A>
                 }
             },
             TINY_REF_NODE_TAG => {
+                //Expand the tiny node and keep `self` on the left (see DenseByteNode::pjoin_dyn)
                 let tiny_node = unsafe{ other.as_tiny_unchecked() };
-                tiny_node.pjoin_dyn(self.as_tagged())
+                match tiny_node.into_full() {
+                    Some(full_node) => self.pjoin_dyn(full_node.as_tagged()),
+                    None => AlgebraicResult::Identity(SELF_IDENT),
+                }
             }
             EMPTY_NODE_TAG => {
                 AlgebraicResult::Identity(SELF_IDENT)

src/ring.rs

@@ -851,15 +851,21 @@ impl Lattice for () {
     fn pmeet(&self, _other: &Self) -> AlgebraicResult<Self> { AlgebraicResult::Identity(SELF_IDENT | COUNTER_IDENT) }
 }
 
+/// Left-biased join; equal values are also `other`'s identity
+#[inline]
+fn left_biased_pjoin<T: PartialEq>(a: &T, b: &T) -> AlgebraicResult<T> {
+    if a == b { AlgebraicResult::Identity(SELF_IDENT | COUNTER_IDENT) } else { AlgebraicResult::Identity(SELF_IDENT) }
+}
+
 //GOAT trash
 impl Lattice for usize {
-    fn pjoin(&self, _other: &usize) -> AlgebraicResult<usize> { AlgebraicResult::Identity(SELF_IDENT) }
+    fn pjoin(&self, other: &usize) -> AlgebraicResult<usize> { left_biased_pjoin(self, other) }
     fn pmeet(&self, _other: &usize) -> AlgebraicResult<usize> { AlgebraicResult::Identity(SELF_IDENT) }
 }
 
 //GOAT trash
 impl Lattice for u64 {
-    fn pjoin(&self, _other: &u64) -> AlgebraicResult<u64> { AlgebraicResult::Identity(SELF_IDENT) }
+    fn pjoin(&self, other: &u64) -> AlgebraicResult<u64> { left_biased_pjoin(self, other) }
     fn pmeet(&self, _other: &u64) -> AlgebraicResult<u64> { AlgebraicResult::Identity(SELF_IDENT) }
 }
 
@@ -873,13 +879,13 @@ impl DistributiveLattice for u64 {
 
 //GOAT trash
 impl Lattice for u32 {
-    fn pjoin(&self, _other: &u32) -> AlgebraicResult<u32> { AlgebraicResult::Identity(SELF_IDENT) }
+    fn pjoin(&self, other: &u32) -> AlgebraicResult<u32> { left_biased_pjoin(self, other) }
     fn pmeet(&self, _other: &u32) -> AlgebraicResult<u32> { AlgebraicResult::Identity(SELF_IDENT) }
 }
 
 //GOAT trash
 impl Lattice for u16 {
-    fn pjoin(&self, _other: &u16) -> AlgebraicResult<u16> { AlgebraicResult::Identity(SELF_IDENT) }
+    fn pjoin(&self, other: &u16) -> AlgebraicResult<u16> { left_biased_pjoin(self, other) }
     fn pmeet(&self, _other: &u16) -> AlgebraicResult<u16> { AlgebraicResult::Identity(SELF_IDENT) }
 }
 
@@ -893,7 +899,7 @@ impl DistributiveLattice for u16 {
 
 //GOAT trash
 impl Lattice for u8 {
-    fn pjoin(&self, _other: &u8) -> AlgebraicResult<u8> { AlgebraicResult::Identity(SELF_IDENT) }
+    fn pjoin(&self, other: &u8) -> AlgebraicResult<u8> { left_biased_pjoin(self, other) }
     fn pmeet(&self, _other: &u8) -> AlgebraicResult<u8> { AlgebraicResult::Identity(SELF_IDENT) }
 }
 

src/write_zipper.rs

@@ -6753,6 +6753,54 @@ mod tests {
         assert_eq!(keys(&m), ["cx", "cy", "d"]);
     }
 
+    /// `join_into` from a source focus partway into a line node (a `TinyRefNode`)
+    #[test]
+    fn write_zipper_join_into_mid_key_source_keeps_destination() {
+        fn mk(ps: &[(&[u8], u64)]) -> PathMap<u64> { let mut m = PathMap::new(); for (p, v) in ps { m.set_val_at(p, *v); } m }
+        fn vals(m: &PathMap<u64>) -> Vec<(Vec<u8>, u64)> { m.iter().map(|(k, v)| (k.to_vec(), *v)).collect() }
+        let src = mk(&[(&[0, 0, 0], 7)]);
+
+        //Dense destination
+        let mut dst = mk(&[(&[0], 7), (&[1], 1), (&[2], 2), (&[3], 3)]);
+        let before = vals(&dst);
+        let st = { let mut wz = dst.write_zipper(); let mut rz = src.read_zipper(); rz.descend_to(&[0, 0]); wz.join_into(&rz) };
+        assert_eq!(st, AlgebraicStatus::Identity);
+        assert_eq!(vals(&dst), before);
+
+        //List destination
+        let mut dst = mk(&[(&[0], 7), (&[0, 0], 0)]);
+        let before = vals(&dst);
+        let st = { let mut wz = dst.write_zipper(); let mut rz = src.read_zipper(); rz.descend_to(&[0, 0]); wz.join_into(&rz) };
+        assert_eq!(st, AlgebraicStatus::Identity);
+        assert_eq!(vals(&dst), before);
+
+        //And a join that does add something still says so, with the destination intact
+        let mut dst = mk(&[(&[1], 1), (&[2], 2), (&[3], 3)]);
+        let st = { let mut wz = dst.write_zipper(); let mut rz = src.read_zipper(); rz.descend_to(&[0, 0]); wz.join_into(&rz) };
+        assert_eq!(st, AlgebraicStatus::Element);
+        assert_eq!(vals(&dst), vec![(vec![0], 7), (vec![1], 1), (vec![2], 2), (vec![3], 3)]);
+    }
+
+    /// `join_into` of a source already contained under a destination child is `Identity`
+    #[test]
+    fn write_zipper_join_into_contained_under_child_is_identity() {
+        fn mk(ps: &[(&[u8], u64)]) -> PathMap<u64> { let mut m = PathMap::new(); for (p, v) in ps { m.set_val_at(p, *v); } m }
+        fn vals(m: &PathMap<u64>) -> Vec<(Vec<u8>, u64)> { m.iter().map(|(k, v)| (k.to_vec(), *v)).collect() }
+        let mut dst = mk(&[(&[0, 0], 0), (&[0, 1], 0)]);
+        let before = vals(&dst);
+        let src = mk(&[(&[0, 0], 0)]);
+        let st = { let mut wz = dst.write_zipper(); wz.join_into(&src.read_zipper()) };
+        assert_eq!(st, AlgebraicStatus::Identity);
+        assert_eq!(vals(&dst), before);
+
+        //The mirror image: the source holds the child, the destination the longer key
+        let mut dst = mk(&[(&[0, 0], 0)]);
+        let src = mk(&[(&[0, 0], 0), (&[0, 1], 0)]);
+        let st = { let mut wz = dst.write_zipper(); wz.join_into(&src.read_zipper()) };
+        assert_eq!(st, AlgebraicStatus::Element);
+        assert_eq!(vals(&dst), vals(&src));
+    }
+
     /// Dense `restrict` is `Identity` even when the restrictor has extra branches
     #[test]
     fn write_zipper_restrict_wider_restrictor_is_identity() {