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.

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

BTreeMap: admit the existence of leaf edges in comments #77395

Merged
merged 1 commit into from
Oct 5, 2020
Merged
Changes from all commits
Commits
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
34 changes: 12 additions & 22 deletions library/alloc/src/collections/btree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,8 @@
// struct Node<K, V, height: usize> {
// keys: [K; 2 * B - 1],
// vals: [V; 2 * B - 1],
// edges: if height > 0 {
// [Box<Node<K, V, height - 1>>; 2 * B]
// } else { () },
// parent: Option<NonNull<Node<K, V, height + 1>>>,
// parent_idx: u16,
// edges: [if height > 0 { Box<Node<K, V, height - 1>> } else { () }; 2 * B],
// parent: Option<(NonNull<Node<K, V, height + 1>>, u16)>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

parent is unrelated to leaf edges, but the fact that parent_idx is not part of the option is not "the ideal", it's probably the only way we can have the compiler merge the two u16 fields. I was going to undo that but it's in here now,

// len: u16,
// }
// ```
Expand All @@ -28,8 +25,8 @@
//
// - Trees must have uniform depth/height. This means that every path down to a leaf from a
// given node has exactly the same length.
// - A node of length `n` has `n` keys, `n` values, and (in an internal node) `n + 1` edges.
// This implies that even an empty internal node has at least one edge.
// - A node of length `n` has `n` keys, `n` values, and `n + 1` edges.
// This implies that even an empty node has at least one edge.

use core::cmp::Ordering;
use core::marker::PhantomData;
Expand Down Expand Up @@ -298,9 +295,8 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
}

impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
/// Finds the length of the node. This is the number of keys or values. In an
/// internal node, the number of edges is `len() + 1`.
/// For any node, the number of possible edge handles is also `len() + 1`.
/// Finds the length of the node. This is the number of keys or values.
/// The number of edges is `len() + 1`.
/// Note that, despite being safe, calling this function can have the side effect
/// of invalidating mutable references that unsafe code has created.
pub fn len(&self) -> usize {
Expand All @@ -321,9 +317,6 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
}

/// Exposes the leaf portion of any leaf or internal node.
/// 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.
Comment on lines -324 to -326
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There hasn't been a separate header for quite a while, so pruning my own comment.

///
/// Returns a raw ptr to avoid invalidating other references to this node,
/// which is possible when BorrowType is marker::ValMut.
Expand Down Expand Up @@ -471,9 +464,6 @@ impl<'a, K, V, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
}

/// 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.
///
/// We don't need to return a raw ptr because we have unique access to the entire node.
fn as_leaf_mut(&mut self) -> &'a mut LeafNode<K, V> {
Expand Down Expand Up @@ -679,9 +669,9 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
}

impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
/// Removes a key/value pair from the end of this node and returns the pair.
/// If this is an internal node, also removes the edge that was to the right
/// of that pair and returns the orphaned node that this edge owned.
/// Removes a key/value pair from the end of the node and returns the pair.
/// Also removes the edge that was to the right of that pair and, if the node
/// is internal, returns the orphaned subtree that this edge owned.
fn pop(&mut self) -> (K, V, Option<Root<K, V>>) {
debug_assert!(self.len() > 0);

Expand All @@ -705,9 +695,9 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
}
}

/// Removes a key/value pair from the beginning of this node and returns the pair.
/// If this is an internal node, also removes the edge that was to the left
/// of that pair and returns the orphaned node that this edge owned.
/// Removes a key/value pair from the beginning of the node and returns the pair.
/// Also removes the edge that was to the left of that pair and, if the node is
/// internal, returns the orphaned subtree that this edge owned.
fn pop_front(&mut self) -> (K, V, Option<Root<K, V>>) {
debug_assert!(self.len() > 0);

Expand Down