Fix to_prev_sibling_byte losing the child of a list node location

src/arena_compact.rs

@@ -2771,7 +2771,48 @@ where Storage: AsRef<[u8]>
         descended
     }
 
+    #[cold]
+    #[inline(never)]
+    fn to_sibling_from_nonexistent_path(&mut self, next: bool) -> Option<u8> {
+        // A sibling can only exist when the final byte alone is nonexistent and
+        // its parent is therefore an existing trie position.
+        if self.invalid != 1 || self.at_root() {
+            return None;
+        }
+
+        let cur_byte = *self.path.last().unwrap();
+        let (sibling_byte, sibling_idx) = match &self.cur_node {
+            Node::Line(line) => {
+                let frame = self.stack.last().unwrap();
+                let byte = *self.tree.get_line(line.path).get(frame.node_depth)?;
+                if (next && byte > cur_byte) || (!next && byte < cur_byte) {
+                    (byte, 0)
+                } else {
+                    return None;
+                }
+            }
+            Node::Branch(node) => {
+                let byte = if next {
+                    node.bytemask.next_bit(cur_byte)
+                } else {
+                    node.bytemask.prev_bit(cur_byte)
+                }?;
+                (byte, node.bytemask.index_of(byte) as usize)
+            }
+        };
+
+        self.path.pop();
+        self.invalid = 0;
+        let result = self.descend_indexed_byte(sibling_idx);
+        debug_assert_eq!(result, Some(sibling_byte));
+        result
+    }
+
     fn to_sibling(&mut self, next: bool) -> Option<u8> {
+        if self.invalid > 0 {
+            return self.to_sibling_from_nonexistent_path(next);
+        }
+
         let top_frame = self.stack.last().unwrap();
         if self.stack.len() <= 1 || top_frame.node_depth > 0 {
             // can't move to sibling at root, or along the path

src/line_list_node.rs

@@ -2482,21 +2482,23 @@ impl<V: Clone + Send + Sync, A: Allocator> TrieNode<V, A> for LineListNode<V, A>
                             && *byte < key[last_key_byte_idx]
                     })
                 };
