Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak and extend internal BTreeMap documentation, including debug asserts. #67812

Merged
merged 1 commit into from
Jan 4, 2020
Merged

Tweak and extend internal BTreeMap documentation, including debug asserts. #67812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/liballoc/collections/btree/map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
BTreeMap { root: node::Root::shared_empty_root(), length: 0 }
}

/// Clears the map, removing all values.
/// Clears the map, removing all elements.
///
/// # Examples
///
Expand Down Expand Up @@ -2605,7 +2605,7 @@ impl<'a, K: Ord, V> OccupiedEntry<'a, K, V> {

// Handle underflow
let mut cur_node = small_leaf.forget_type();
while cur_node.len() < node::CAPACITY / 2 {
while cur_node.len() < node::MIN_LEN {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this constant really is documentation: no other code uses CAPACITY / 2, but CAPACITY equals 2 * B - 1, so CAPACITY / 2 equals B - 1 which is MIN_LEN.

match handle_underfull_node(cur_node) {
AtRoot => break,
EmptyParent(_) => unreachable!(),
Expand Down
50 changes: 37 additions & 13 deletions src/liballoc/collections/btree/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,15 +308,15 @@ impl<K, V> Root<K, V> {
/// `Leaf`, the `NodeRef` points to a leaf node, when this is `Internal` the
/// `NodeRef` points to an internal node, and when this is `LeafOrInternal` the
/// `NodeRef` could be pointing to either type of node.
/// Note that in case of a leaf node, this might still be the shared root! Only turn
/// this into a `LeafNode` reference if you know it is not a root! Shared references
/// must be dereferencable *for the entire size of their pointee*, so `&InternalNode`
/// pointing to the shared root is UB.
/// Turning this into a `NodeHeader` is always safe.
/// Note that in case of a leaf node, this might still be the shared root!
/// Only turn this into a `LeafNode` reference if you know it is not the shared root!
/// Shared references must be dereferencable *for the entire size of their pointee*,
/// so '&LeafNode` or `&InternalNode` pointing to the shared root is undefined behavior.
/// Turning this into a `NodeHeader` reference is always safe.
pub struct NodeRef<BorrowType, K, V, Type> {
height: usize,
node: NonNull<LeafNode<K, V>>,
// This is null unless the borrow type is `Mut`
// `root` is null unless the borrow type is `Mut`
root: *const Root<K, V>,
_marker: PhantomData<(BorrowType, Type)>,
}
Expand Down Expand Up @@ -370,23 +370,33 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
NodeRef { height: self.height, node: self.node, root: self.root, _marker: PhantomData }
}

/// Assert that this is indeed a proper leaf node, and not the shared root.
/// Exposes the leaf "portion" of any leaf or internal node that is not the shared root.
/// If the node is a leaf, this function simply opens up its data.
/// If the node is an internal node, so not a leaf, it does have all the data a leaf has
/// (header, keys and values), and this function exposes that.
/// See `NodeRef` on why the node may not be a shared root.
unsafe fn as_leaf(&self) -> &LeafNode<K, V> {
debug_assert!(!self.is_shared_root());
self.node.as_ref()
}

fn as_header(&self) -> &NodeHeader<K, V> {
unsafe { &*(self.node.as_ptr() as *const NodeHeader<K, V>) }
}

/// Returns whether the node is the shared, empty root.
pub fn is_shared_root(&self) -> bool {
self.as_header().is_shared_root()
}

/// Borrows a view into the keys stored in the node.
/// Works on all possible nodes, including the shared root.
pub fn keys(&self) -> &[K] {
self.reborrow().into_key_slice()
}

/// Borrows a view into the values stored in the node.
/// The caller must ensure that the node is not the shared root.
fn vals(&self) -> &[V] {
self.reborrow().into_val_slice()
}
Expand Down Expand Up @@ -491,16 +501,24 @@ impl<'a, K, V, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
NodeRef { height: self.height, node: self.node, root: self.root, _marker: PhantomData }
}

/// Exposes the leaf "portion" of any leaf or internal node for writing.
/// If the node is a leaf, this function simply opens up its data.
/// If the node is an internal node, so not a leaf, it does have all the data a leaf has
/// (header, keys and values), and this function exposes that.
///
/// Returns a raw ptr to avoid asserting exclusive access to the entire node.
/// This also implies you can invoke this member on the shared root, but the resulting pointer
/// might not be properly aligned and definitely would not allow accessing keys and values.
fn as_leaf_mut(&mut self) -> *mut LeafNode<K, V> {
// We are mutable, so we cannot be the shared root, so accessing this as a leaf is okay.
self.node.as_ptr()
}

/// The caller must ensure that the node is not the shared root.
fn keys_mut(&mut self) -> &mut [K] {
unsafe { self.reborrow_mut().into_key_slice_mut() }
}

/// The caller must ensure that the node is not the shared root.
fn vals_mut(&mut self) -> &mut [V] {
unsafe { self.reborrow_mut().into_val_slice_mut() }
}
Expand Down Expand Up @@ -551,9 +569,10 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Immut<'a>, K, V, Type> {
}
}

/// The caller must ensure that the node is not the shared root.
fn into_val_slice(self) -> &'a [V] {
debug_assert!(!self.is_shared_root());
// We cannot be the shared root, so `as_leaf` is okay
// We cannot be the shared root, so `as_leaf` is okay.
unsafe { slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len()) }
}

