From 7c7a598fcc519f14e8cd77f3b9e4fcf785b8f50d Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Thu, 17 Sep 2026 04:20:33 +0000 Subject: [PATCH 1/2] Fix the default k-path walk looping forever at a leaf With nothing below the base and no sibling, k_path_default_internal never reached its exit and spun. It also let k = 0 step sideways from the base. Stop when back at the base. --- src/zipper.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/zipper.rs b/src/zipper.rs index fcede695..f9986d94 100644 --- a/src/zipper.rs +++ b/src/zipper.rs @@ -1119,6 +1119,8 @@ fn k_path_default_internal(z: &mut if z.depth() == base_idx + k { return true } } } + //Back at the base: nothing (more) below it, and its own siblings are out of bounds + if z.depth() == base_idx { return false } //A sibling step replaces the last byte rather than adding one, so the observer sees the old //byte retracted before the new one arrives if let Some(byte) = z.to_next_sibling_byte() { @@ -3396,6 +3398,26 @@ pub(crate) mod read_zipper_core { } } + /// The default k-path walk ends when there is nothing below its base, and `k = 0` returns `false` + #[test] + fn default_k_path_walk_at_a_leaf() { + use crate::zipper::ProductZipperG; + let mut leaf = PathMap::::new(); + leaf.set_val_at(&[1u8], 1); + let empty = PathMap::::new(); + for (map, path) in [(&empty, &[][..]), (&leaf, &[1u8][..]), (&leaf, &[][..])] { + for k in 0..3 { + let mut z = ProductZipperG::new(map.read_zipper(), [empty.read_zipper()]); + z.descend_to(path); + let found = z.descend_first_k_path(k); + assert_eq!(found, map.val_count() > 0 && path.is_empty() && k == 1, "{path:?} k={k}"); + if !found { + assert_eq!(z.path(), path, "{path:?} k={k}"); + } + } + } + } + /// Validate we don't accidentially reallocate the path buffer when we don't need to #[test] fn read_zipper_reserve_buffer_test() { From 66c7acb0cc512f51588a535560fbc628a7b38f79 Mon Sep 17 00:00:00 2001 From: Luke Peterson Date: Fri, 25 Sep 2026 01:43:58 -0600 Subject: [PATCH 2/2] Lifting ZipperIteration test from a specific Zipper type to the ZipperIteration test macro --- src/zipper.rs | 54 +++++++++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/src/zipper.rs b/src/zipper.rs index d106b2cb..84f4fd6c 100644 --- a/src/zipper.rs +++ b/src/zipper.rs @@ -1119,11 +1119,10 @@ fn k_path_default_internal(z: &mut if z.depth() == base_idx + k { return true } } } - //Back at the base: nothing (more) below it, and its own siblings are out of bounds if z.depth() == base_idx { return false } - //A sibling step replaces the last byte rather than adding one, so the observer sees the old - //byte retracted before the new one arrives if let Some(byte) = z.to_next_sibling_byte() { + //A sibling step replaces the last byte rather than adding one, so the observer sees the old + //byte retracted before the new one arrives obs.ascend(1); obs.descend_to_byte(byte); if z.depth() == base_idx + k { return true } @@ -3399,26 +3398,6 @@ pub(crate) mod read_zipper_core { } } - /// The default k-path walk ends when there is nothing below its base, and `k = 0` returns `false` - #[test] - fn default_k_path_walk_at_a_leaf() { - use crate::zipper::ProductZipperG; - let mut leaf = PathMap::::new(); - leaf.set_val_at(&[1u8], 1); - let empty = PathMap::::new(); - for (map, path) in [(&empty, &[][..]), (&leaf, &[1u8][..]), (&leaf, &[][..])] { - for k in 0..3 { - let mut z = ProductZipperG::new(map.read_zipper(), [empty.read_zipper()]); - z.descend_to(path); - let found = z.descend_first_k_path(k); - assert_eq!(found, map.val_count() > 0 && path.is_empty() && k == 1, "{path:?} k={k}"); - if !found { - assert_eq!(z.path(), path, "{path:?} k={k}"); - } - } - } - } - /// `get_val_with_witness` agrees with `val` on owned read zippers, including at a root without a value #[test] fn read_zipper_owned_get_val_with_witness() { @@ -4839,6 +4818,22 @@ pub(crate) mod zipper_iteration_tests { crate::zipper::zipper_iteration_tests::run_test(&mut temp_store, $make_z, b"", crate::zipper::zipper_iteration_tests::k_path_zero) } + #[test] + fn [<$z_name _k_path_walk_at_a_leaf>]() { + use crate::zipper::zipper_iteration_tests::{k_path_walk_at_a_leaf, run_test}; + const LEAF_KEYS: &[&[u8]] = &[&[1u8]]; + for (keys, path, has_child) in [ + (&[][..], &[][..], false), + (LEAF_KEYS, &[1u8][..], false), + (LEAF_KEYS, &[][..], true), + ] { + let mut temp_store = $read_keys(keys); + run_test(&mut temp_store, $make_z, b"", |zipper| { + k_path_walk_at_a_leaf(zipper, path, has_child) + }); + } + } + #[test] fn [<$z_name _k_path_test2>]() { let paths = crate::zipper::zipper_iteration_tests::k_path_test2_paths(); @@ -4964,6 +4959,19 @@ pub(crate) mod zipper_iteration_tests { assert_eq!(zipper.path(), b"a"); } + /// A k-path walk must stop at an empty root or leaf without stepping to a sibling. + pub fn k_path_walk_at_a_leaf(mut zipper: Z, path: &[u8], has_child: bool) { + for k in 0..3 { + zipper.reset(); + zipper.descend_to(path); + let found = zipper.descend_first_k_path(k); + assert_eq!(found, has_child && k == 1, "{path:?} k={k}"); + if !found { + assert_eq!(zipper.path(), path, "{path:?} k={k}"); + } + } + } + /// This is a toy encoding where `:n:` precedes a symbol `n` characters long pub const K_PATH_TEST1_KEYS: &[&[u8]] = &[ b":5:above:3:the:4:fray:",