Providing default implementation for ZipperMoving::to_prev_sibling_byte, which fixes the remaining WriteZipper ZipperMoving test

Author: luketpeterson

src/utils.rs

@@ -8,6 +8,7 @@ pub struct ByteMask(pub [u64; 4]);
 
 impl ByteMask {
     pub const EMPTY: ByteMask = Self(empty_mask());
+    pub const FULL: ByteMask = Self([!0u64; 4]);
 
     /// Create a new empty ByteMask
     #[inline]
@@ -90,7 +91,7 @@ impl ByteMask {
     }
 
     /// Returns the bit in the mask corresponding to the next highest bit above `byte`, or `None`
-    /// if `byte` was the highest set bit in the mask
+    /// if `byte` was at or above the highest set bit in the mask
     pub fn next_bit(&self, byte: u8) -> Option<u8> {
         if byte == 255 {
             return None
@@ -130,6 +131,48 @@ impl ByteMask {
         }
         None
     }
+
+    /// Returns the bit in the mask corresponding to the previous bit below `byte`, or `None`
+    /// if `byte` was at or below the lowest set bit in the mask
+    pub fn prev_bit(&self, byte: u8) -> Option<u8> {
+        if byte == 0 {
+            return None
+        }
+        let byte = byte - 1;
+        let word_idx = byte >> 6;
+        let mod_idx = byte & 0x3F;
+        let mut mask = !0u64 >> (63 - mod_idx);
+        if word_idx == 3 {
+            let cnt = (self.0[3] & mask).leading_zeros() as u8;
+            if cnt < 64 {
+                return Some(255 - cnt)
+            }
+            mask = !0u64;
+        }
+        if word_idx > 1 {
+            let cnt = (self.0[2] & mask).leading_zeros() as u8;
+            if cnt < 64 {
+                return Some(191 - cnt)
+            }
+            if word_idx == 2 {
+                mask = !0u64;
+            }
+        }
+        if word_idx > 0 {
+            let cnt = (self.0[1] & mask).leading_zeros() as u8;
+            if cnt < 64 {
+                return Some(127 - cnt)
+            }
+            if word_idx == 1 {
+                mask = !0u64;
+            }
+        }
+        let cnt = (self.0[0] & mask).leading_zeros() as u8;
+        if cnt < 64 {
+            return Some(63 - cnt)
+        }
+        None
+    }
 }
 
 impl core::fmt::Debug for ByteMask {
@@ -548,22 +591,41 @@ fn bit_utils_test() {
 
 #[test]
 fn next_bit_test() {
-    let test_mask = ByteMask::from([
-        0b1010010010010010010010000000000000000000000000000000000000010101u64,
-        0b0000000000000000000000000000000000000000100000000000000000000000u64,
-        0b0000000000000000000000000000000000000000000000000000000000000000u64,
-        0b1000000000000000000000000000010000000000000000000000000000000001u64,
-    ]);
-    let set_bits: Vec<u8> = (0..=255).into_iter().filter(|i| test_mask.test_bit(*i)).collect();
-
-    let mut i = 0;
-    let mut cnt = test_mask.test_bit(0) as usize;
-    while let Some(next_bit) = test_mask.next_bit(i) {
-        assert!(test_mask.test_bit(next_bit));
-        i = next_bit;
-        cnt += 1;
-    }
-    assert_eq!(cnt, set_bits.len());
+    fn do_test(test_mask: ByteMask) {
+        let set_bits: Vec<u8> = (0..=255).into_iter().filter(|i| test_mask.test_bit(*i)).collect();
+
+        let mut i = 0;
+        let mut cnt = test_mask.test_bit(0) as usize;
+        while let Some(next_bit) = test_mask.next_bit(i) {
+            assert!(test_mask.test_bit(next_bit));
+            i = next_bit;
+            cnt += 1;
+        }
+        assert_eq!(cnt, set_bits.len());
+
+        let mut i = 255;
+        let mut cnt = test_mask.test_bit(255) as usize;
+        while let Some(prev_bit) = test_mask.prev_bit(i) {
+            assert!(test_mask.test_bit(prev_bit));
+            i = prev_bit;
+            cnt += 1;
+        }
+        assert_eq!(cnt, set_bits.len());
+    }
+
+    // do_test(ByteMask::from([
+    //     0b1010010010010010010010000000000000000000000000000000000000010101u64,
+    //     0b0000000000000000000000000000000000000000100000000000000000000000u64,
+    //     0b0000000000000000000000000000000000000000000000000000000000000000u64,
+    //     0b1001000000000000000000000000000000000000000000000000000000000001u64,
+    // ]));
+    // do_test(ByteMask::from([
+    //     0b0000000000000000000000000000000000000000000000000000000000000000u64,
+    //     0b0000000000000000000000000000000000000000100000000000000000000000u64,
+    //     0b0000000000000000000000000000000000000000000000000000000000000000u64,
+    //     0b1001000000000000000000000000000000000000000000000000000000000001u64,
+    // ]));
+    do_test(ByteMask::from(ByteMask::FULL));
 }
 
 #[test]

src/write_zipper.rs

@@ -813,10 +813,6 @@ impl<V: Clone + Send + Sync + Unpin> ZipperMoving for WriteZipperCore<'_, '_, V>
         }
     }
 
-    fn to_prev_sibling_byte(&mut self) -> bool {
-        unimplemented!()
-    }
-
     fn ascend(&mut self, mut steps: usize) -> bool {
         loop {
             if self.key.node_key().len() == 0 {

src/zipper.rs

@@ -120,7 +120,11 @@ pub trait ZipperMoving: Zipper + ZipperMovingPriv {
     }
 
     /// Resets the zipper's focus back to its root
-    fn reset(&mut self);
+    fn reset(&mut self) {
+        while !self.at_root() {
+            self.ascend_byte();
+        }
+    }
 
     /// Returns the path from the zipper's root to the current focus
     fn path(&self) -> &[u8];
@@ -259,11 +263,15 @@ pub trait ZipperMoving: Zipper + ZipperMovingPriv {
 
     /// Ascends the zipper to the nearest upstream branch point or value.  Returns `true` if the zipper
     /// focus moved upwards, otherwise returns `false` if the zipper was already at the root
+    ///
+    /// NOTE: A default implementation could be provided, but all current zippers have more optimal native implementations.
     fn ascend_until(&mut self) -> bool;
 
     /// Ascends the zipper to the nearest upstream branch point, skipping over values along the way.  Returns
     /// `true` if the zipper focus moved upwards, otherwise returns `false` if the zipper was already at the
     /// root
+    ///
+    /// NOTE: A default implementation could be provided, but all current zippers have more optimal native implementations.
     fn ascend_until_branch(&mut self) -> bool;
 
 //GOAT, I think this method ought to behave like the other two, and ascend above the current value, instead
@@ -337,7 +345,28 @@ pub trait ZipperMoving: Zipper + ZipperMovingPriv {
     ///
     /// This method is equivalent to calling [Self::ascend] with `1`, followed by [Self::descend_indexed_branch]
     /// where the index passed is 1 less than the index of the current focus position.
-    fn to_prev_sibling_byte(&mut self) -> bool;
+    fn to_prev_sibling_byte(&mut self) -> bool {
+        let cur_byte = match self.path().last() {
+            Some(byte) => *byte,
+            None => return false
+        };
+        if !self.ascend_byte() {
+            return false
+        }
+        let mask = self.child_mask();
+        match mask.prev_bit(cur_byte) {
+            Some(byte) => {
+                let descended = self.descend_to_byte(byte);
+                debug_assert!(descended);
+                true
+            },
+            None => {
+                let descended = self.descend_to_byte(cur_byte);
+                debug_assert!(descended);
+                false
+            }
+        }
+    }
 
     /// Advances the zipper to visit every existing path within the trie in a depth-first order
     ///