diff --git a/src/dense_byte_node.rs b/src/dense_byte_node.rs index ba66a7cb..140c0144 100644 --- a/src/dense_byte_node.rs +++ b/src/dense_byte_node.rs @@ -505,6 +505,11 @@ impl> ByteNode new_node.values.push(cf.clone()); } else { + //Dropping this value changes the destination + if cf.val().is_some() { + is_identity = false; + } + //If there is an onward link in the CF and other node, continue the restriction recursively if let Some(self_child) = cf.rec() { let other_child = other.get_node_at_key(&[key_byte]); @@ -531,6 +536,9 @@ impl> ByteNode is_identity = false; } } + } else { + //The target slot is dangling and the restrictor has no value here: drop it + is_identity = false; } } } else { @@ -2359,7 +2367,8 @@ impl> ByteNode // Iterate the overlap mask directly. Slot indexes are recovered with // prefix popcounts in each dense-mask word. let mut mm: ByteMask = self.mask & other.mask; - let mut is_identity = self.mask == mm && other.mask == mm; + //Restrict is non-commutative: only `self`'s branches matter for identity + let mut is_identity = self.mask == mm; let mmc = [mm.0[0].count_ones(), mm.0[1].count_ones(), mm.0[2].count_ones(), mm.0[3].count_ones()]; diff --git a/src/write_zipper.rs b/src/write_zipper.rs index 30f3b793..4647adb1 100644 --- a/src/write_zipper.rs +++ b/src/write_zipper.rs @@ -6753,6 +6753,100 @@ mod tests { assert_eq!(keys(&m), ["cx", "cy", "d"]); } + /// Dense `restrict` is `Identity` even when the restrictor has extra branches + #[test] + fn write_zipper_restrict_wider_restrictor_is_identity() { + fn mk(ps: &[(&[u8], u64)]) -> PathMap { let mut m = PathMap::new(); for (p, v) in ps { m.set_val_at(p, *v); } m } + fn vals(m: &PathMap) -> Vec<(Vec, u64)> { m.iter().map(|(k, v)| (k.to_vec(), *v)).collect() } + + let mut dst = mk(&[(&[0], 1), (&[1], 2), (&[2], 3)]); + let before = vals(&dst); + let restrictor = mk(&[(&[0], 0), (&[1], 0), (&[2], 0), (&[3], 0), (&[4], 0)]); + let st = { let mut wz = dst.write_zipper(); wz.restrict(&restrictor.read_zipper()) }; + assert_eq!(st, AlgebraicStatus::Identity); + assert_eq!(vals(&dst), before); + + //A restriction that really does drop a branch still reports it + let mut dst = mk(&[(&[0], 1), (&[1], 2), (&[2], 3)]); + let restrictor = mk(&[(&[0], 0), (&[1], 0), (&[3], 0), (&[4], 0)]); + let st = { let mut wz = dst.write_zipper(); wz.restrict(&restrictor.read_zipper()) }; + assert_eq!(st, AlgebraicStatus::Element); + assert_eq!(vals(&dst), vec![(vec![0], 1), (vec![1], 2)]); + } + + /// Dense `restrict` against a list node drops a value in the same node as a kept child + #[test] + fn write_zipper_restrict_drops_value_beside_kept_child() { + fn mk(ps: &[(&[u8], u64)]) -> PathMap { let mut m = PathMap::new(); for (p, v) in ps { m.set_val_at(p, *v); } m } + fn vals(m: &PathMap) -> Vec<(Vec, u64)> { m.iter().map(|(k, v)| (k.to_vec(), *v)).collect() } + + //Dense root with a value and a child at [0] + let mut dst = mk(&[(&[0], 7), (&[0, 0], 1), (&[1], 2), (&[2], 3)]); + let filter = mk(&[(&[0], 0), (&[0, 0], 0), (&[5], 0), (&[6], 0)]); + { let mut wz = dst.write_zipper(); wz.meet_into(&filter.read_zipper(), false); } + assert_eq!(vals(&dst), vec![(vec![0], 7), (vec![0, 0], 1)]); + + //The restrictor has no value at `[0]`, so `[0]` is not kept, while `[0, 0]` is + let restrictor = mk(&[(&[0, 0], 0)]); + let st = { let mut wz = dst.write_zipper(); wz.restrict(&restrictor.read_zipper()) }; + assert_eq!(vals(&dst), vec![(vec![0, 0], 1)]); + assert_eq!(st, AlgebraicStatus::Element); + } + + /// Dense `restrict` drops a dangling path not kept by the restrictor + #[test] + fn write_zipper_restrict_drops_dangling_branch() { + fn mk(ps: &[(&[u8], u64)]) -> PathMap { let mut m = PathMap::new(); for (p, v) in ps { m.set_val_at(p, *v); } m } + fn vals(m: &PathMap) -> Vec<(Vec, u64)> { m.iter().map(|(k, v)| (k.to_vec(), *v)).collect() } + + //Dense root with [0] dangling + let mut dst = mk(&[(&[0], 1), (&[1], 2), (&[2], 3), (&[3], 4)]); + let filter = mk(&[(&[0], 1), (&[1], 2)]); + { let mut wz = dst.write_zipper(); wz.meet_into(&filter.read_zipper(), false); } + dst.remove_val_at(&[0u8], false); + assert_eq!(dst.path_exists_at(&[0u8]), true, "[0] should be left dangling"); + assert_eq!(vals(&dst), vec![(vec![1], 2)]); + + //The restrictor has no value at [0], so the dangling [0] goes + let restrictor = mk(&[(&[0, 9], 0), (&[1], 0)]); + let st = { let mut wz = dst.write_zipper(); wz.restrict(&restrictor.read_zipper()) }; + assert_eq!(st, AlgebraicStatus::Element); + assert_eq!(dst.path_exists_at(&[0u8]), false); + assert_eq!(vals(&dst), vec![(vec![1], 2)]); + + //A dangling path that is kept stays: the restrictor has a value at [0]. + let mut dst = mk(&[(&[0], 1), (&[1], 2), (&[2], 3), (&[3], 4)]); + { let mut wz = dst.write_zipper(); wz.meet_into(&filter.read_zipper(), false); } + dst.remove_val_at(&[0u8], false); + let restrictor = mk(&[(&[0], 0), (&[1], 0)]); + let st = { let mut wz = dst.write_zipper(); wz.restrict(&restrictor.read_zipper()) }; + assert_eq!(st, AlgebraicStatus::Identity); + assert_eq!(dst.path_exists_at(&[0u8]), true); + assert_eq!(vals(&dst), vec![(vec![1], 2)]); + + //Fuzzer reproducer + let mut map0 = PathMap::::new(); + map0.set_val_at(&[0u8], 0); + let mut map1 = PathMap::::new(); + map1.set_val_at(&[0u8, 0, 0, 0], 0); + map1.set_val_at(&[1u8], 0); + { + let mut wz = map0.write_zipper_at_path(&[]); + let mut rz = map1.read_zipper_at_path(&[]); + wz.join_into(&rz); + wz.descend_first_byte(); + rz.to_next_val(); + wz.subtract_into(&rz, false); + rz.to_next_val(); + wz.meet_into(&rz, false); + } + assert_eq!(map0.path_exists_at(&[0u8]), true, "meet_into leaves [0] dangling"); + let st = { let mut wz = map0.write_zipper(); wz.restrict(&map1.read_zipper()) }; + assert_eq!(st, AlgebraicStatus::Element); + assert_eq!(map0.path_exists_at(&[0u8]), false); + assert_eq!(vals(&map0), vec![(vec![1], 0)]); + + } /// `remove_unmasked_branches` at or below a dangling path does nothing #[test] fn write_zipper_test_remove_unmasked_branches_dangling_focus() {