Expand Down Expand Up @@ -587,6 +606,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
}
}

/// The caller must ensure that the node is not the shared root.
fn into_val_slice_mut(mut self) -> &'a mut [V] {
debug_assert!(!self.is_shared_root());
unsafe {
Expand All @@ -597,6 +617,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
}
}

/// The caller must ensure that the node is not the shared root.
fn into_slices_mut(mut self) -> (&'a mut [K], &'a mut [V]) {
debug_assert!(!self.is_shared_root());
// We cannot use the getters here, because calling the second one
Expand Down Expand Up @@ -655,6 +676,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
// Necessary for correctness, but this is an internal module
debug_assert!(edge.height == self.height - 1);
debug_assert!(self.len() < CAPACITY);
debug_assert!(!self.is_shared_root());

let idx = self.len();

Expand Down Expand Up @@ -686,6 +708,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
// Necessary for correctness, but this is an internal module
debug_assert!(edge.height == self.height - 1);
debug_assert!(self.len() < CAPACITY);
debug_assert!(!self.is_shared_root());

unsafe {
slice_insert(self.keys_mut(), 0, key);
Expand Down Expand Up @@ -773,6 +796,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
}
}

/// The caller must ensure that the node is not the shared root.
fn into_kv_pointers_mut(mut self) -> (*mut K, *mut V) {
(self.keys_mut().as_mut_ptr(), self.vals_mut().as_mut_ptr())
}
Expand Down Expand Up @@ -1116,8 +1140,8 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::KV>
}
}

/// Removes the key/value pair pointed to by this handle, returning the edge between the
/// now adjacent key/value pairs to the left and right of this handle.
/// Removes the key/value pair pointed to by this handle and returns it, along with the edge
/// between the now adjacent key/value pairs (if any) to the left and right of this handle.
pub fn remove(
mut self,
) -> (Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>, K, V) {
Expand Down Expand Up @@ -1260,7 +1284,7 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::
}
}

/// This removes a key/value pair from the left child and replaces it with the key/value pair
/// This removes a key/value pair from the left child and places it in the key/value storage
/// pointed to by this handle while pushing the old key/value pair of this handle into the right
/// child.
pub fn steal_left(&mut self) {
Expand All @@ -1277,7 +1301,7 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::
}
}

/// This removes a key/value pair from the right child and replaces it with the key/value pair
/// This removes a key/value pair from the right child and places it in the key/value storage
/// pointed to by this handle while pushing the old key/value pair of this handle into the left
/// child.
pub fn steal_right(&mut self) {
Expand Down