Updating ZipperMoving::descend_until to accept generic write sink as opposed to Option<&mut Vec<u8>> and fixing a couple other bugs & todos along the way

luketpeterson · luketpeterson · commit 18e20806af52 · 2025-10-10T23:29:28.000-06:00
diff --git a/benches/oeis.rs b/benches/oeis.rs
@@ -1,21 +1,21 @@
 use std::io::Read;
 use std::usize;
 use pathmap::PathMap;
-use pathmap::zipper::{Zipper, ZipperValues, ZipperMoving, ZipperWriting, ZipperCreation};
+use pathmap::zipper::{Zipper, ZipperValues, ZipperMoving, ZipperPath, ZipperWriting, ZipperCreation};
 use num::BigInt;
 use divan::{Divan, Bencher, black_box};
 
 const MAX_OFFSET: u8 = 10;
 
-fn drop_symbol_head_byte<Z: ZipperWriting<usize> + Zipper + ZipperMoving>(loc: &mut Z) {
+fn drop_symbol_head_byte<Z: ZipperWriting<usize> + Zipper + ZipperMoving + ZipperPath>(loc: &mut Z) {
   let mut it = loc.child_mask().iter();
 
   let p = loc.path().to_vec();
   while let Some(b) = it.next() {
     if b == 0 { continue }
     assert!(loc.descend_to(&[b]));
     loc.join_k_path_into(b as usize, true);
-    assert!(loc.ascend(1));
+    assert_eq!(loc.ascend(1), 1);
   }
   loc.reset();
   loc.descend_to(&p[..]);
diff --git a/pathmap-derive/src/lib.rs b/pathmap-derive/src/lib.rs
@@ -300,9 +300,9 @@ pub fn derive_poly_zipper(input: TokenStream) -> TokenStream {
                     }
                 }
 
-                fn descend_until(&mut self, mut dst_path: Option<&mut Vec<u8>>) -> bool {
+                fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool {
                     match self {
-                        #(#variant_arms => inner.descend_until(dst_path),)*
+                        #(#variant_arms => inner.descend_until(desc_bytes),)*
                     }
                 }
 
diff --git a/src/arena_compact.rs b/src/arena_compact.rs
@@ -1879,9 +1879,7 @@ where Storage: AsRef<[u8]>
         self.descend_indexed_byte(0)
     }
 
-    /// Descends the zipper's focus until a branch or a value is encountered.  Returns `true` if the focus
-    /// moved otherwise returns `false`
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {
+    fn descend_until<W: std::io::Write>(&mut self, mut desc_bytes: W) -> bool {
         self.trace_pos();
         let mut descended = false;
         let orig_len = self.path.len();
@@ -1925,9 +1923,7 @@ where Storage: AsRef<[u8]>
                 }
             }
         }
-        if let Some(dst) = dst {
-            dst.extend_from_slice(&self.path[orig_len..]);
-        }
+        let _ = desc_bytes.write_all(&self.path[orig_len..]);
         descended
     }
 
diff --git a/src/empty_zipper.rs b/src/empty_zipper.rs
@@ -45,7 +45,7 @@ impl ZipperMoving for EmptyZipper {
     }
     fn descend_indexed_byte(&mut self, _idx: usize) -> Option<u8> { None }
     fn descend_first_byte(&mut self) -> Option<u8> { None }
