From 37b5cca3d58413fafdf40aa231bcc5ababaaa0fe Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Thu, 2 Jan 2020 12:42:31 +0100 Subject: [PATCH 1/2] Simplify into_key_slice_mut and document bits and bobs --- src/liballoc/collections/btree/node.rs | 22 +++++++++------------- src/liballoc/collections/btree/search.rs | 11 +++++++++++ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 260e51d635dbb..03cb54ebce782 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -397,6 +397,7 @@ impl NodeRef { /// Borrows a view into the values stored in the node. /// The caller must ensure that the node is not the shared root. + /// This function is not public, so doesn't have to support shared roots like `keys` does. fn vals(&self) -> &[V] { self.reborrow().into_val_slice() } @@ -514,6 +515,7 @@ impl<'a, K, V, Type> NodeRef, K, V, Type> { } /// The caller must ensure that the node is not the shared root. + /// This function is not public, so doesn't have to support shared roots like `keys` does. fn keys_mut(&mut self) -> &mut [K] { unsafe { self.reborrow_mut().into_key_slice_mut() } } @@ -590,19 +592,13 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { } fn into_key_slice_mut(mut self) -> &'a mut [K] { - // Same as for `into_key_slice` above, we try to avoid a run-time check. - if (mem::align_of::>() > mem::align_of::>() - || mem::size_of::>() != mem::size_of::>()) - && self.is_shared_root() - { - &mut [] - } else { - unsafe { - slice::from_raw_parts_mut( - MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys), - self.len(), - ) - } + debug_assert!(!self.is_shared_root()); + // We cannot be the shared root, so `as_leaf_mut` is okay. + unsafe { + slice::from_raw_parts_mut( + MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys), + self.len(), + ) } } diff --git a/src/liballoc/collections/btree/search.rs b/src/liballoc/collections/btree/search.rs index 3f3c49a2ef875..bdca4d186cfbd 100644 --- a/src/liballoc/collections/btree/search.rs +++ b/src/liballoc/collections/btree/search.rs @@ -46,6 +46,11 @@ where } } +/// Returns the index in the node at which the key (or an equivalent) exists +/// or could exist, and whether it exists in the node itself. If it doesn't +/// exist in the node itself, it may exist in the subtree with that index +/// (if the node has subtrees). If the key doesn't exist in node or subtree, +/// the returned index is the position or subtree to insert at. pub fn search_linear( node: &NodeRef, key: &Q, @@ -54,6 +59,12 @@ where Q: Ord, K: Borrow, { + // This function is defined over all borrow types (immutable, mutable, owned), + // and may be called on the shared root in each case. + // Crucially, we use `keys()` here, i.e., we work with immutable data. + // We do not need to make `keys_mut()` public and require support for the shared root. + // Using `keys()` is fine here even if BorrowType is mutable, as all we return + // is an index -- not a reference. for (i, k) in node.keys().iter().enumerate() { match key.cmp(k.borrow()) { Ordering::Greater => {} From 9b92bf83156fbe4892fd7a1aa186ce15cce3b770 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Thu, 9 Jan 2020 12:03:49 +0100 Subject: [PATCH 2/2] Apply suggestions from code review Co-Authored-By: Ralf Jung --- src/liballoc/collections/btree/node.rs | 1 + src/liballoc/collections/btree/search.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 03cb54ebce782..f40e0b0c30479 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -591,6 +591,7 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { unsafe { &mut *(self.root as *mut Root) } } + /// The caller must ensure that the node is not the shared root. 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. diff --git a/src/liballoc/collections/btree/search.rs b/src/liballoc/collections/btree/search.rs index bdca4d186cfbd..48cbf67eea254 100644 --- a/src/liballoc/collections/btree/search.rs +++ b/src/liballoc/collections/btree/search.rs @@ -62,7 +62,7 @@ where // This function is defined over all borrow types (immutable, mutable, owned), // and may be called on the shared root in each case. // Crucially, we use `keys()` here, i.e., we work with immutable data. - // We do not need to make `keys_mut()` public and require support for the shared root. + // `keys_mut()` does not support the shared root, so we cannot use it. // Using `keys()` is fine here even if BorrowType is mutable, as all we return // is an index -- not a reference. for (i, k) in node.keys().iter().enumerate() {