From 7cad30941009f6c8c61cff33c405b1c22382add0 Mon Sep 17 00:00:00 2001 From: Igor Malovitsa Date: Wed, 16 Sep 2026 23:22:36 +0000 Subject: [PATCH 1/2] Fix remove_unmasked_branches at a dangling path A dangling path is a link to the empty node, which get_child_mut won't return, so the zipper passed the stub's own key to the parent node as a branch and tripped a LineListNode debug assert. There is nothing below a dangling path to filter, so do nothing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019R2H8fnco29asY2v3TPbtF --- src/write_zipper.rs | 49 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/src/write_zipper.rs b/src/write_zipper.rs index 0892e475..90bd4eac 100644 --- a/src/write_zipper.rs +++ b/src/write_zipper.rs @@ -2269,9 +2269,22 @@ impl <'a, 'path, V: Clone + Send + Sync + Unpin, A: Allocator + 'a> WriteZipperC } /// See [WriteZipper::remove_unmasked_branches] pub fn remove_unmasked_branches(&mut self, mask: ByteMask, prune: bool) { - let mut focus_node = self.focus_stack.top_mut().unwrap(); let node_key = self.key.node_key(); - if node_key.len() > 0 { + + //A dangling focus has no branches to filter; `get_child_mut` won't descend into it + let below_dangling_stub = node_key.len() > 0 + && match self.focus_stack.top() { + Some(focus_node) => match focus_node.node_get_child(node_key) { + Some((_consumed_bytes, child_node)) => child_node.is_empty(), + None => false + }, + None => false + }; + + let mut focus_node = self.focus_stack.top_mut().unwrap(); + if below_dangling_stub { + //Nothing to do + } else if node_key.len() > 0 { match focus_node.node_get_child_mut(node_key) { Some((consumed_bytes, child_node)) => { if node_key.len() >= consumed_bytes { @@ -6672,4 +6685,36 @@ mod tests { } assert_eq!(keys(&m), ["cx", "cy", "d"]); } + + /// `remove_unmasked_branches` at or below a dangling path does nothing + #[test] + fn write_zipper_test_remove_unmasked_branches_dangling_focus() { + //List node root: onward link at [0], dangling stub at [1, 0] + let mut map = PathMap::::new(); + map.set_val_at([0u8], 0); + map.set_val_at([0u8, 0], 1); + assert!(map.create_path([1u8, 0])); + + //Focus exactly on the dangling path + let mut wz = map.write_zipper_at_path(&[1u8, 0]); + assert!(wz.path_exists()); + wz.remove_unmasked_branches(ByteMask::EMPTY, false); + assert!(wz.path_exists()); + drop(wz); + + //Focus below the dangling path + let mut wz = map.write_zipper_at_path(&[1u8, 0, 7]); + wz.remove_unmasked_branches(ByteMask::EMPTY, false); + drop(wz); + + //Nothing may have changed + assert_eq!(map.val_at([0u8]), Some(&0)); + assert_eq!(map.val_at([0u8, 0]), Some(&1)); + assert_eq!(map.val_at([1u8, 0]), None); + let mut rz = map.read_zipper(); + rz.descend_to([1u8, 0]); + assert!(rz.path_exists()); + drop(rz); + assert_eq!(map.val_count(), 2); + } } From 36c5726b477c9e90af06f3d590f6bc38c47dd6b1 Mon Sep 17 00:00:00 2001 From: Luke Peterson Date: Sat, 19 Sep 2026 06:43:40 -0600 Subject: [PATCH 2/2] Making the logic around remove_unmasked_branches less convoluted than what was in the PR. --- src/line_list_node.rs | 7 ++++--- src/write_zipper.rs | 23 +++++------------------ 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/src/line_list_node.rs b/src/line_list_node.rs index e0aa5b7a..4eb80d1e 100644 --- a/src/line_list_node.rs +++ b/src/line_list_node.rs @@ -1925,15 +1925,16 @@ impl TrieNode for LineListNode remove_0 = !mask.test_bit(key0[key_len]); } else { //We can only get here if key0 == key, and the calling code should have descend - // through this node if that key specifies an onward link - debug_assert!(!self.is_child_ptr::<0>()); + // through this node if that key specifies a non-dangling onward link + debug_assert!(!self.is_used_child_0() || unsafe{ self.child_in_slot::<0>().is_empty() }); } } if starts_with(key1, key) { if key1.len() > key_len { remove_1 = !mask.test_bit(key1[key_len]); } else { - debug_assert!(!self.is_child_ptr::<1>()); //See comment above + //See comment above + debug_assert!(!self.is_used_child_1() || unsafe{ self.child_in_slot::<1>().is_empty() }); } } self.remove_subtries(remove_0, remove_1, key0_starts_with, prune, key.len()); diff --git a/src/write_zipper.rs b/src/write_zipper.rs index b9d2e8a6..30f3b793 100644 --- a/src/write_zipper.rs +++ b/src/write_zipper.rs @@ -2270,30 +2270,17 @@ impl <'a, 'path, V: Clone + Send + Sync + Unpin, A: Allocator + 'a> WriteZipperC /// See [WriteZipper::remove_unmasked_branches] pub fn remove_unmasked_branches(&mut self, mask: ByteMask, prune: bool) { let node_key = self.key.node_key(); - - //A dangling focus has no branches to filter; `get_child_mut` won't descend into it - let below_dangling_stub = node_key.len() > 0 - && match self.focus_stack.top() { - Some(focus_node) => match focus_node.node_get_child(node_key) { - Some((_consumed_bytes, child_node)) => child_node.is_empty(), - None => false - }, - None => false - }; - let mut focus_node = self.focus_stack.top_mut().unwrap(); - if below_dangling_stub { - //Nothing to do - } else if node_key.len() > 0 { + if node_key.len() > 0 { match focus_node.node_get_child_mut(node_key) { Some((consumed_bytes, child_node)) => { - if node_key.len() >= consumed_bytes { + if node_key.len() >= consumed_bytes && !child_node.is_empty() { child_node.make_mut().node_remove_unmasked_branches(&node_key[consumed_bytes..], mask, prune); if child_node.as_tagged().node_is_empty() { focus_node.node_remove_all_branches(&node_key[..consumed_bytes], prune); } } else { - //Zipper is positioned at non-existent node. Removing anything from nothing is nothing + //Zipper is positioned at non-existent or dangling node. Removing anything from nothing is nothing } }, None => { @@ -3751,7 +3738,7 @@ mod tests { assert_eq!(*wz.get_val_or_set_mut_with(|| 3), 3); assert_eq!(wz.val(), Some(&3)); } - assert_eq!(m0.get_val_at(&[0u8, 0, 0]), Some(&3)); + assert_eq!(m0.val_at(&[0u8, 0, 0]), Some(&3)); assert_eq!(m0.val_count(), 1); // through meet_into with prune @@ -3766,7 +3753,7 @@ mod tests { assert_eq!(*wz.get_val_or_set_mut_with(|| 3), 3); assert_eq!(wz.val(), Some(&3)); } - assert_eq!(m0.get_val_at(&[0u8, 0, 0]), Some(&3)); + assert_eq!(m0.val_at(&[0u8, 0, 0]), Some(&3)); }