From 5f9d95c3b0756627c91105231a3c0bab2cd1f2ef Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Thu, 17 Sep 2026 04:32:12 +0000 Subject: [PATCH 1/3] Fix OverlayZipper focus_byte at the root Its sources may be rooted at different paths, so at the root their focus bytes differ and the debug assert failed; sibling steps then tried to move the root. Report None at the root. --- src/overlay_zipper.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/overlay_zipper.rs b/src/overlay_zipper.rs index 211c8aa3..f462cd15 100644 --- a/src/overlay_zipper.rs +++ b/src/overlay_zipper.rs @@ -159,6 +159,10 @@ impl ZipperMoving #[inline] fn focus_byte(&self) -> Option { + //The sources may be rooted at different paths, so at the root their bytes differ + if self.depth() == 0 { + return None; + } let byte = self.a.focus_byte(); debug_assert_eq!(byte, self.b.focus_byte()); byte @@ -577,4 +581,21 @@ mod tests { assert_eq!(moved, true); assert_eq!(observed, oz.path(), "observer must match the resulting path"); } + + /// Sources rooted at different paths: no focus byte, and no sibling step, at the root + #[test] + fn overlay_sources_at_different_roots() { + use crate::zipper::ZipperIteration; + let mut a = PathMap::::new(); + for p in [&[1u8, 5][..], &[1, 6], &[2, 5, 1]] { a.set_val_at(p, 1); } + let mut z = OverlayZipper::new(a.read_zipper_at_path(&[1u8]), a.read_zipper_at_path(&[2u8])); + assert_eq!(z.focus_byte(), None); + assert_eq!(z.to_next_sibling_byte(), None); + assert_eq!(z.to_prev_sibling_byte(), None); + let mut steps = vec![]; + while z.to_next_step() { steps.push(z.path().to_vec()); assert!(steps.len() < 16); } + assert_eq!(steps, vec![vec![5], vec![5, 1], vec![6]]); + let mut o = Vec::new(); + while z.to_next_val_observed(&mut o) { assert_eq!(&o[..], z.path()); } + } } From c7b519668f162e1227480c5ef839176f1f382aac Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Thu, 17 Sep 2026 04:33:04 +0000 Subject: [PATCH 2/3] Fix OverlayZipper::descend_to_val when the second source stops first The branch meant to bring the second source down to the first one's depth descended the first source again, leaving them at different depths. --- src/overlay_zipper.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/overlay_zipper.rs b/src/overlay_zipper.rs index f462cd15..d9f1ae94 100644 --- a/src/overlay_zipper.rs +++ b/src/overlay_zipper.rs @@ -215,7 +215,7 @@ impl ZipperMoving self.a.ascend(depth_a - depth_o); depth_o } else { - self.a.descend_to(&path[depth_o..depth_a]); + self.b.descend_to(&path[depth_o..depth_a]); depth_a } } else { @@ -598,4 +598,21 @@ mod tests { let mut o = Vec::new(); while z.to_next_val_observed(&mut o) { assert_eq!(&o[..], z.path()); } } + + /// `descend_to_val` keeps both sources at the same place when the second one stops first + #[test] + fn overlay_descend_to_val_second_stops_first() { + let mut a = PathMap::::new(); + a.set_val_at(&[1u8, 2, 3], 1); + let mut b = PathMap::::new(); + b.set_val_at(&[1u8, 7], 2); + for (x, y) in [(&a, &b), (&b, &a)] { + let mut z = OverlayZipper::new(x.read_zipper(), y.read_zipper()); + assert_eq!(z.descend_to_val(&[1u8, 2, 3, 4, 5]), 3); + assert_eq!(z.depth(), 3); + assert_eq!(z.path(), &[1u8, 2, 3]); + assert_eq!(z.ascend(3), 3); + assert_eq!(z.depth(), 0); + } + } } From 7eed4930f4f3d1f9bc154d906f33ceab2892dc11 Mon Sep 17 00:00:00 2001 From: Luke Peterson Date: Thu, 1 Oct 2026 19:56:49 -0600 Subject: [PATCH 3/3] Adding test and fix for newly-identified bug in OverlayZipper::descend_to_val --- src/overlay_zipper.rs | 72 +++++++++++++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 17 deletions(-) diff --git a/src/overlay_zipper.rs b/src/overlay_zipper.rs index d9f1ae94..3cb88a2d 100644 --- a/src/overlay_zipper.rs +++ b/src/overlay_zipper.rs @@ -200,27 +200,37 @@ impl ZipperMoving fn descend_to_val>(&mut self, path: K) -> usize { let path = path.as_ref(); - let depth_a = self.a.descend_to_val(path); - let depth_o = self.b.descend_to_val(path); - if depth_a < depth_o { - if self.a.is_val() { - self.b.ascend(depth_o - depth_a); - depth_a - } else { - self.a.descend_to(&path[depth_a..depth_o]); - depth_o - } - } else if depth_o < depth_a { - if self.b.is_val() { - self.a.ascend(depth_a - depth_o); - depth_o + let mut descended = 0; + while descended < path.len() { + let remaining = &path[descended..]; + let depth_a = self.a.descend_to_val(remaining); + let depth_b = self.b.descend_to_val(remaining); + // A source at a value can return zero without finding a new value along the path. + let advanced = if depth_a < depth_b { + if depth_a > 0 && self.a.is_val() { + self.b.ascend(depth_b - depth_a); + depth_a + } else { + self.a.descend_to(&remaining[depth_a..depth_b]); + depth_b + } + } else if depth_b < depth_a { + if depth_b > 0 && self.b.is_val() { + self.a.ascend(depth_a - depth_b); + depth_b + } else { + self.b.descend_to(&remaining[depth_b..depth_a]); + depth_a + } } else { - self.b.descend_to(&path[depth_o..depth_a]); depth_a + }; + descended += advanced; + if advanced == 0 || self.is_val() { + break; } - } else { - depth_a } + descended } fn descend_to_byte(&mut self, k: u8) { @@ -414,6 +424,7 @@ mod tests { zipper_moving_tests, ZipperMoving, ZipperPath, + ZipperValues, OverlayZipper }, }; @@ -615,4 +626,31 @@ mod tests { assert_eq!(z.depth(), 0); } } + + #[test] + fn overlay_descend_to_val_skips_values_filtered_by_mapping() { + fn only_a<'a>(a: Option<&'a u64>, _: Option<&'a u64>) -> Option<&'a u64> { a } + fn only_b<'a>(_: Option<&'a u64>, b: Option<&'a u64>) -> Option<&'a u64> { b } + + let mut a = PathMap::::new(); + a.set_val_at(&[1u8, 2, 3], 3); + let mut b = PathMap::::new(); + b.set_val_at(&[1u8], 1); + + let mut z = OverlayZipper::with_mapping(a.read_zipper(), b.read_zipper(), only_a); + assert_eq!(z.descend_to_val(&[1u8, 2, 3]), 3); + assert_eq!(z.path(), &[1u8, 2, 3]); + assert_eq!(z.val(), Some(&3)); + + let mut a = PathMap::::new(); + a.set_val_at(&[1u8], 1); + a.set_val_at(&[1u8, 2], 2); + let mut b = PathMap::::new(); + b.set_val_at(&[1u8, 2, 3], 3); + + let mut z = OverlayZipper::with_mapping(a.read_zipper(), b.read_zipper(), only_b); + assert_eq!(z.descend_to_val(&[1u8, 2, 3]), 3); + assert_eq!(z.path(), &[1u8, 2, 3]); + assert_eq!(z.val(), Some(&3)); + } }