-                let (sibling_byte, slot) = match key_byte(key1) {
-                    Some(byte) => (byte, 1),
-                    None => match key_byte(key0) {
-                        Some(byte) => (byte, 0),
-                        None => return (None, None),
-                    },
+                let sibling_byte = match key_byte(key1).or_else(|| key_byte(key0)) {
+                    Some(byte) => byte,
+                    None => return (None, None),
                 };
-                let sib_node = match slot {
-                    0 if key0.len() == key.len() && self.is_child_ptr::<0>() => {
-                        Some(unsafe { self.child_in_slot::<0>().as_tagged() })
-                    },
-                    1 if key1.len() == key.len() && self.is_child_ptr::<1>() => {
-                        Some(unsafe { self.child_in_slot::<1>().as_tagged() })
-                    },
-                    _ => None,
+                //A value+child location is two same-key slots, so check both for the child
+                let holds_child = |candidate: &[u8], slot: usize| {
+                    candidate.len() == key.len()
+                        && candidate[last_key_byte_idx] == sibling_byte
+                        && candidate[..last_key_byte_idx] == common_key[..]
+                        && if slot == 0 { self.is_child_ptr::<0>() } else { self.is_child_ptr::<1>() }
+                };
+                let sib_node = if holds_child(key1, 1) {
+                    Some(unsafe { self.child_in_slot::<1>().as_tagged() })
+                } else if holds_child(key0, 0) {
+                    Some(unsafe { self.child_in_slot::<0>().as_tagged() })
+                } else {
+                    None
                 };
                 (Some(sibling_byte), sib_node)
             }

src/zipper.rs

@@ -3642,6 +3642,18 @@ pub(crate) mod zipper_moving_tests {
                     crate::zipper::zipper_moving_tests::run_test(&mut temp_store, $make_z, &[], crate::zipper::zipper_moving_tests::zipper_dangling_descend_test)
                 }
 
+                #[test]
+                fn [<$z_name _prev_sibling_sparse_paths>]() {
+                    let mut temp_store = $read_keys(crate::zipper::zipper_moving_tests::ZIPPER_PREV_SIBLING_SPARSE_PATHS_KEYS);
+                    crate::zipper::zipper_moving_tests::run_test(&mut temp_store, $make_z, &[], crate::zipper::zipper_moving_tests::prev_sibling_sparse_paths)
+                }
+
+                #[test]
+                fn [<$z_name _prev_sibling_value_and_child_location>]() {
+                    let mut temp_store = $read_keys(crate::zipper::zipper_moving_tests::ZIPPER_PREV_SIBLING_VALUE_AND_CHILD_LOCATION_KEYS);
+                    crate::zipper::zipper_moving_tests::run_test(&mut temp_store, $make_z, &[], crate::zipper::zipper_moving_tests::prev_sibling_value_and_child_location)
+                }
+
                 #[test]
                 fn [<$z_name _zipper_indexed_bytes_test1>]() {
                     let mut temp_store = $read_keys(crate::zipper::zipper_moving_tests::ZIPPER_INDEXED_BYTE_TEST1_KEYS);
@@ -3816,6 +3828,40 @@ pub(crate) mod zipper_moving_tests {
     /// from https://en.wikipedia.org/wiki/Radix_tree#/media/File:Patricia_trie.svg
     pub const ZIPPER_MOVING_BASIC_TEST_KEYS: &[&[u8]] = &[b"romane", b"romanus", b"romulus", b"rubens", b"ruber", b"rubicon", b"rubicundus", b"rom'i"];
 
+    pub const ZIPPER_PREV_SIBLING_SPARSE_PATHS_KEYS: &[&[u8]] = &[b"AA", b"CCC"];
+
+    pub const ZIPPER_PREV_SIBLING_VALUE_AND_CHILD_LOCATION_KEYS: &[&[u8]] = &[&[2, 0], &[2, 1], &[2]];
+
+    pub fn prev_sibling_sparse_paths<Z: ZipperMoving + ZipperPath>(mut zipper: Z) {
+        // An absent root child can have a preceding sibling, while positions
+        // inside a line have no preceding sibling at their respective depths.
+        zipper.descend_to(b"C");
+        assert_eq!(zipper.to_prev_sibling_byte(), Some(b'A'));
+        assert_eq!(zipper.path(), b"A");
+        zipper.reset();
+        zipper.descend_to(b"CC");
+        assert_eq!(zipper.to_prev_sibling_byte(), None);
+        zipper.reset();
+        zipper.descend_to(b"CCC");
+        assert_eq!(zipper.to_prev_sibling_byte(), None);
+    }
+
+    /// `to_prev_sibling_byte` onto a location holding both a value and a child subtree.
+    pub fn prev_sibling_value_and_child_location<Z: ZipperMoving + ZipperPath>(mut zipper: Z) {
+        for start in [&[3u8][..], &[9u8]] {
+            zipper.reset();
+            zipper.descend_to(start);
+            assert!(!zipper.path_exists());
+            assert_eq!(zipper.to_prev_sibling_byte(), Some(2), "from {start:?}");
+            assert_eq!(zipper.path(), &[2u8]);
+            assert!(zipper.is_val());
+            assert_eq!(zipper.child_count(), 2, "from {start:?}");
+            assert_eq!(zipper.child_mask().iter().collect::<Vec<_>>(), vec![0u8, 1]);
+            assert_eq!(zipper.descend_first_byte(), Some(0), "from {start:?}");
+            assert_eq!(zipper.path(), &[2u8, 0]);
+        }
+    }
+
     pub fn zipper_moving_basic_test<Z: ZipperMoving + ZipperPath>(mut zipper: Z) {
         fn assert_in_list(val: &[u8], list: &[&[u8]]) {
             for test_val in list {