-    fn descend_until(&mut self, _dst_path: Option<&mut Vec<u8>>) -> bool { false }
+    fn descend_until<W: std::io::Write>(&mut self, _desc_bytes: W) -> bool { false }
     fn ascend(&mut self, steps: usize) -> usize {
         let old_path_len = self.path.len() - self.path_start_idx;
         if steps > old_path_len {
diff --git a/src/experimental.rs b/src/experimental.rs
@@ -55,10 +55,8 @@ impl ZipperMoving for FullZipper {
         self.path.push(0);
         Some(0)
     }
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {
-        self.path.push(0); // not sure?
-        if let Some(dst) = dst { dst.push(0) }
-        true
+    fn descend_until<W: std::io::Write>(&mut self, _desc_bytes: W) -> bool {
+        false
     }
     fn ascend(&mut self, steps: usize) -> usize {
         if steps > self.path.len() {
diff --git a/src/morphisms.rs b/src/morphisms.rs
@@ -415,7 +415,7 @@ fn cata_side_effect_body<'a, Z, V: 'a, W, Err, AlgF, const JUMPING: bool>(mut z:
         //Descend to the next forking point, or leaf
         let mut is_leaf = false;
         while z.child_count() < 2 {
-            if !z.descend_until(None) {
+            if !z.descend_until(std::io::sink()) {
                 is_leaf = true;
                 break;
             }
@@ -735,7 +735,7 @@ fn into_cata_cached_body<'a, Z, V: 'a, W, E, AlgF, Cache, const JUMPING: bool>(
             // Descend until leaf or branch
             let mut is_leaf = false;
             'descend: while zipper.child_count() < 2 {
-                if !zipper.descend_until(None) {
+                if !zipper.descend_until(std::io::sink()) {
                     is_leaf = true;
                     break 'descend;
                 }
diff --git a/src/overlay_zipper.rs b/src/overlay_zipper.rs
@@ -212,11 +212,10 @@ impl<AV, BV, OutV, AZipper, BZipper, Mapping> ZipperMoving
         Some(byte)
     }
 
-    fn descend_until(&mut self, _dst: Option<&mut Vec<u8>>) -> bool {
-        // TODO: track dst
+    fn descend_until<W: std::io::Write>(&mut self, mut desc_bytes: W) -> bool {
         let start_depth = self.a.path().len();
-        let desc_a = self.a.descend_until(None);
-        let desc_b = self.b.descend_until(None);
+        let desc_a = self.a.descend_until(std::io::sink());
+        let desc_b = self.b.descend_until(std::io::sink());
         let path_a = &self.a.path()[start_depth..];
         let path_b = &self.b.path()[start_depth..];
         if !desc_a && !desc_b {
@@ -255,7 +254,14 @@ impl<AV, BV, OutV, AZipper, BZipper, Mapping> ZipperMoving
             let ascended = self.b.ascend(to_ascend);
             debug_assert_eq!(ascended, to_ascend);
         }
-        overlap > 0
+        debug_assert_eq!(self.a.path(), self.b.path());
+        debug_assert_eq!(start_depth + overlap, self.a.path().len());
+        if overlap > 0 {
+            let _ = desc_bytes.write_all(&self.a.path()[start_depth..]);
+            true
+        } else {
+            false
+        }
     }
 
     fn ascend(&mut self, steps: usize) -> usize {
diff --git a/src/path_tracker.rs b/src/path_tracker.rs
@@ -132,12 +132,10 @@ impl<Z: ZipperMoving> ZipperMoving for PathTracker<Z> {
         self.path.push(byte);
         Some(byte)
     }
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {
+    fn descend_until<W: std::io::Write>(&mut self, mut desc_bytes: W) -> bool {
         let orig_len = self.path.len();
-        let descended = self.zipper.descend_until(Some(&mut self.path));
-        if let Some(dst) = dst {
-            dst.extend_from_slice(&self.path[orig_len..]);
-        }
+        let descended = self.zipper.descend_until(&mut self.path);
+        let _ = desc_bytes.write_all(&self.path[orig_len..]);
         descended
     }
     // TODO: using default impl. re-using zipper's own `to_next_step` implementation
diff --git a/src/prefix_zipper.rs b/src/prefix_zipper.rs
@@ -369,7 +369,7 @@ impl<'prefix, Z> ZipperMoving for PrefixZipper<'prefix, Z>
         self.descend_indexed_byte(0)
     }
 
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {
+    fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool {
         if self.position.is_invalid() {
             return false;
         }
@@ -378,7 +378,7 @@ impl<'prefix, Z> ZipperMoving for PrefixZipper<'prefix, Z>
             self.position = PrefixPos::Source;
         }
         let len_before = self.source.path().len();
-        if !self.source.descend_until(dst) {
+        if !self.source.descend_until(desc_bytes) {
             return false;
         }
         let path = self.source.path();
diff --git a/src/product_zipper.rs b/src/product_zipper.rs
@@ -239,8 +239,8 @@ impl<'trie, V: Clone + Send + Sync + Unpin + 'trie, A: Allocator + 'trie> Zipper
         self.ensure_descend_next_factor();
         result
     }
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {
-        let result = self.z.descend_until(dst);
+    fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool {
+        let result = self.z.descend_until(desc_bytes);
         self.ensure_descend_next_factor();
         result
     }
@@ -729,19 +729,19 @@ impl<'trie, PrimaryZ, SecondaryZ, V> ZipperMoving for ProductZipperG<'trie, Prim
     fn descend_first_byte(&mut self) -> Option<u8> {
         self.descend_indexed_byte(0)
     }
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {
+    fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool {
         self.enter_factors();
         let rv = if let Some(idx) = self.factor_idx(false) {
             let zipper = &mut self.secondary[idx];
             let before = zipper.path().len();
-            let rv = zipper.descend_until(dst);
+            let rv = zipper.descend_until(desc_bytes);
             let path = zipper.path();
             if path.len() > before {
                 self.primary.descend_to(&path[before..]);
             }
             rv
         } else {
-            self.primary.descend_until(dst)
+            self.primary.descend_until(desc_bytes)
         };
         self.enter_factors();
         rv
@@ -1385,6 +1385,10 @@ mod tests {
     // together, so the resulting virtual product trie is just one long path with repetitions,
     // and then validate that ascend, ascend_until, ascend_until_branch, etc. all do the right
     // thing traversing across multiple factors, not stopping spuriously at the factor stitch points.
+    //
+    // UPDATE, Also test `descend_until` in this case, because the correct behavior should be seamlessly
+    // descend flowing across multiple factor zippers in one call, and some of the impls don't appear to
+    // do that.
 
             }
             // --- END OF MACRO GENERATED MOD ---
diff --git a/src/utils/debug/diff_zipper.rs b/src/utils/debug/diff_zipper.rs
@@ -123,13 +123,17 @@ impl<A: Zipper + ZipperMoving, B: Zipper + ZipperMoving> ZipperMoving for DiffZi
         assert_eq!(a, b);
         a
     }
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {
-        let a = self.a.descend_until(dst);
-        let b = self.b.descend_until(None);
+    fn descend_until<W: std::io::Write>(&mut self, mut desc_bytes: W) -> bool {
+        let mut a_bytes: Vec<u8> = vec![];
+        let mut b_bytes: Vec<u8> = vec![];
+        let a = self.a.descend_until(&mut a_bytes);
+        let b = self.b.descend_until(&mut b_bytes);
         if self.log_moves {
             println!("DiffZipper: descend_until")
         }
+        assert_eq!(a_bytes, b_bytes);
         assert_eq!(a, b);
+        let _ = desc_bytes.write_all(&a_bytes);
         a
     }
     fn ascend(&mut self, steps: usize) -> usize {
diff --git a/src/utils/ints.rs b/src/utils/ints.rs
@@ -302,7 +302,7 @@ fn int_range_generator_5() {
     drop(buildz);
     let mut z = zh.read_zipper_at_path(&[0]).unwrap();
 
-    z.descend_until(None);
+    z.descend_until(std::io::sink());
     z.descend_first_byte();
     let _z2 = zh.read_zipper_at_path(z.origin_path()).unwrap();
 
diff --git a/src/write_zipper.rs b/src/write_zipper.rs
@@ -352,7 +352,7 @@ impl<'a, 'path, V: Clone + Send + Sync + Unpin, A: Allocator + 'a> ZipperMoving
     fn descend_to_byte(&mut self, k: u8) -> bool { self.z.descend_to_byte(k) }
     fn descend_indexed_byte(&mut self, child_idx: usize) -> Option<u8> { self.z.descend_indexed_byte(child_idx) }
     fn descend_first_byte(&mut self) -> Option<u8> { self.z.descend_first_byte() }
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool { self.z.descend_until(dst) }
+    fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool { self.z.descend_until(desc_bytes) }
     fn to_next_sibling_byte(&mut self) -> Option<u8> { self.z.to_next_sibling_byte() }
     fn to_prev_sibling_byte(&mut self) -> Option<u8> { self.z.to_prev_sibling_byte() }
     fn ascend(&mut self, steps: usize) -> usize { self.z.ascend(steps) }
@@ -508,7 +508,7 @@ impl<'a, 'path, V: Clone + Send + Sync + Unpin, A: Allocator + 'a> ZipperMoving
     fn descend_to_byte(&mut self, k: u8) -> bool { self.z.descend_to_byte(k) }
     fn descend_indexed_byte(&mut self, child_idx: usize) -> Option<u8> { self.z.descend_indexed_byte(child_idx) }
     fn descend_first_byte(&mut self) -> Option<u8> { self.z.descend_first_byte() }
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool { self.z.descend_until(dst) }
+    fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool { self.z.descend_until(desc_bytes) }
     fn to_next_sibling_byte(&mut self) -> Option<u8> { self.z.to_next_sibling_byte() }
     fn to_prev_sibling_byte(&mut self) -> Option<u8> { self.z.to_prev_sibling_byte() }
     fn ascend(&mut self, steps: usize) -> usize { self.z.ascend(steps) }
@@ -688,7 +688,7 @@ impl<V: Clone + Send + Sync + Unpin, A: Allocator> ZipperMoving for WriteZipperO
     fn descend_to_byte(&mut self, k: u8) -> bool { self.z.descend_to_byte(k) }
     fn descend_indexed_byte(&mut self, child_idx: usize) -> Option<u8> { self.z.descend_indexed_byte(child_idx) }
     fn descend_first_byte(&mut self) -> Option<u8> { self.z.descend_first_byte() }
-    fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool { self.z.descend_until(dst) }
+    fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool { self.z.descend_until(desc_bytes) }
     fn to_next_sibling_byte(&mut self) -> Option<u8> { self.z.to_next_sibling_byte() }
     fn to_prev_sibling_byte(&mut self) -> Option<u8> { self.z.to_prev_sibling_byte() }
     fn ascend(&mut self, steps: usize) -> usize { self.z.ascend(steps) }
@@ -4264,7 +4264,7 @@ mod tests {
 
         assert_eq!(zipper.path(), b"");
         assert_eq!(zipper.val_count(), 2);
-        assert_eq!(zipper.descend_until(None), true);
+        assert_eq!(zipper.descend_until(std::io::sink()), true);
         assert_eq!(zipper.path(), b"arrow");
         assert_eq!(zipper.val_count(), 1);
     }
diff --git a/src/zipper.rs b/src/zipper.rs
diff --git a/src/zipper_head.rs b/src/zipper_head.rs

Original file line number	Diff line number	Diff line change
`@@ -300,9 +300,9 @@ pub fn derive_poly_zipper(input: TokenStream) -> TokenStream {`
`300`	`300`	`}`
`301`	`301`	`}`
`302`	`302`
`303`		`- fn descend_until(&mut self, mut dst_path: Option<&mut Vec<u8>>) -> bool {`
	`303`	`+ fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool {`
`304`	`304`	`match self {`
`305`		`- #(#variant_arms => inner.descend_until(dst_path),)*`
	`305`	`+ #(#variant_arms => inner.descend_until(desc_bytes),)*`
`306`	`306`	`}`
`307`	`307`	`}`
`308`	`308`
Original file line number	Diff line number	Diff line change
`@@ -1879,9 +1879,7 @@ where Storage: AsRef<[u8]>`
`1879`	`1879`	`self.descend_indexed_byte(0)`
`1880`	`1880`	`}`
`1881`	`1881`
`1882`		- /// Descends the zipper's focus until a branch or a value is encountered. Returns `true` if the focus
`1883`		- /// moved otherwise returns `false`
`1884`		`- fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {`
	`1882`	`+ fn descend_until<W: std::io::Write>(&mut self, mut desc_bytes: W) -> bool {`
`1885`	`1883`	`self.trace_pos();`
`1886`	`1884`	`let mut descended = false;`
`1887`	`1885`	`let orig_len = self.path.len();`
`@@ -1925,9 +1923,7 @@ where Storage: AsRef<[u8]>`
`1925`	`1923`	`}`
`1926`	`1924`	`}`
`1927`	`1925`	`}`
`1928`		`- if let Some(dst) = dst {`
`1929`		`- dst.extend_from_slice(&self.path[orig_len..]);`
`1930`		`- }`
	`1926`	`+ let _ = desc_bytes.write_all(&self.path[orig_len..]);`
`1931`	`1927`	`descended`
`1932`	`1928`	`}`
`1933`	`1929`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ impl ZipperMoving for EmptyZipper {`
`45`	`45`	`}`
`46`	`46`	`fn descend_indexed_byte(&mut self, _idx: usize) -> Option<u8> { None }`
`47`	`47`	`fn descend_first_byte(&mut self) -> Option<u8> { None }`
`48`		`- fn descend_until(&mut self, _dst_path: Option<&mut Vec<u8>>) -> bool { false }`
	`48`	`+ fn descend_until<W: std::io::Write>(&mut self, _desc_bytes: W) -> bool { false }`
`49`	`49`	`fn ascend(&mut self, steps: usize) -> usize {`
`50`	`50`	`let old_path_len = self.path.len() - self.path_start_idx;`
`51`	`51`	`if steps > old_path_len {`
Original file line number	Diff line number	Diff line change
`@@ -55,10 +55,8 @@ impl ZipperMoving for FullZipper {`
`55`	`55`	`self.path.push(0);`
`56`	`56`	`Some(0)`
`57`	`57`	`}`
`58`		`- fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {`
`59`		`- self.path.push(0); // not sure?`
`60`		`- if let Some(dst) = dst { dst.push(0) }`
`61`		`- true`
	`58`	`+ fn descend_until<W: std::io::Write>(&mut self, _desc_bytes: W) -> bool {`
	`59`	`+ false`
`62`	`60`	`}`
`63`	`61`	`fn ascend(&mut self, steps: usize) -> usize {`
`64`	`62`	`if steps > self.path.len() {`
Original file line number	Diff line number	Diff line change
`@@ -369,7 +369,7 @@ impl<'prefix, Z> ZipperMoving for PrefixZipper<'prefix, Z>`
`369`	`369`	`self.descend_indexed_byte(0)`
`370`	`370`	`}`
`371`	`371`
`372`		`- fn descend_until(&mut self, dst: Option<&mut Vec<u8>>) -> bool {`
	`372`	`+ fn descend_until<W: std::io::Write>(&mut self, desc_bytes: W) -> bool {`
`373`	`373`	`if self.position.is_invalid() {`
`374`	`374`	`return false;`
`375`	`375`	`}`
`@@ -378,7 +378,7 @@ impl<'prefix, Z> ZipperMoving for PrefixZipper<'prefix, Z>`
`378`	`378`	`self.position = PrefixPos::Source;`
`379`	`379`	`}`
`380`	`380`	`let len_before = self.source.path().len();`
`381`		`- if !self.source.descend_until(dst) {`
	`381`	`+ if !self.source.descend_until(desc_bytes) {`
`382`	`382`	`return false;`
`383`	`383`	`}`
`384`	`384`	`let path = self.source.path();`