From caf52a2b619ef7f89453e6740c6ae34df3a03c02 Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Thu, 17 Sep 2026 04:38:18 +0000 Subject: [PATCH 1/2] Fix PrefixZipper over a source rooted at a missing path The prefix always reported itself as existing, with one child, so a walk descended through it into a source that doesn't exist. The prefix now exists only when the source's root does. --- src/prefix_zipper.rs | 46 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/src/prefix_zipper.rs b/src/prefix_zipper.rs index 10eb42f2..789e3d87 100644 --- a/src/prefix_zipper.rs +++ b/src/prefix_zipper.rs @@ -122,6 +122,9 @@ impl<'prefix, Z> PrefixZipper<'prefix, Z> /// Returns `true` if the focus moved. The descended bytes are appended to this zipper's path /// buffer and reported to `obs`. Does nothing if the focus is already within the source. fn consume_prefix(&mut self, obs: &mut Obs) -> bool { + if !self.source.path_exists() { + return false; + } match self.position.prefixed_depth() { Some(prefixed_depth) => { let prefix_rest = &self.prefix[self.origin_depth + prefixed_depth..]; @@ -331,7 +334,9 @@ impl<'prefix, Z> Zipper for PrefixZipper<'prefix, Z> { fn path_exists(&self) -> bool { match self.position { - PrefixPos::Prefix {..} => true, + //The source stays at its root while the focus is in the prefix, and the prefix only + // exists if the source's root does + PrefixPos::Prefix {..} => self.source.path_exists(), PrefixPos::PrefixOff {..} => false, PrefixPos::Source => self.source.path_exists(), } @@ -344,17 +349,18 @@ impl<'prefix, Z> Zipper for PrefixZipper<'prefix, Z> } fn child_count(&self) -> usize { match self.position { - PrefixPos::Prefix {..} => 1, + PrefixPos::Prefix {..} => self.source.path_exists() as usize, PrefixPos::PrefixOff {..} => 0, PrefixPos::Source => self.source.child_count(), } } fn child_mask(&self) -> ByteMask { match self.position { - PrefixPos::Prefix { valid } => { + PrefixPos::Prefix { valid } if self.source.path_exists() => { let byte = self.prefix[self.origin_depth + valid]; ByteMask::from(byte) }, + PrefixPos::Prefix {..} => ByteMask::EMPTY, PrefixPos::PrefixOff {..} => ByteMask::EMPTY, PrefixPos::Source => self.source.child_mask(), } @@ -404,7 +410,7 @@ impl<'prefix, Z> ZipperMoving for PrefixZipper<'prefix, Z> if let PrefixPos::Prefix { valid } = &self.position { let valid = *valid; let rest_prefix = &self.prefix[self.origin_depth + valid..]; - let overlap = find_prefix_overlap(rest_prefix, path); + let overlap = if self.source.path_exists() { find_prefix_overlap(rest_prefix, path) } else { 0 }; path = &path[overlap..]; self.set_valid(valid + overlap); descended += overlap; @@ -561,7 +567,7 @@ impl<'prefix, Z> ZipperIteration for PrefixZipper<'prefix, Z> if k == 0 { return false; } - if self.position.is_invalid() { + if self.position.is_invalid() || !self.source.path_exists() { return false; } //The prefix is a single forced path, so the bytes it contributes always exist and never @@ -983,4 +989,34 @@ mod tests { //...so `descend_until` must report that it moved assert_eq!(moved, true); } + + /// A prefix in front of a source rooted at a missing path doesn't exist either + #[test] + fn prefix_zipper_over_missing_source() { + let mut map = PathMap::::new(); + map.set_val_at(&[0u8], 1); + let mut z = PrefixZipper::new(&[0u8, 7][..], map.read_zipper_at_path(&[5u8])); + assert!(!z.path_exists()); + assert_eq!(z.child_count(), 0); + assert_eq!(z.descend_first_byte(), None); + assert!(!z.to_next_step()); + assert!(!z.to_next_val()); + assert!(!z.descend_until()); + assert!(!z.descend_first_k_path(1)); + assert_eq!(z.path(), &[] as &[u8]); + assert_eq!(z.descend_to_existing(&[0u8, 7]), 0); + z.descend_to(&[0u8]); + assert!(!z.path_exists()); + assert_eq!(z.descend_first_byte(), None); + + //With the source present the prefix exists as before + let mut z = PrefixZipper::new(&[0u8, 7][..], map.read_zipper()); + assert!(z.path_exists()); + assert_eq!(z.descend_first_byte(), Some(0)); + assert!(z.path_exists()); + let mut steps = vec![]; + z.reset(); + while z.to_next_step() { steps.push(z.path().to_vec()); } + assert_eq!(steps, vec![vec![0], vec![0, 7], vec![0, 7, 0]]); + } } From c0c04035b4e39194f88e28cb373aefdbc6c92359 Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Thu, 17 Sep 2026 05:29:14 +0000 Subject: [PATCH 2/2] Fix PrefixZipper fork rooted at the focus --- src/prefix_zipper.rs | 56 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/src/prefix_zipper.rs b/src/prefix_zipper.rs index 789e3d87..de424480 100644 --- a/src/prefix_zipper.rs +++ b/src/prefix_zipper.rs @@ -59,6 +59,8 @@ pub struct PrefixZipper<'prefix, Z> { prefix: Cow<'prefix, [u8]>, origin_depth: usize, position: PrefixPos, + /// The zipper's own root is off the trie, as for a fork taken past a diverged prefix + off_root: bool, } impl<'prefix, Z> PrefixZipper<'prefix, Z> @@ -83,6 +85,7 @@ impl<'prefix, Z> PrefixZipper<'prefix, Z> prefix, origin_depth: 0, position, + off_root: false, } } @@ -110,7 +113,9 @@ impl<'prefix, Z> PrefixZipper<'prefix, Z> fn set_valid(&mut self, valid: usize) { debug_assert!(valid <= self.prefix.len(), "valid prefix can't be outside prefix"); - self.position = if valid == self.prefix.len() - self.origin_depth { + self.position = if self.off_root { + PrefixPos::PrefixOff { valid: 0, invalid: 0 } + } else if valid == self.prefix.len() - self.origin_depth { PrefixPos::Source } else { PrefixPos::Prefix { valid } @@ -379,7 +384,7 @@ impl<'prefix, Z> ZipperMoving for PrefixZipper<'prefix, Z> fn at_root(&self) -> bool { match self.position { PrefixPos::Prefix { valid } => valid == 0, - PrefixPos::PrefixOff {..} => false, + PrefixPos::PrefixOff { valid, invalid } => self.off_root && valid == 0 && invalid == 0, PrefixPos::Source => self.prefix.len() <= self.origin_depth && self.source.at_root(), } } @@ -643,12 +648,19 @@ impl<'prefix, Z, V> ZipperForking for PrefixZipper<'prefix, Z> { type ReadZipperT<'a> = PrefixZipper<'prefix, Z::ReadZipperT<'a>> where Self: 'a; fn fork_read_zipper<'a>(&'a self) -> >::ReadZipperT<'a> { + //The fork is rooted at the focus: in the source, partway along the prefix, or off the trie + let (prefix, position, off_root) = match self.position { + PrefixPos::Source => (Cow::Borrowed(&[][..]), PrefixPos::Source, false), + PrefixPos::Prefix { valid } => (Cow::Owned(self.prefix[self.origin_depth + valid..].to_vec()), PrefixPos::Prefix { valid: 0 }, false), + PrefixPos::PrefixOff {..} => (Cow::Borrowed(&[][..]), PrefixPos::PrefixOff { valid: 0, invalid: 0 }, true), + }; PrefixZipper { path: Vec::new(), - position: PrefixPos::Prefix { valid: 0 }, + position, source: self.source.fork_read_zipper(), - prefix: self.prefix.clone(), + prefix, origin_depth: 0, + off_root, } } } @@ -1019,4 +1031,40 @@ mod tests { while z.to_next_step() { steps.push(z.path().to_vec()); } assert_eq!(steps, vec![vec![0], vec![0, 7], vec![0, 7, 0]]); } + + /// A fork is rooted at the focus, wherever the focus is + #[test] + fn prefix_zipper_fork_at_focus() { + use crate::zipper::ZipperForking; + let mut map = PathMap::::new(); + map.set_val_at(&[5u8], 1); + map.set_val_at(&[5u8, 6], 2); + fn steps(z: &mut Z) -> Vec> { + let mut v = vec![]; + while z.to_next_step() { v.push(z.path().to_vec()); assert!(v.len() < 16); } + v + } + let mut z = PrefixZipper::new(&[2u8, 3][..], map.read_zipper()); + for (at, want) in [ + (&[][..], vec![vec![2], vec![2, 3], vec![2, 3, 5], vec![2, 3, 5, 6]]), + (&[2u8][..], vec![vec![3], vec![3, 5], vec![3, 5, 6]]), + (&[2u8, 3, 5][..], vec![vec![6]]), + (&[9u8][..], vec![]), + ] { + z.reset(); + z.descend_to(at); + let mut f = z.fork_read_zipper(); + assert!(f.at_root(), "{at:?}"); + assert_eq!(f.path_exists(), z.path_exists(), "{at:?}"); + assert_eq!(steps(&mut f), want, "{at:?}"); + f.descend_to(&[1u8, 1]); + assert_eq!(f.ascend(5), 2, "{at:?}"); + assert!(f.at_root(), "{at:?}"); + } + //An empty prefix + let mut z = PrefixZipper::new(&[][..], map.read_zipper()); + z.set_root_prefix_path(&[]).unwrap(); + let mut f = z.fork_read_zipper(); + assert_eq!(steps(&mut f), vec![vec![5], vec![5, 6]]); + } }