Skip to content

Commit

Permalink
Auto merge of #78631 - ssomers:btree-alias_for_underfull, r=Mark-Simu…
Browse files Browse the repository at this point in the history
…lacrum

BTreeMap: fix pointer provenance rules in underfullness

Continuing on #78480, and for readability, and possibly for performance: avoid aliasing when handling underfull nodes, and consolidate the code doing that. In particular:
- Avoid the rather explicit aliasing for internal nodes in `remove_kv_tracking`.
- Climb down to the root to handle underfull nodes using a reborrowed handle, rather than one copied with `ptr::read`, before resuming on the leaf level.
- Integrate the code tracking leaf edge position into the functions performing changes, rather than bolting it on.

r? `@Mark-Simulacrum`
  • Loading branch information
bors committed Nov 16, 2020
2 parents f4d014c + 4cfa5bd commit f5230fb
Show file tree
Hide file tree
Showing 7 changed files with 320 additions and 203 deletions.
11 changes: 3 additions & 8 deletions library/alloc/src/collections/btree/append.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,20 +89,15 @@ impl<K, V> Root<K, V> {
let mut cur_node = self.node_as_mut();
while let Internal(internal) = cur_node.force() {
// Check if right-most child is underfull.
let mut last_edge = internal.last_edge();
let right_child_len = last_edge.reborrow().descend().len();
let mut last_kv = internal.last_kv().consider_for_balancing();
let right_child_len = last_kv.right_child_len();
if right_child_len < MIN_LEN {
// We need to steal.
let mut last_kv = match last_edge.left_kv() {
Ok(left) => left,
Err(_) => unreachable!(),
};
last_kv.bulk_steal_left(MIN_LEN - right_child_len);
last_edge = last_kv.right_edge();
}

// Go further down.
cur_node = last_edge.descend();
cur_node = last_kv.into_right_child();
}
}
}
Expand Down
1 change: 1 addition & 0 deletions library/alloc/src/collections/btree/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use core::ptr;
/// relevant function.
///
/// If a panic occurs in the `change` closure, the entire process will be aborted.
#[allow(dead_code)] // keep as illustration and for future use
#[inline]
pub fn take_mut<T>(v: &mut T, change: impl FnOnce(T) -> T) {
replace(v, |value| (change(value), ()))
Expand Down
14 changes: 0 additions & 14 deletions library/alloc/src/collections/btree/navigate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,20 +362,6 @@ impl<'a, K, V> Handle<NodeRef<marker::ValMut<'a>, K, V, marker::Leaf>, marker::E
}
}

impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge> {
/// Moves the leaf edge handle to the next leaf edge.
///
/// # Safety
/// There must be another KV in the direction travelled.
pub unsafe fn move_next_unchecked(&mut self) {
super::mem::take_mut(self, |leaf_edge| {
let kv = leaf_edge.next_kv();
let kv = unsafe { unwrap_unchecked(kv.ok()) };
kv.next_leaf_edge()
})
}
}

impl<K, V> Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge> {
/// Moves the leaf edge handle to the next leaf edge and returns the key and value
/// in between, deallocating any node left behind while leaving the corresponding
Expand Down
Loading

0 comments on commit f5230fb

Please sign in to comment.