OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #79376 - ssomers:btree_choose_parent_kv, r=Mark-Simulacrum

Browse Source 
BTreeMap: clarify comments and panics around choose_parent_kv

Fixes a lie in recent code: `unreachable!("empty non-root node")` should shout "empty internal node", but it might as well be good and keep quiet

r? `@Mark-Simulacrum`
This commit is contained in:
bors 2020-12-13 14:42:37 +00:00
commit cbab347e68
2 changed files with 14 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
library/alloc/src/collections/btree/node.rs
View File

@ -1288,10 +1288,12 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
/// Chooses a balancing context involving the node as a child, thus between
/// the KV immediately to the left or to the right in the parent node.
/// Returns an `Err` if there is no parent.
/// Panics if the parent is empty.
///
/// This method optimizes for a node that has fewer elements than its left
/// and right siblings, if they exist, by preferring the left parent KV.
/// Merging with the left sibling is faster, since we only need to move
/// Prefers the left side, to be optimal if the given node is somehow
/// underfull, meaning here only that it has fewer elements than its left
/// sibling and than its right sibling, if they exist. In that case,
/// merging with the left sibling is faster, since we only need to move
/// the node's N elements, instead of shifting them to the right and moving
/// more than N elements in front. Stealing from the left sibling is also
/// typically faster, since we only need to shift the node's N elements to
@ -1299,19 +1301,19 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
/// the left.
pub fn choose_parent_kv(self) -> Result<LeftOrRight<BalancingContext<'a, K, V>>, Self> {
match unsafe { ptr::read(&self) }.ascend() {
Ok(parent) => match parent.left_kv() {
Ok(parent_edge) => match parent_edge.left_kv() {
Ok(left_parent_kv) => Ok(LeftOrRight::Left(BalancingContext {
parent: unsafe { ptr::read(&left_parent_kv) },
left_child: left_parent_kv.left_edge().descend(),
right_child: self,
})),
Err(parent) => match parent.right_kv() {
Err(parent_edge) => match parent_edge.right_kv() {
Ok(right_parent_kv) => Ok(LeftOrRight::Right(BalancingContext {
parent: unsafe { ptr::read(&right_parent_kv) },
left_child: self,
right_child: right_parent_kv.right_edge().descend(),
})),
Err(_) => unreachable!("empty non-root node"),
Err(_) => unreachable!("empty internal node"),
},
},
Err(root) => Err(root),

12
library/alloc/src/collections/btree/remove.rs
View File

@ -54,12 +54,12 @@ impl<'a, K: 'a, V: 'a> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, mark
pos = unsafe { new_pos.cast_to_leaf_unchecked() };
// Only if we merged, the parent (if any) has shrunk, but skipping
// the following step does not pay off in benchmarks.
// the following step otherwise does not pay off in benchmarks.
//
// SAFETY: We won't destroy or rearrange the leaf where `pos` is at
// by handling its parent recursively; at worst we will destroy or
// rearrange the parent through the grandparent, thus change the
// leaf's parent pointer.
// link to the parent inside the leaf.
if let Ok(parent) = unsafe { pos.reborrow_mut() }.into_node().ascend() {
parent.into_node().handle_shrunk_node_recursively(handle_emptied_internal_root);
}
@ -90,8 +90,8 @@ impl<'a, K: 'a, V: 'a> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>,
}
impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
/// Stocks up a possibly underfull internal node, recursively.
/// Climbs up until it reaches an ancestor that has elements to spare or the root.
/// Stocks up a possibly underfull internal node and its ancestors,
/// until it reaches an ancestor that has elements to spare or is the root.
fn handle_shrunk_node_recursively<F: FnOnce()>(mut self, handle_emptied_internal_root: F) {
loop {
self = match self.len() {
@ -122,7 +122,7 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
) -> Option<NodeRef<marker::Mut<'a>, K, V, marker::Internal>> {
match self.forget_type().choose_parent_kv() {
Ok(Left(left_parent_kv)) => {
debug_assert!(left_parent_kv.right_child_len() == MIN_LEN - 1);
debug_assert_eq!(left_parent_kv.right_child_len(), MIN_LEN - 1);
if left_parent_kv.can_merge() {
let pos = left_parent_kv.merge(None);
let parent_edge = unsafe { unwrap_unchecked(pos.into_node().ascend().ok()) };
@ -134,7 +134,7 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
}
}
Ok(Right(right_parent_kv)) => {
debug_assert!(right_parent_kv.left_child_len() == MIN_LEN - 1);
debug_assert_eq!(right_parent_kv.left_child_len(), MIN_LEN - 1);
if right_parent_kv.can_merge() {
let pos = right_parent_kv.merge(None);
let parent_edge = unsafe { unwrap_unchecked(pos.into_node().ascend().ok()) };