Keep a value in subtract when the other side only passes through

src/dense_byte_node.rs

@@ -418,10 +418,11 @@ impl<V: Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> ByteNode<Cf, A>
                 let cf = unsafe{ self_node.values.get_unchecked(cf_idx) };
                 let mut new_cf = Cf::new(None, None);
 
-                //If there is a value at this key_byte, and the other node contains a value, subtract them
+                //If there is a value at this key_byte, and the other node contains a value, subtract them;
+                // otherwise keep it
                 if let Some(self_val) = cf.val() {
-                    if let Some(other_val) = other.node_get_val(&[key_byte]) {
-                        match self_val.psubtract(other_val) {
+                    match other.node_get_val(&[key_byte]) {
+                        Some(other_val) => match self_val.psubtract(other_val) {
                             AlgebraicResult::None => { is_identity = false; },
                             AlgebraicResult::Identity(mask) => {
                                 debug_assert_eq!(mask, SELF_IDENT); //subtract is not commutative
@@ -431,7 +432,8 @@ impl<V: Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> ByteNode<Cf, A>
                                 is_identity = false;
                                 new_cf.set_val(e);
                             },
-                        }
+                        },
+                        None => new_cf.set_val(self_val.clone()),
                     }
                 }
 
@@ -461,10 +463,12 @@ impl<V: Clone + Send + Sync, A: Allocator, Cf: CoFree<V=V, A=A>> ByteNode<Cf, A>
                     }
                 }
 
