Skip to content

Commit 7c95768

Browse files
committed
btree: some cleanup with less unsafe
1 parent f524236 commit 7c95768

File tree

1 file changed

+14
-19
lines changed
  • library/alloc/src/collections/btree

1 file changed

+14
-19
lines changed

library/alloc/src/collections/btree/node.rs

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
use core::marker::PhantomData;
3535
use core::mem::{self, MaybeUninit};
36+
use core::num::NonZero;
3637
use core::ptr::{self, NonNull};
3738
use core::slice::SliceIndex;
3839

@@ -143,7 +144,7 @@ type BoxedNode<K, V> = NonNull<LeafNode<K, V>>;
143144
///
144145
/// A reference to a node.
145146
///
146-
/// This type has a number of parameters that controls how it acts:
147+
/// This type has a number of parameters that control how it acts:
147148
/// - `BorrowType`: A dummy type that describes the kind of borrow and carries a lifetime.
148149
/// - When this is `Immut<'a>`, the `NodeRef` acts roughly like `&'a Node`.
149150
/// - When this is `ValMut<'a>`, the `NodeRef` acts roughly like `&'a Node`
@@ -226,33 +227,27 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::Leaf> {
226227

227228
fn from_new_leaf<A: Allocator + Clone>(leaf: Box<LeafNode<K, V>, A>) -> Self {
228229
// The allocator must be dropped, not leaked. See also `BTreeMap::alloc`.
229-
let (leaf, _alloc) = Box::into_raw_with_allocator(leaf);
230-
// SAFETY: the node was just allocated.
231-
let node = unsafe { NonNull::new_unchecked(leaf) };
230+
let (node, _alloc) = Box::into_non_null_with_allocator(leaf);
232231
NodeRef { height: 0, node, _marker: PhantomData }
233232
}
234233
}
235234

236235
impl<K, V> NodeRef<marker::Owned, K, V, marker::Internal> {
236+
/// Creates a new internal (height > 0) `NodeRef`
237237
fn new_internal<A: Allocator + Clone>(child: Root<K, V>, alloc: A) -> Self {
238238
let mut new_node = unsafe { InternalNode::new(alloc) };
239239
new_node.edges[0].write(child.node);
240-
unsafe { NodeRef::from_new_internal(new_node, child.height + 1) }
240+
NodeRef::from_new_internal(new_node, NonZero::new(child.height + 1).unwrap())
241241
}
242242

243-
/// # Safety
244-
/// `height` must not be zero.
245-
unsafe fn from_new_internal<A: Allocator + Clone>(
243+
/// Creates a new internal (height > 0) `NodeRef` from an existing internal node
244+
fn from_new_internal<A: Allocator + Clone>(
246245
internal: Box<InternalNode<K, V>, A>,
247-
height: usize,
246+
height: NonZero<usize>,
248247
) -> Self {
249-
debug_assert!(height > 0);
250248
// The allocator must be dropped, not leaked. See also `BTreeMap::alloc`.
251-
let (internal, _alloc) = Box::into_raw_with_allocator(internal);
252-
// SAFETY: the node was just allocated.
253-
let internal = unsafe { NonNull::new_unchecked(internal) };
254-
let node = internal.cast();
255-
let mut this = NodeRef { height, node, _marker: PhantomData };
249+
let (node, _alloc) = Box::into_non_null_with_allocator(internal);
250+
let mut this = NodeRef { height: height.into(), node: node.cast(), _marker: PhantomData };
256251
this.borrow_mut().correct_all_childrens_parent_links();
257252
this
258253
}
@@ -625,9 +620,8 @@ impl<K, V> NodeRef<marker::Owned, K, V, marker::LeafOrInternal> {
625620
let top = self.node;
626621

627622
// SAFETY: we asserted to be internal.
628-
let internal_self = unsafe { self.borrow_mut().cast_to_internal_unchecked() };
629-
// SAFETY: we borrowed `self` exclusively and its borrow type is exclusive.
630-
let internal_node = unsafe { &mut *NodeRef::as_internal_ptr(&internal_self) };
623+
let mut internal_self = unsafe { self.borrow_mut().cast_to_internal_unchecked() };
624+
let internal_node = internal_self.as_internal_mut();
631625
// SAFETY: the first edge is always initialized.
632626
self.node = unsafe { internal_node.edges[0].assume_init_read() };
633627
self.height -= 1;
@@ -1305,7 +1299,8 @@ impl<'a, K: 'a, V: 'a> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>,
13051299
&mut new_node.edges[..new_len + 1],
13061300
);
13071301

1308-
let height = self.node.height;
1302+
// SAFETY: self is `marker::Internal`, so `self.node.height` is positive
1303+
let height = NonZero::new_unchecked(self.node.height);
13091304
let right = NodeRef::from_new_internal(new_node, height);
13101305

13111306
SplitResult { left: self.node, kv, right }

0 commit comments

Comments
 (0)