From e61f126fb48f3f21cc019b7dc005b5287cef79e5 Mon Sep 17 00:00:00 2001
From: Mark Rousskov <mark.simulacrum@gmail.com>
Date: Wed, 18 Mar 2020 10:25:46 -0400
Subject: [PATCH 1/7] Replace shared root with optional root
This simplifies the node manipulation, as we can (in later commits) always know
when traversing nodes that we are not in a shared root.
---
src/liballoc/collections/btree/map.rs | 189 +++++++++++++++-----------
1 file changed, 112 insertions(+), 77 deletions(-)
diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs
index 9da324ba2d4..478fa8e1526 100644
--- a/src/liballoc/collections/btree/map.rs
+++ b/src/liballoc/collections/btree/map.rs
@@ -122,7 +122,7 @@ use UnderflowResult::*;
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub struct BTreeMap<K, V> {
- root: node::Root<K, V>,
+ root: Option<node::Root<K, V>>,
length: usize,
}
@@ -147,10 +147,11 @@ impl<K: Clone, V: Clone> Clone for BTreeMap<K, V> {
{
match node.force() {
Leaf(leaf) => {
- let mut out_tree = BTreeMap { root: node::Root::new_leaf(), length: 0 };
+ let mut out_tree = BTreeMap { root: Some(node::Root::new_leaf()), length: 0 };
{
- let mut out_node = match out_tree.root.as_mut().force() {
+ let root = out_tree.root.as_mut().unwrap();
+ let mut out_node = match root.as_mut().force() {
Leaf(leaf) => leaf,
Internal(_) => unreachable!(),
};
@@ -170,8 +171,13 @@ impl<K: Clone, V: Clone> Clone for BTreeMap<K, V> {
Internal(internal) => {
let mut out_tree = clone_subtree(internal.first_edge().descend());
+ // Cannot call ensure_root_is_owned() because lacking K: Ord
+ if out_tree.root.is_none() {
+ out_tree.root = Some(node::Root::new_leaf());
+ }
+
{
- let mut out_node = out_tree.root.push_level();
+ let mut out_node = out_tree.root.as_mut().unwrap().push_level();
let mut in_edge = internal.first_edge();
while let Ok(kv) = in_edge.right_kv() {
let (k, v) = kv.into_kv();
@@ -190,7 +196,7 @@ impl<K: Clone, V: Clone> Clone for BTreeMap<K, V> {
(root, length)
};
- out_node.push(k, v, subroot);
+ out_node.push(k, v, subroot.unwrap_or_else(|| node::Root::new_leaf()));
out_tree.length += 1 + sublength;
}
}
@@ -203,9 +209,9 @@ impl<K: Clone, V: Clone> Clone for BTreeMap<K, V> {
if self.is_empty() {
// Ideally we'd call `BTreeMap::new` here, but that has the `K:
// Ord` constraint, which this method lacks.
- BTreeMap { root: node::Root::shared_empty_root(), length: 0 }
+ BTreeMap { root: None, length: 0 }
} else {
- clone_subtree(self.root.as_ref())
+ clone_subtree(self.root.as_ref().unwrap().as_ref())
}
}
@@ -271,14 +277,14 @@ where
type Key = K;
fn get(&self, key: &Q) -> Option<&K> {
- match search::search_tree(self.root.as_ref(), key) {
+ match search::search_tree(self.root.as_ref()?.as_ref(), key) {
Found(handle) => Some(handle.into_kv().0),
GoDown(_) => None,
}
}
fn take(&mut self, key: &Q) -> Option<K> {
- match search::search_tree(self.root.as_mut(), key) {
+ match search::search_tree(self.root.as_mut()?.as_mut(), key) {
Found(handle) => Some(
OccupiedEntry { handle, length: &mut self.length, _marker: PhantomData }
.remove_kv()
@@ -290,7 +296,7 @@ where
fn replace(&mut self, key: K) -> Option<K> {
self.ensure_root_is_owned();
- match search::search_tree::<marker::Mut<'_>, K, (), K>(self.root.as_mut(), &key) {
+ match search::search_tree::<marker::Mut<'_>, K, (), K>(self.root.as_mut()?.as_mut(), &key) {
Found(handle) => Some(mem::replace(handle.into_kv_mut().0, key)),
GoDown(handle) => {
VacantEntry { key, handle, length: &mut self.length, _marker: PhantomData }
@@ -344,15 +350,18 @@ pub struct IterMut<'a, K: 'a, V: 'a> {
/// [`BTreeMap`]: struct.BTreeMap.html
#[stable(feature = "rust1", since = "1.0.0")]
pub struct IntoIter<K, V> {
- front: Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge>,
- back: Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge>,
+ front: Option<Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge>>,
+ back: Option<Handle<NodeRef<marker::Owned, K, V, marker::Leaf>, marker::Edge>>,
length: usize,
}
#[stable(feature = "collection_debug", since = "1.17.0")]
impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for IntoIter<K, V> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- let range = Range { front: self.front.reborrow(), back: self.back.reborrow() };
+ let range = Range {
+ front: self.front.as_ref().map(|f| f.reborrow()),
+ back: self.back.as_ref().map(|b| b.reborrow()),
+ };
f.debug_list().entries(range).finish()
}
}
@@ -417,8 +426,8 @@ pub struct ValuesMut<'a, K: 'a, V: 'a> {
/// [`BTreeMap`]: struct.BTreeMap.html
#[stable(feature = "btree_range", since = "1.17.0")]
pub struct Range<'a, K: 'a, V: 'a> {
- front: Handle<NodeRef<marker::Immut<'a>, K, V, marker::Leaf>, marker::Edge>,
- back: Handle<NodeRef<marker::Immut<'a>, K, V, marker::Leaf>, marker::Edge>,
+ front: Option<Handle<NodeRef<marker::Immut<'a>, K, V, marker::Leaf>, marker::Edge>>,
+ back: Option<Handle<NodeRef<marker::Immut<'a>, K, V, marker::Leaf>, marker::Edge>>,
}
#[stable(feature = "collection_debug", since = "1.17.0")]
@@ -437,8 +446,8 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for Range<'_, K, V> {
/// [`BTreeMap`]: struct.BTreeMap.html
#[stable(feature = "btree_range", since = "1.17.0")]
pub struct RangeMut<'a, K: 'a, V: 'a> {
- front: Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>,
- back: Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>,
+ front: Option<Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>>,
+ back: Option<Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>>,
// Be invariant in `K` and `V`
_marker: PhantomData<&'a mut (K, V)>,
@@ -447,7 +456,10 @@ pub struct RangeMut<'a, K: 'a, V: 'a> {
#[stable(feature = "collection_debug", since = "1.17.0")]
impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for RangeMut<'_, K, V> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- let range = Range { front: self.front.reborrow(), back: self.back.reborrow() };
+ let range = Range {
+ front: self.front.as_ref().map(|f| f.reborrow()),
+ back: self.back.as_ref().map(|b| b.reborrow()),
+ };
f.debug_list().entries(range).finish()
}
}
@@ -544,7 +556,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn new() -> BTreeMap<K, V> {
- BTreeMap { root: node::Root::shared_empty_root(), length: 0 }
+ BTreeMap { root: None, length: 0 }
}
/// Clears the map, removing all elements.
@@ -589,7 +601,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
K: Borrow<Q>,
Q: Ord,
{
- match search::search_tree(self.root.as_ref(), key) {
+ match search::search_tree(self.root.as_ref()?.as_ref(), key) {
Found(handle) => Some(handle.into_kv().1),
GoDown(_) => None,
}
@@ -616,7 +628,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
K: Borrow<Q>,
Q: Ord,
{
- match search::search_tree(self.root.as_ref(), k) {
+ match search::search_tree(self.root.as_ref()?.as_ref(), k) {
Found(handle) => Some(handle.into_kv()),
GoDown(_) => None,
}
@@ -645,7 +657,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
T: Ord,
K: Borrow<T>,
{
- let front = self.root.as_ref().first_leaf_edge();
+ let front = self.root.as_ref()?.as_ref().first_leaf_edge();
front.right_kv().ok().map(Handle::into_kv)
}
@@ -674,7 +686,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
T: Ord,
K: Borrow<T>,
{
- let front = self.root.as_mut().first_leaf_edge();
+ let front = self.root.as_mut()?.as_mut().first_leaf_edge();
if let Ok(kv) = front.right_kv() {
Some(OccupiedEntry {
handle: kv.forget_node_type(),
@@ -708,7 +720,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
T: Ord,
K: Borrow<T>,
{
- let back = self.root.as_ref().last_leaf_edge();
+ let back = self.root.as_ref()?.as_ref().last_leaf_edge();
back.left_kv().ok().map(Handle::into_kv)
}
@@ -737,7 +749,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
T: Ord,
K: Borrow<T>,
{
- let back = self.root.as_mut().last_leaf_edge();
+ let back = self.root.as_mut()?.as_mut().last_leaf_edge();
if let Ok(kv) = back.left_kv() {
Some(OccupiedEntry {
handle: kv.forget_node_type(),
@@ -801,7 +813,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
K: Borrow<Q>,
Q: Ord,
{
- match search::search_tree(self.root.as_mut(), key) {
+ match search::search_tree(self.root.as_mut()?.as_mut(), key) {
Found(handle) => Some(handle.into_kv_mut().1),
GoDown(_) => None,
}
@@ -896,7 +908,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
K: Borrow<Q>,
Q: Ord,
{
- match search::search_tree(self.root.as_mut(), key) {
+ match search::search_tree(self.root.as_mut()?.as_mut(), key) {
Found(handle) => Some(
OccupiedEntry { handle, length: &mut self.length, _marker: PhantomData }
.remove_entry(),
@@ -992,11 +1004,15 @@ impl<K: Ord, V> BTreeMap<K, V> {
K: Borrow<T>,
R: RangeBounds<T>,
{
- let root1 = self.root.as_ref();
- let root2 = self.root.as_ref();
- let (f, b) = range_search(root1, root2, range);
+ if let Some(root) = &self.root {
+ let root1 = root.as_ref();
+ let root2 = root.as_ref();
+ let (f, b) = range_search(root1, root2, range);
- Range { front: f, back: b }
+ Range { front: Some(f), back: Some(b) }
+ } else {
+ Range { front: None, back: None }
+ }
}
/// Constructs a mutable double-ended iterator over a sub-range of elements in the map.
@@ -1036,11 +1052,15 @@ impl<K: Ord, V> BTreeMap<K, V> {
K: Borrow<T>,
R: RangeBounds<T>,
{
- let root1 = self.root.as_mut();
- let root2 = unsafe { ptr::read(&root1) };
- let (f, b) = range_search(root1, root2, range);
+ if let Some(root) = &mut self.root {
+ let root1 = root.as_mut();
+ let root2 = unsafe { ptr::read(&root1) };
+ let (f, b) = range_search(root1, root2, range);
- RangeMut { front: f, back: b, _marker: PhantomData }
+ RangeMut { front: Some(f), back: Some(b), _marker: PhantomData }
+ } else {
+ RangeMut { front: None, back: None, _marker: PhantomData }
+ }
}
/// Gets the given key's corresponding entry in the map for in-place manipulation.
@@ -1065,7 +1085,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
pub fn entry(&mut self, key: K) -> Entry<'_, K, V> {
// FIXME(@porglezomp) Avoid allocating if we don't insert
self.ensure_root_is_owned();
- match search::search_tree(self.root.as_mut(), &key) {
+ match search::search_tree(self.root.as_mut().unwrap().as_mut(), &key) {
Found(handle) => {
Occupied(OccupiedEntry { handle, length: &mut self.length, _marker: PhantomData })
}
@@ -1077,7 +1097,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
fn from_sorted_iter<I: Iterator<Item = (K, V)>>(&mut self, iter: I) {
self.ensure_root_is_owned();
- let mut cur_node = self.root.as_mut().last_leaf_edge().into_node();
+ let mut cur_node = self.root.as_mut().unwrap().as_mut().last_leaf_edge().into_node();
// Iterate through all key-value pairs, pushing them into nodes at the right level.
for (key, value) in iter {
// Try to push key-value pair into the current leaf node.
@@ -1126,7 +1146,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
fn fix_right_edge(&mut self) {
// Handle underfull nodes, start from the top.
- let mut cur_node = self.root.as_mut();
+ let mut cur_node = self.root.as_mut().unwrap().as_mut();
while let Internal(internal) = cur_node.force() {
// Check if right-most child is underfull.
let mut last_edge = internal.last_edge();
@@ -1187,14 +1207,14 @@ impl<K: Ord, V> BTreeMap<K, V> {
let total_num = self.len();
let mut right = Self::new();
- right.root = node::Root::new_leaf();
- for _ in 0..(self.root.as_ref().height()) {
- right.root.push_level();
+ right.root = Some(node::Root::new_leaf());
+ for _ in 0..(self.root.as_ref().unwrap().as_ref().height()) {
+ right.root.as_mut().unwrap().push_level();
}
{
- let mut left_node = self.root.as_mut();
- let mut right_node = right.root.as_mut();
+ let mut left_node = self.root.as_mut().unwrap().as_mut();
+ let mut right_node = right.root.as_mut().unwrap().as_mut();
loop {
let mut split_edge = match search::search_node(left_node, key) {
@@ -1223,7 +1243,9 @@ impl<K: Ord, V> BTreeMap<K, V> {
self.fix_right_border();
right.fix_left_border();
- if self.root.as_ref().height() < right.root.as_ref().height() {
+ if self.root.as_ref().unwrap().as_ref().height()
+ < right.root.as_ref().unwrap().as_ref().height()
+ {
self.recalc_length();
right.length = total_num - self.len();
} else {
@@ -1261,19 +1283,19 @@ impl<K: Ord, V> BTreeMap<K, V> {
res
}
- self.length = dfs(self.root.as_ref());
+ self.length = dfs(self.root.as_ref().unwrap().as_ref());
}
/// Removes empty levels on the top.
fn fix_top(&mut self) {
loop {
{
- let node = self.root.as_ref();
+ let node = self.root.as_ref().unwrap().as_ref();
if node.height() == 0 || node.len() > 0 {
break;
}
}
- self.root.pop_level();
+ self.root.as_mut().unwrap().pop_level();
}
}
@@ -1281,7 +1303,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
self.fix_top();
{
- let mut cur_node = self.root.as_mut();
+ let mut cur_node = self.root.as_mut().unwrap().as_mut();
while let Internal(node) = cur_node.force() {
let mut last_kv = node.last_kv();
@@ -1307,7 +1329,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
self.fix_top();
{
- let mut cur_node = self.root.as_mut();
+ let mut cur_node = self.root.as_mut().unwrap().as_mut();
while let Internal(node) = cur_node.force() {
let mut first_kv = node.first_kv();
@@ -1329,8 +1351,8 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// If the root node is the shared root node, allocate our own node.
fn ensure_root_is_owned(&mut self) {
- if self.root.is_shared_root() {
- self.root = node::Root::new_leaf();
+ if self.root.is_none() {
+ self.root = Some(node::Root::new_leaf());
}
}
}
@@ -1458,12 +1480,21 @@ impl<K, V> IntoIterator for BTreeMap<K, V> {
type IntoIter = IntoIter<K, V>;
fn into_iter(self) -> IntoIter<K, V> {
- let root1 = unsafe { ptr::read(&self.root).into_ref() };
- let root2 = unsafe { ptr::read(&self.root).into_ref() };
+ if self.root.is_none() {
+ mem::forget(self);
+ return IntoIter { front: None, back: None, length: 0 };
+ }
+
+ let root1 = unsafe { unwrap_unchecked(ptr::read(&self.root)).into_ref() };
+ let root2 = unsafe { unwrap_unchecked(ptr::read(&self.root)).into_ref() };
let len = self.length;
mem::forget(self);
- IntoIter { front: root1.first_leaf_edge(), back: root2.last_leaf_edge(), length: len }
+ IntoIter {
+ front: Some(root1.first_leaf_edge()),
+ back: Some(root2.last_leaf_edge()),
+ length: len,
+ }
}
}
@@ -1480,7 +1511,8 @@ impl<K, V> Drop for IntoIter<K, V> {
// No need to avoid the shared root, because the tree was definitely not empty.
unsafe {
- let mut node = ptr::read(&self.0.front).into_node().forget_type();
+ let mut node =
+ unwrap_unchecked(ptr::read(&self.0.front)).into_node().forget_type();
while let Some(parent) = node.deallocate_and_ascend() {
node = parent.into_node().forget_type();
}
@@ -1495,14 +1527,13 @@ impl<K, V> Drop for IntoIter<K, V> {
}
unsafe {
- let mut node = ptr::read(&self.front).into_node().forget_type();
- if node.is_shared_root() {
- return;
- }
- // Most of the nodes have been deallocated while traversing
- // but one pile from a leaf up to the root is left standing.
- while let Some(parent) = node.deallocate_and_ascend() {
- node = parent.into_node().forget_type();
+ if let Some(front) = ptr::read(&self.front) {
+ let mut node = front.into_node().forget_type();
+ // Most of the nodes have been deallocated while traversing
+ // but one pile from a leaf up to the root is left standing.
+ while let Some(parent) = node.deallocate_and_ascend() {
+ node = parent.into_node().forget_type();
+ }
}
}
}
@@ -1517,7 +1548,7 @@ impl<K, V> Iterator for IntoIter<K, V> {
None
} else {
self.length -= 1;
- Some(unsafe { self.front.next_unchecked() })
+ Some(unsafe { self.front.as_mut().unwrap().next_unchecked() })
}
}
@@ -1533,7 +1564,7 @@ impl<K, V> DoubleEndedIterator for IntoIter<K, V> {
None
} else {
self.length -= 1;
- Some(unsafe { self.back.next_back_unchecked() })
+ Some(unsafe { self.back.as_mut().unwrap().next_back_unchecked() })
}
}
}
@@ -1683,7 +1714,7 @@ impl<'a, K, V> Range<'a, K, V> {
}
unsafe fn next_unchecked(&mut self) -> (&'a K, &'a V) {
- self.front.next_unchecked()
+ unwrap_unchecked(self.front.as_mut()).next_unchecked()
}
}
@@ -1696,7 +1727,7 @@ impl<'a, K, V> DoubleEndedIterator for Range<'a, K, V> {
impl<'a, K, V> Range<'a, K, V> {
unsafe fn next_back_unchecked(&mut self) -> (&'a K, &'a V) {
- self.back.next_back_unchecked()
+ unwrap_unchecked(self.back.as_mut()).next_back_unchecked()
}
}
@@ -1734,7 +1765,7 @@ impl<'a, K, V> RangeMut<'a, K, V> {
}
unsafe fn next_unchecked(&mut self) -> (&'a mut K, &'a mut V) {
- self.front.next_unchecked()
+ unwrap_unchecked(self.front.as_mut()).next_unchecked()
}
}
@@ -1755,7 +1786,7 @@ impl<K, V> FusedIterator for RangeMut<'_, K, V> {}
impl<'a, K, V> RangeMut<'a, K, V> {
unsafe fn next_back_unchecked(&mut self) -> (&'a mut K, &'a mut V) {
- self.back.next_back_unchecked()
+ unwrap_unchecked(self.back.as_mut()).next_back_unchecked()
}
}
@@ -1969,8 +2000,8 @@ impl<K, V> BTreeMap<K, V> {
pub fn iter(&self) -> Iter<'_, K, V> {
Iter {
range: Range {
- front: self.root.as_ref().first_leaf_edge(),
- back: self.root.as_ref().last_leaf_edge(),
+ front: self.root.as_ref().map(|r| r.as_ref().first_leaf_edge()),
+ back: self.root.as_ref().map(|r| r.as_ref().last_leaf_edge()),
},
length: self.length,
}
@@ -1999,13 +2030,17 @@ impl<K, V> BTreeMap<K, V> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn iter_mut(&mut self) -> IterMut<'_, K, V> {
- let root1 = self.root.as_mut();
- let root2 = unsafe { ptr::read(&root1) };
IterMut {
- range: RangeMut {
- front: root1.first_leaf_edge(),
- back: root2.last_leaf_edge(),
- _marker: PhantomData,
+ range: if let Some(root) = &mut self.root {
+ let root1 = root.as_mut();
+ let root2 = unsafe { ptr::read(&root1) };
+ RangeMut {
+ front: Some(root1.first_leaf_edge()),
+ back: Some(root2.last_leaf_edge()),
+ _marker: PhantomData,
+ }
+ } else {
+ RangeMut { front: None, back: None, _marker: PhantomData }
},
length: self.length,
}
From 1c44f852df205ae6d363cc9a2b12f8093b3ed342 Mon Sep 17 00:00:00 2001
From: Mark Rousskov <mark.simulacrum@gmail.com>
Date: Wed, 18 Mar 2020 10:28:33 -0400
Subject: [PATCH 2/7] Remove shared root code and assertions from BTree nodes
---
src/liballoc/collections/btree/node.rs | 53 +-----------------------
src/liballoc/collections/btree/search.rs | 14 +++----
2 files changed, 8 insertions(+), 59 deletions(-)
diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs
index 1132ffdaf80..5530c9593a5 100644
--- a/src/liballoc/collections/btree/node.rs
+++ b/src/liballoc/collections/btree/node.rs
@@ -111,21 +111,6 @@ impl<K, V> LeafNode<K, V> {
}
}
-impl<K, V> NodeHeader<K, V> {
- fn is_shared_root(&self) -> bool {
- ptr::eq(self, &EMPTY_ROOT_NODE as *const _ as *const _)
- }
-}
-
-// We need to implement Sync here in order to make a static instance.
-unsafe impl Sync for NodeHeader<(), ()> {}
-
-// An empty node used as a placeholder for the root node, to avoid allocations.
-// We use just a header in order to save space, since no operation on an empty tree will
-// ever take a pointer past the first key.
-static EMPTY_ROOT_NODE: NodeHeader<(), ()> =
- NodeHeader { parent: ptr::null(), parent_idx: MaybeUninit::uninit(), len: 0 };
-
/// The underlying representation of internal nodes. As with `LeafNode`s, these should be hidden
/// behind `BoxedNode`s to prevent dropping uninitialized keys and values. Any pointer to an
/// `InternalNode` can be directly casted to a pointer to the underlying `LeafNode` portion of the
@@ -154,8 +139,8 @@ impl<K, V> InternalNode<K, V> {
}
/// A managed, non-null pointer to a node. This is either an owned pointer to
-/// `LeafNode<K, V>`, an owned pointer to `InternalNode<K, V>`, or a (not owned)
-/// pointer to `NodeHeader<(), ()` (more specifically, the pointer to EMPTY_ROOT_NODE).
+/// `LeafNode<K, V>` or an owned pointer to `InternalNode<K, V>`.
+///
/// All of these types have a `NodeHeader<K, V>` prefix, meaning that they have at
/// least the same size as `NodeHeader<K, V>` and store the same kinds of data at the same
/// offsets; and they have a pointer alignment at least as large as `NodeHeader<K, V>`'s.
@@ -196,20 +181,6 @@ unsafe impl<K: Sync, V: Sync> Sync for Root<K, V> {}
unsafe impl<K: Send, V: Send> Send for Root<K, V> {}
impl<K, V> Root<K, V> {
- /// Whether the instance of `Root` wraps a shared, empty root node. If not,
- /// the entire tree is uniquely owned by the owner of the `Root` instance.
- pub fn is_shared_root(&self) -> bool {
- self.as_ref().is_shared_root()
- }
-
- /// Returns a shared tree, wrapping a shared root node that is eternally empty.
- pub fn shared_empty_root() -> Self {
- Root {
- node: unsafe { BoxedNode::from_ptr(NonNull::from(&EMPTY_ROOT_NODE).cast()) },
- height: 0,
- }
- }
-
/// Returns a new owned tree, with its own root node that is initially empty.
pub fn new_leaf() -> Self {
Root { node: BoxedNode::from_leaf(Box::new(unsafe { LeafNode::new() })), height: 0 }
@@ -245,7 +216,6 @@ impl<K, V> Root<K, V> {
/// Adds a new internal node with a single edge, pointing to the previous root, and make that
/// new node the root. This increases the height by 1 and is the opposite of `pop_level`.
pub fn push_level(&mut self) -> NodeRef<marker::Mut<'_>, K, V, marker::Internal> {
- debug_assert!(!self.is_shared_root());
let mut new_node = Box::new(unsafe { InternalNode::new() });
new_node.edges[0].write(unsafe { BoxedNode::from_ptr(self.node.as_ptr()) });
@@ -381,7 +351,6 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
/// Unsafe because the node must not be the shared root. For more information,
/// see the `NodeRef` comments.
unsafe fn as_leaf(&self) -> &LeafNode<K, V> {
- debug_assert!(!self.is_shared_root());
self.node.as_ref()
}
@@ -389,11 +358,6 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
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.
/// Unsafe because the caller must ensure that the node is not the shared root.
pub unsafe fn keys(&self) -> &[K] {
@@ -464,7 +428,6 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::LeafOrInternal> {
pub unsafe fn deallocate_and_ascend(
self,
) -> Option<Handle<NodeRef<marker::Owned, K, V, marker::Internal>, marker::Edge>> {
- assert!(!self.is_shared_root());
let height = self.height;
let node = self.node;
let ret = self.ascend().ok();
@@ -527,14 +490,12 @@ impl<'a, K, V, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Immut<'a>, K, V, Type> {
/// Unsafe because the caller must ensure that the node is not the shared root.
unsafe fn into_key_slice(self) -> &'a [K] {
- debug_assert!(!self.is_shared_root());
// We cannot be the shared root, so `as_leaf` is okay.
slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().keys), self.len())
}
/// Unsafe because the caller must ensure that the node is not the shared root.
unsafe fn into_val_slice(self) -> &'a [V] {
- debug_assert!(!self.is_shared_root());
// We cannot be the shared root, so `as_leaf` is okay.
slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len())
}
@@ -555,7 +516,6 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
/// Unsafe because the caller must ensure that the node is not the shared root.
unsafe fn into_key_slice_mut(mut self) -> &'a mut [K] {
- debug_assert!(!self.is_shared_root());
// We cannot be the shared root, so `as_leaf_mut` is okay.
slice::from_raw_parts_mut(
MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys),
@@ -565,7 +525,6 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
/// Unsafe because the caller must ensure that the node is not the shared root.
unsafe fn into_val_slice_mut(mut self) -> &'a mut [V] {
- debug_assert!(!self.is_shared_root());
slice::from_raw_parts_mut(
MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).vals),
self.len(),
@@ -574,7 +533,6 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
/// Unsafe because the caller must ensure that the node is not the shared root.
unsafe 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
// invalidates the reference returned by the first.
// More precisely, it is the call to `len` that is the culprit,
@@ -592,7 +550,6 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Leaf> {
/// Adds a key/value pair the end of the node.
pub fn push(&mut self, key: K, val: V) {
assert!(self.len() < CAPACITY);
- debug_assert!(!self.is_shared_root());
let idx = self.len();
@@ -607,7 +564,6 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Leaf> {
/// Adds a key/value pair to the beginning of the node.
pub fn push_front(&mut self, key: K, val: V) {
assert!(self.len() < CAPACITY);
- debug_assert!(!self.is_shared_root());
unsafe {
slice_insert(self.keys_mut(), 0, key);
@@ -624,7 +580,6 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
pub fn push(&mut self, key: K, val: V, edge: Root<K, V>) {
assert!(edge.height == self.height - 1);
assert!(self.len() < CAPACITY);
- debug_assert!(!self.is_shared_root());
let idx = self.len();
@@ -658,7 +613,6 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::Internal> {
pub fn push_front(&mut self, key: K, val: V, edge: Root<K, V>) {
assert!(edge.height == self.height - 1);
assert!(self.len() < CAPACITY);
- debug_assert!(!self.is_shared_root());
unsafe {
slice_insert(self.keys_mut(), 0, key);
@@ -904,7 +858,6 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge
fn insert_fit(&mut self, key: K, val: V) -> *mut V {
// Necessary for correctness, but in a private module
debug_assert!(self.node.len() < CAPACITY);
- debug_assert!(!self.node.is_shared_root());
unsafe {
slice_insert(self.node.keys_mut(), self.idx, key);
@@ -1081,7 +1034,6 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::KV>
/// - All the key/value pairs to the right of this handle are put into a newly
/// allocated node.
pub fn split(mut self) -> (NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, K, V, Root<K, V>) {
- assert!(!self.node.is_shared_root());
unsafe {
let mut new_node = Box::new(LeafNode::new());
@@ -1113,7 +1065,6 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::KV>
pub fn remove(
mut self,
) -> (Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>, K, V) {
- assert!(!self.node.is_shared_root());
unsafe {
let k = slice_remove(self.node.keys_mut(), self.idx);
let v = slice_remove(self.node.vals_mut(), self.idx);
diff --git a/src/liballoc/collections/btree/search.rs b/src/liballoc/collections/btree/search.rs
index 2ba5cebbdee..f9354263864 100644
--- a/src/liballoc/collections/btree/search.rs
+++ b/src/liballoc/collections/btree/search.rs
@@ -72,14 +72,12 @@ where
// Using `keys()` is fine here even if BorrowType is mutable, as all we return
// is an index -- not a reference.
let len = node.len();
- if len > 0 {
- let keys = unsafe { node.keys() }; // safe because a non-empty node cannot be the shared root
- for (i, k) in keys.iter().enumerate() {
- match key.cmp(k.borrow()) {
- Ordering::Greater => {}
- Ordering::Equal => return (i, true),
- Ordering::Less => return (i, false),
- }
+ let keys = unsafe { node.keys() }; // safe because a non-empty node cannot be the shared root
+ for (i, k) in keys.iter().enumerate() {
+ match key.cmp(k.borrow()) {
+ Ordering::Greater => {}
+ Ordering::Equal => return (i, true),
+ Ordering::Less => return (i, false),
}
}
(len, false)
From 3c04fda751352fe00884782e7adf792a5e2a91c4 Mon Sep 17 00:00:00 2001
From: Mark Rousskov <mark.simulacrum@gmail.com>
Date: Wed, 18 Mar 2020 10:45:35 -0400
Subject: [PATCH 3/7] Make functions dependent only on shared root avoidance
safe
---
src/liballoc/collections/btree/map.rs | 4 +-
src/liballoc/collections/btree/node.rs | 107 ++++++++++++-----------
src/liballoc/collections/btree/search.rs | 5 +-
3 files changed, 58 insertions(+), 58 deletions(-)
diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs
index 478fa8e1526..21f99257d81 100644
--- a/src/liballoc/collections/btree/map.rs
+++ b/src/liballoc/collections/btree/map.rs
@@ -1349,7 +1349,8 @@ impl<K: Ord, V> BTreeMap<K, V> {
self.fix_top();
}
- /// If the root node is the shared root node, allocate our own node.
+ /// If the root node is the empty (non-allocated) root node, allocate our
+ /// own node.
fn ensure_root_is_owned(&mut self) {
if self.root.is_none() {
self.root = Some(node::Root::new_leaf());
@@ -1509,7 +1510,6 @@ impl<K, V> Drop for IntoIter<K, V> {
// don't have to care about panics this time (they'll abort).
while let Some(_) = self.0.next() {}
- // No need to avoid the shared root, because the tree was definitely not empty.
unsafe {
let mut node =
unwrap_unchecked(ptr::read(&self.0.front)).into_node().forget_type();
diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs
index 5530c9593a5..99f531264ba 100644
--- a/src/liballoc/collections/btree/node.rs
+++ b/src/liballoc/collections/btree/node.rs
@@ -169,8 +169,9 @@ impl<K, V> BoxedNode<K, V> {
}
}
-/// Either an owned tree or a shared, empty tree. Note that this does not have a destructor,
-/// and must be cleaned up manually if it is an owned tree.
+/// An owned tree.
+///
+/// Note that this does not have a destructor, and must be cleaned up manually.
pub struct Root<K, V> {
node: BoxedNode<K, V>,
/// The number of levels below the root node.
@@ -278,10 +279,7 @@ 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 the shared root!
-/// Shared references must be dereferenceable *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> {
/// The number of levels below the node.
@@ -344,14 +342,15 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, 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 that is not the shared root.
+ /// 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.
- /// Unsafe because the node must not be the shared root. For more information,
- /// see the `NodeRef` comments.
- unsafe fn as_leaf(&self) -> &LeafNode<K, V> {
- self.node.as_ref()
+ fn as_leaf(&self) -> &LeafNode<K, V> {
+ // The node must be valid for at least the LeafNode portion.
+ // This is not a reference in the NodeRef type because we don't know if
+ // it should be unique or shared.
+ unsafe { self.node.as_ref() }
}
fn as_header(&self) -> &NodeHeader<K, V> {
@@ -359,14 +358,12 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
}
/// Borrows a view into the keys stored in the node.
- /// Unsafe because the caller must ensure that the node is not the shared root.
- pub unsafe fn keys(&self) -> &[K] {
+ pub fn keys(&self) -> &[K] {
self.reborrow().into_key_slice()
}
/// Borrows a view into the values stored in the node.
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn vals(&self) -> &[V] {
+ fn vals(&self) -> &[V] {
self.reborrow().into_val_slice()
}
@@ -470,39 +467,37 @@ impl<'a, K, V, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
/// (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> {
self.node.as_ptr()
}
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn keys_mut(&mut self) -> &mut [K] {
- self.reborrow_mut().into_key_slice_mut()
+ fn keys_mut(&mut self) -> &mut [K] {
+ // SAFETY: the caller will not be able to call further methods on self
+ // until the key slice reference is dropped, as we have unique access
+ // for the lifetime of the borrow.
+ unsafe { self.reborrow_mut().into_key_slice_mut() }
}
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn vals_mut(&mut self) -> &mut [V] {
- self.reborrow_mut().into_val_slice_mut()
+ fn vals_mut(&mut self) -> &mut [V] {
+ // SAFETY: the caller will not be able to call further methods on self
+ // until the value slice reference is dropped, as we have unique access
+ // for the lifetime of the borrow.
+ unsafe { self.reborrow_mut().into_val_slice_mut() }
}
}
impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Immut<'a>, K, V, Type> {
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn into_key_slice(self) -> &'a [K] {
- // We cannot be the shared root, so `as_leaf` is okay.
- slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().keys), self.len())
+ fn into_key_slice(self) -> &'a [K] {
+ unsafe { slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().keys), self.len()) }
}
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn into_val_slice(self) -> &'a [V] {
- // We cannot be the shared root, so `as_leaf` is okay.
- slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len())
+ fn into_val_slice(self) -> &'a [V] {
+ unsafe { slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len()) }
}
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn into_slices(self) -> (&'a [K], &'a [V]) {
- let k = ptr::read(&self);
+ fn into_slices(self) -> (&'a [K], &'a [V]) {
+ // SAFETY: equivalent to reborrow() except not requiring Type: 'a
+ let k = unsafe { ptr::read(&self) };
(k.into_key_slice(), self.into_val_slice())
}
}
@@ -514,25 +509,27 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
unsafe { &mut *(self.root as *mut Root<K, V>) }
}
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn into_key_slice_mut(mut self) -> &'a mut [K] {
- // We cannot be the shared root, so `as_leaf_mut` is okay.
- slice::from_raw_parts_mut(
- MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys),
- self.len(),
- )
+ fn into_key_slice_mut(mut self) -> &'a mut [K] {
+ // SAFETY: The keys of a node must always be initialized up to length.
+ unsafe {
+ slice::from_raw_parts_mut(
+ MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys),
+ self.len(),
+ )
+ }
}
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn into_val_slice_mut(mut self) -> &'a mut [V] {
- slice::from_raw_parts_mut(
- MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).vals),
- self.len(),
- )
+ fn into_val_slice_mut(mut self) -> &'a mut [V] {
+ // SAFETY: The values of a node must always be initialized up to length.
+ unsafe {
+ slice::from_raw_parts_mut(
+ MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).vals),
+ self.len(),
+ )
+ }
}
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn into_slices_mut(mut self) -> (&'a mut [K], &'a mut [V]) {
+ fn into_slices_mut(mut self) -> (&'a mut [K], &'a mut [V]) {
// We cannot use the getters here, because calling the second one
// invalidates the reference returned by the first.
// More precisely, it is the call to `len` that is the culprit,
@@ -540,8 +537,13 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
// overlap with the keys (and even the values, for ZST keys).
let len = self.len();
let leaf = self.as_leaf_mut();
- let keys = slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).keys), len);
- let vals = slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).vals), len);
+ // SAFETY: The keys and values of a node must always be initialized up to length.
+ let keys = unsafe {
+ slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).keys), len)
+ };
+ let vals = unsafe {
+ slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).vals), len)
+ };
(keys, vals)
}
}
@@ -698,8 +700,7 @@ impl<'a, K, V> NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal> {
}
}
- /// Unsafe because the caller must ensure that the node is not the shared root.
- unsafe fn into_kv_pointers_mut(mut self) -> (*mut K, *mut V) {
+ fn into_kv_pointers_mut(mut self) -> (*mut K, *mut V) {
(self.keys_mut().as_mut_ptr(), self.vals_mut().as_mut_ptr())
}
}
diff --git a/src/liballoc/collections/btree/search.rs b/src/liballoc/collections/btree/search.rs
index f9354263864..4e80f7f21eb 100644
--- a/src/liballoc/collections/btree/search.rs
+++ b/src/liballoc/collections/btree/search.rs
@@ -67,12 +67,11 @@ where
Q: Ord,
K: Borrow<Q>,
{
- // This function is defined over all borrow types (immutable, mutable, owned),
- // and may be called on the shared root in each case.
+ // This function is defined over all borrow types (immutable, mutable, owned).
// Using `keys()` is fine here even if BorrowType is mutable, as all we return
// is an index -- not a reference.
let len = node.len();
- let keys = unsafe { node.keys() }; // safe because a non-empty node cannot be the shared root
+ let keys = node.keys();
for (i, k) in keys.iter().enumerate() {
match key.cmp(k.borrow()) {
Ordering::Greater => {}
From 54b7c38889ee7bb59f4d6449822fcfdfa3448ddb Mon Sep 17 00:00:00 2001
From: Mark Rousskov <mark.simulacrum@gmail.com>
Date: Wed, 18 Mar 2020 11:10:21 -0400
Subject: [PATCH 4/7] Drop NodeHeader type from BTree code
We no longer have a separate header because the shared root is gone; all code
can work solely with leafs now.
---
src/liballoc/collections/btree/node.rs | 46 +++-----------------------
1 file changed, 5 insertions(+), 41 deletions(-)
diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs
index 99f531264ba..6ebb98c42cd 100644
--- a/src/liballoc/collections/btree/node.rs
+++ b/src/liballoc/collections/btree/node.rs
@@ -44,34 +44,7 @@ const B: usize = 6;
pub const MIN_LEN: usize = B - 1;
pub const CAPACITY: usize = 2 * B - 1;
-/// The underlying representation of leaf nodes. Note that it is often unsafe to actually store
-/// these, since only the first `len` keys and values are assumed to be initialized. As such,
-/// these should always be put behind pointers, and specifically behind `BoxedNode` in the owned
-/// case.
-///
-/// We have a separate type for the header and rely on it matching the prefix of `LeafNode`, in
-/// order to statically allocate a single dummy node to avoid allocations. This struct is
-/// `repr(C)` to prevent them from being reordered. `LeafNode` does not just contain a
-/// `NodeHeader` because we do not want unnecessary padding between `len` and the keys.
-/// Crucially, `NodeHeader` can be safely transmuted to different K and V. (This is exploited
-/// by `as_header`.)
-#[repr(C)]
-struct NodeHeader<K, V> {
- /// We use `*const` as opposed to `*mut` so as to be covariant in `K` and `V`.
- /// This either points to an actual node or is null.
- parent: *const InternalNode<K, V>,
-
- /// This node's index into the parent node's `edges` array.
- /// `*node.parent.edges[node.parent_idx]` should be the same thing as `node`.
- /// This is only guaranteed to be initialized when `parent` is non-null.
- parent_idx: MaybeUninit<u16>,
-
- /// The number of keys and values this node stores.
- ///
- /// This next to `parent_idx` to encourage the compiler to join `len` and
- /// `parent_idx` into the same 32-bit word, reducing space overhead.
- len: u16,
-}
+/// The underlying representation of leaf nodes.
#[repr(C)]
struct LeafNode<K, V> {
/// We use `*const` as opposed to `*mut` so as to be covariant in `K` and `V`.
@@ -141,10 +114,7 @@ impl<K, V> InternalNode<K, V> {
/// A managed, non-null pointer to a node. This is either an owned pointer to
/// `LeafNode<K, V>` or an owned pointer to `InternalNode<K, V>`.
///
-/// All of these types have a `NodeHeader<K, V>` prefix, meaning that they have at
-/// least the same size as `NodeHeader<K, V>` and store the same kinds of data at the same
-/// offsets; and they have a pointer alignment at least as large as `NodeHeader<K, V>`'s.
-/// However, `BoxedNode` contains no information as to which of the three types
+/// However, `BoxedNode` contains no information as to which of the two types
/// of nodes it actually contains, and, partially due to this lack of information,
/// has no destructor.
struct BoxedNode<K, V> {
@@ -279,8 +249,6 @@ 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.
-///
-/// Turning this into a `NodeHeader` reference is always safe.
pub struct NodeRef<BorrowType, K, V, Type> {
/// The number of levels below the node.
height: usize,
@@ -322,7 +290,7 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
/// 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 {
- self.as_header().len as usize
+ self.as_leaf().len as usize
}
/// Returns the height of this node in the whole tree. Zero height denotes the
@@ -353,10 +321,6 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
unsafe { self.node.as_ref() }
}
- fn as_header(&self) -> &NodeHeader<K, V> {
- unsafe { &*(self.node.as_ptr() as *const NodeHeader<K, V>) }
- }
-
/// Borrows a view into the keys stored in the node.
pub fn keys(&self) -> &[K] {
self.reborrow().into_key_slice()
@@ -377,7 +341,7 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
pub fn ascend(
self,
) -> Result<Handle<NodeRef<BorrowType, K, V, marker::Internal>, marker::Edge>, Self> {
- let parent_as_leaf = self.as_header().parent as *const LeafNode<K, V>;
+ let parent_as_leaf = self.as_leaf().parent as *const LeafNode<K, V>;
if let Some(non_zero) = NonNull::new(parent_as_leaf as *mut _) {
Ok(Handle {
node: NodeRef {
@@ -386,7 +350,7 @@ impl<BorrowType, K, V, Type> NodeRef<BorrowType, K, V, Type> {
root: self.root,
_marker: PhantomData,
},
- idx: unsafe { usize::from(*self.as_header().parent_idx.as_ptr()) },
+ idx: unsafe { usize::from(*self.as_leaf().parent_idx.as_ptr()) },
_marker: PhantomData,
})
} else {
From 13f6d771bb7f3617e247d9806e1b9472022cdaf4 Mon Sep 17 00:00:00 2001
From: Mark Rousskov <mark.simulacrum@gmail.com>
Date: Wed, 18 Mar 2020 19:00:15 -0400
Subject: [PATCH 5/7] Simplify ensure_root_is_owned callers
This makes ensure_root_is_owned return a reference to the (now guaranteed to
exist) root, allowing callers to operate on it without going through another
unwrap.
Unfortunately this is only rarely useful as it's frequently the case that both
the length and the root need to be accessed and field-level borrows in methods
don't yet exist.
---
src/liballoc/collections/btree/map.rs | 28 +++++++++++++--------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs
index 21f99257d81..3ba7befc046 100644
--- a/src/liballoc/collections/btree/map.rs
+++ b/src/liballoc/collections/btree/map.rs
@@ -170,13 +170,13 @@ impl<K: Clone, V: Clone> Clone for BTreeMap<K, V> {
}
Internal(internal) => {
let mut out_tree = clone_subtree(internal.first_edge().descend());
-
- // Cannot call ensure_root_is_owned() because lacking K: Ord
- if out_tree.root.is_none() {
- out_tree.root = Some(node::Root::new_leaf());
- }
+ out_tree.ensure_root_is_owned();
{
+ // Ideally we'd use the return of ensure_root_is_owned
+ // instead of re-unwrapping here but unfortunately that
+ // borrows all of out_tree and we need access to the
+ // length below.
let mut out_node = out_tree.root.as_mut().unwrap().push_level();
let mut in_edge = internal.first_edge();
while let Ok(kv) = in_edge.right_kv() {
@@ -1207,9 +1207,9 @@ impl<K: Ord, V> BTreeMap<K, V> {
let total_num = self.len();
let mut right = Self::new();
- right.root = Some(node::Root::new_leaf());
+ let right_root = right.ensure_root_is_owned();
for _ in 0..(self.root.as_ref().unwrap().as_ref().height()) {
- right.root.as_mut().unwrap().push_level();
+ right_root.push_level();
}
{
@@ -1348,14 +1348,6 @@ impl<K: Ord, V> BTreeMap<K, V> {
self.fix_top();
}
-
- /// If the root node is the empty (non-allocated) root node, allocate our
- /// own node.
- fn ensure_root_is_owned(&mut self) {
- if self.root.is_none() {
- self.root = Some(node::Root::new_leaf());
- }
- }
}
#[stable(feature = "rust1", since = "1.0.0")]
@@ -2151,6 +2143,12 @@ impl<K, V> BTreeMap<K, V> {
pub fn is_empty(&self) -> bool {
self.len() == 0
}
+
+ /// If the root node is the empty (non-allocated) root node, allocate our
+ /// own node.
+ fn ensure_root_is_owned(&mut self) -> &mut node::Root<K, V> {
+ self.root.get_or_insert_with(|| node::Root::new_leaf())
+ }
}
impl<'a, K: Ord, V> Entry<'a, K, V> {
From 4d85314a0008ec809054854dff55c09913b1e80f Mon Sep 17 00:00:00 2001
From: Mark Rousskov <mark.simulacrum@gmail.com>
Date: Fri, 20 Mar 2020 09:40:57 -0400
Subject: [PATCH 6/7] Update test commentary for shared root removal
---
src/liballoc/tests/btree/map.rs | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/liballoc/tests/btree/map.rs b/src/liballoc/tests/btree/map.rs
index d05eec19346..3a3462d546f 100644
--- a/src/liballoc/tests/btree/map.rs
+++ b/src/liballoc/tests/btree/map.rs
@@ -67,7 +67,7 @@ fn test_basic_large() {
#[test]
fn test_basic_small() {
let mut map = BTreeMap::new();
- // Empty, shared root:
+ // Empty, root is absent (None):
assert_eq!(map.remove(&1), None);
assert_eq!(map.len(), 0);
assert_eq!(map.get(&1), None);
@@ -123,7 +123,7 @@ fn test_basic_small() {
assert_eq!(map.values().collect::<Vec<_>>(), vec![&4]);
assert_eq!(map.remove(&2), Some(4));
- // Empty but private root:
+ // Empty but root is owned (Some(...)):
assert_eq!(map.len(), 0);
assert_eq!(map.get(&1), None);
assert_eq!(map.get_mut(&1), None);
@@ -263,13 +263,6 @@ fn test_iter_mut_mutation() {
do_test_iter_mut_mutation::<Align32>(144);
}
-#[test]
-fn test_into_key_slice_with_shared_root_past_bounds() {
- let mut map: BTreeMap<Align32, ()> = BTreeMap::new();
- assert_eq!(map.get(&Align32(1)), None);
- assert_eq!(map.get_mut(&Align32(1)), None);
-}
-
#[test]
fn test_values_mut() {
let mut a = BTreeMap::new();
From bce7f6f3a09c0dc2e56c8b3a9f17c8a18cada009 Mon Sep 17 00:00:00 2001
From: Mark Rousskov <mark.simulacrum@gmail.com>
Date: Wed, 18 Mar 2020 15:29:05 -0400
Subject: [PATCH 7/7] Fix debugger pretty printing of BTrees
---
src/etc/gdb_rust_pretty_printing.py | 34 ++++++++++++--------
src/test/debuginfo/pretty-std-collections.rs | 24 ++++++++++----
2 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/src/etc/gdb_rust_pretty_printing.py b/src/etc/gdb_rust_pretty_printing.py
index 0914c22eb13..cae64ef6665 100755
--- a/src/etc/gdb_rust_pretty_printing.py
+++ b/src/etc/gdb_rust_pretty_printing.py
@@ -370,12 +370,17 @@ class RustStdBTreeSetPrinter(object):
("(len: %i)" % self.__val.get_wrapped_value()['map']['length']))
def children(self):
- root = self.__val.get_wrapped_value()['map']['root']
- node_ptr = root['node']
- i = 0
- for child in children_of_node(node_ptr, root['height'], False):
- yield (str(i), child)
- i = i + 1
+ prev_idx = None
+ innermap = GdbValue(self.__val.get_wrapped_value()['map'])
+ if innermap.get_wrapped_value()['length'] > 0:
+ root = GdbValue(innermap.get_wrapped_value()['root'])
+ type_name = str(root.type.ty.name).replace('core::option::Option<', '')[:-1]
+ root = root.get_wrapped_value().cast(gdb.lookup_type(type_name))
+ node_ptr = root['node']
+ i = 0
+ for child in children_of_node(node_ptr, root['height'], False):
+ yield (str(i), child)
+ i = i + 1
class RustStdBTreeMapPrinter(object):
@@ -391,13 +396,16 @@ class RustStdBTreeMapPrinter(object):
("(len: %i)" % self.__val.get_wrapped_value()['length']))
def children(self):
- root = self.__val.get_wrapped_value()['root']
- node_ptr = root['node']
- i = 0
- for child in children_of_node(node_ptr, root['height'], True):
- yield (str(i), child[0])
- yield (str(i), child[1])
- i = i + 1
+ if self.__val.get_wrapped_value()['length'] > 0:
+ root = GdbValue(self.__val.get_wrapped_value()['root'])
+ type_name = str(root.type.ty.name).replace('core::option::Option<', '')[:-1]
+ root = root.get_wrapped_value().cast(gdb.lookup_type(type_name))
+ node_ptr = root['node']
+ i = 0
+ for child in children_of_node(node_ptr, root['height'], True):
+ yield (str(i), child[0])
+ yield (str(i), child[1])
+ i = i + 1
class RustStdStringPrinter(object):
diff --git a/src/test/debuginfo/pretty-std-collections.rs b/src/test/debuginfo/pretty-std-collections.rs
index f8997fad9a5..3d2d88a676d 100644
--- a/src/test/debuginfo/pretty-std-collections.rs
+++ b/src/test/debuginfo/pretty-std-collections.rs
@@ -17,35 +17,43 @@
// gdb-command: print btree_set
// gdb-check:$1 = BTreeSet<i32>(len: 15) = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}
+// gdb-command: print empty_btree_set
+// gdb-check:$2 = BTreeSet<i32>(len: 0)
+
// gdb-command: print btree_map
-// gdb-check:$2 = BTreeMap<i32, i32>(len: 15) = {[0] = 0, [1] = 1, [2] = 2, [3] = 3, [4] = 4, [5] = 5, [6] = 6, [7] = 7, [8] = 8, [9] = 9, [10] = 10, [11] = 11, [12] = 12, [13] = 13, [14] = 14}
+// gdb-check:$3 = BTreeMap<i32, i32>(len: 15) = {[0] = 0, [1] = 1, [2] = 2, [3] = 3, [4] = 4, [5] = 5, [6] = 6, [7] = 7, [8] = 8, [9] = 9, [10] = 10, [11] = 11, [12] = 12, [13] = 13, [14] = 14}
+
+// gdb-command: print empty_btree_map
+// gdb-check:$4 = BTreeMap<i32, u32>(len: 0)
// gdb-command: print vec_deque
-// gdb-check:$3 = VecDeque<i32>(len: 3, cap: 8) = {5, 3, 7}
+// gdb-check:$5 = VecDeque<i32>(len: 3, cap: 8) = {5, 3, 7}
// gdb-command: print vec_deque2
-// gdb-check:$4 = VecDeque<i32>(len: 7, cap: 8) = {2, 3, 4, 5, 6, 7, 8}
+// gdb-check:$6 = VecDeque<i32>(len: 7, cap: 8) = {2, 3, 4, 5, 6, 7, 8}
#![allow(unused_variables)]
-use std::collections::BTreeSet;
use std::collections::BTreeMap;
+use std::collections::BTreeSet;
use std::collections::VecDeque;
-
fn main() {
-
// BTreeSet
let mut btree_set = BTreeSet::new();
for i in 0..15 {
btree_set.insert(i);
}
+ let mut empty_btree_set: BTreeSet<i32> = BTreeSet::new();
+
// BTreeMap
let mut btree_map = BTreeMap::new();
for i in 0..15 {
btree_map.insert(i, i);
}
+ let mut empty_btree_map: BTreeMap<i32, u32> = BTreeMap::new();
+
// VecDeque
let mut vec_deque = VecDeque::new();
vec_deque.push_back(5);
@@ -63,4 +71,6 @@ fn main() {
zzz(); // #break
}
-fn zzz() { () }
+fn zzz() {
+ ()
+}