-                //If we ended up with a value or a link in the CF, insert it into a new node
+                //Keep the CF if it still has a value or link; otherwise the location, dangling or not, is gone
                 if new_cf.has_rec() || new_cf.has_val() {
                     new_node.mask.set_bit(key_byte);
                     new_node.values.push(new_cf);
+                } else {
+                    is_identity = false;
                 }
             } else {
                 new_node.mask.set_bit(key_byte);

src/trie_map.rs

@@ -876,6 +876,35 @@ mod tests {
         //                                          c-> List("aaa")
     }
 
+    /// `subtract` keeps a value when the other map only has a longer path through it
+    #[test]
+    fn map_subtract_keeps_value_under_other_path() {
+        let mut a: PathMap<u64> = PathMap::new();
+        a.insert([0], 0);
+        a.insert([0, 0], 0);
+        a.insert([0, 0, 0], 0);
+        a.insert([1, 0, 0], 0);
+        let mut b: PathMap<u64> = PathMap::new();
+        b.insert([0, 0], 0);
+        b.insert([1, 0, 0], 0);
+
+        let diff = a.subtract(&b);
+        let paths: Vec<(Vec<u8>, u64)> = diff.iter().map(|(k, v)| (k.to_vec(), *v)).collect();
+        assert_eq!(paths, vec![(vec![0], 0), (vec![0, 0, 0], 0)]);
+
+        // distinct values
+        let mut a: PathMap<u64> = PathMap::new();
+        a.insert([0], 5);
+        a.insert([0, 0], 6);
+        a.insert([0, 0, 0], 7);
+        a.insert([1, 0, 0], 8);
+        let diff = a.subtract(&b);
+        assert_eq!(diff.val_at([0]), Some(&5));
+        assert_eq!(diff.val_at([0, 0]), Some(&6));   // 6 - 0 is Element(6) for u64
+        assert_eq!(diff.val_at([0, 0, 0]), Some(&7));
+        assert_eq!(diff.val_at([1, 0, 0]), Some(&8));
+    }
+
     #[test]
     fn map_insert_test() {
         let keys = [

src/write_zipper.rs

@@ -3754,6 +3754,39 @@ mod tests {
         assert_eq!(map.iter().count(), 1);
     }
 
+    /// `subtract_into` keeps a value when the source only has a longer path through it
+    #[test]
+    fn write_zipper_subtract_into_value_under_source_path() {
+        let mut map: PathMap<u64> = PathMap::new();
+        map.insert([0], 0);
+        map.insert([0, 0], 0);
+        map.insert([0, 0, 0], 0);
+        map.insert([1, 0, 0], 0);
+        let mut src: PathMap<u64> = PathMap::new();
+        src.insert([0, 0], 0);
+        src.insert([1, 0, 0], 0);
+
+        assert_eq!(map.write_zipper().subtract_into(&src.read_zipper(), false), AlgebraicStatus::Element);
+        let remaining: Vec<(Vec<u8>, u64)> = map.iter().map(|(k, v)| (k.to_vec(), *v)).collect();
+        assert_eq!(remaining, vec![(vec![0], 0), (vec![0, 0, 0], 0)]);
+
+        // Same shape, reached through a join first
+        let mut map: PathMap<u64> = PathMap::new();
+        map.insert([], 0);
+        map.insert([0], 0);
+        map.insert([0, 0, 0], 0);
+        let mut src: PathMap<u64> = PathMap::new();
+        src.insert([], 0);
+        src.insert([0, 0], 0);
+        src.insert([1, 0, 0], 0);
+        let mut wz = map.write_zipper();
+        wz.join_into(&src.read_zipper());
+        wz.subtract_into(&src.read_zipper(), false);
+        drop(wz);
+        let remaining: Vec<(Vec<u8>, u64)> = map.iter().map(|(k, v)| (k.to_vec(), *v)).collect();
+        assert_eq!(remaining, vec![(vec![0], 0), (vec![0, 0, 0], 0)]);
+    }
+
     /// Tests how `subtract_into` handles dangling paths, including situations with extraneous empty nodes hanging around
     #[test]
     fn write_zipper_subtract_into_test2() {
@@ -6672,4 +6705,51 @@ mod tests {
         }
         assert_eq!(keys(&m), ["cx", "cy", "d"]);
     }
+
+    /// Every location in `map`, dangling ones included, with its value
+    fn all_locations(map: &PathMap<u64>) -> Vec<(Vec<u8>, Option<u64>)> {
+        let mut rz = map.read_zipper();
+        let mut locations = vec![];
+        loop {
+            locations.push((rz.path().to_vec(), rz.val().cloned()));
+            if !rz.to_next_step() { break }
+        }
+        locations
+    }
+
+    /// Dense `subtract_into` against a list or tiny node drops a dangling path the source reaches
+    #[test]
+    fn write_zipper_subtract_into_dense_drops_reached_dangling_path() {
+        // Empty onward link at [2]; the source (a LineListNode) reaches [2]
+        let mut dst = PathMap::<u64>::new();
+        for b in [1u8, 3, 4] { dst.set_val_at(&[b], 1); }
+        dst.create_path(&[2u8]);
+        let mut src = PathMap::<u64>::new();
+        src.set_val_at(&[2u8, 0, 1], 246);
+        let mut wz = dst.write_zipper();
+        assert_eq!(wz.subtract_into(&src.read_zipper(), false), AlgebraicStatus::Element);
+        drop(wz);
+        assert_eq!(all_locations(&dst), vec![(vec![], None), (vec![1], Some(1)), (vec![3], Some(1)), (vec![4], Some(1))]);
+
+        // Dangling [2] from an empty meet; the source is a TinyRefNode
+        let mut dst = PathMap::<u64>::new();
+        dst.set_val_at(&[2u8, 0, 0, 0, 0], 0);
+        dst.set_val_at(&[0u8, 0, 0, 0, 0], 0);
+        dst.set_val_at(&[0u8], 0);
+        let mut srcs = PathMap::<u64>::new();
+        srcs.set_val_at(&[1u8, 2, 0], 0);
+        let mut wz = dst.write_zipper();
+        wz.descend_to_byte(2);
+        let ra = srcs.read_zipper_at_path(&[1u8]);
+        let rb = srcs.read_zipper_at_path(&[1u8, 0]);
+        assert_eq!(wz.meet_2(&ra, &rb), AlgebraicStatus::None);
+        wz.reset();
+        assert_eq!({ let mut probe = wz.fork_read_zipper(); probe.descend_to(&[2u8]); probe.path_exists() }, true, "the meet should leave [2] dangling");
+        assert_eq!(wz.subtract_into(&ra, false), AlgebraicStatus::Element);
+        drop(wz);
+        assert_eq!(all_locations(&dst), vec![
+            (vec![], None), (vec![0], Some(0)), (vec![0, 0], None), (vec![0, 0, 0], None),
+            (vec![0, 0, 0, 0], None), (vec![0, 0, 0, 0, 0], Some(0)),
+        ]);
+    }
 }