]> git.lizzy.rs Git - rust.git/commitdiff
Rollup merge of #75519 - ssomers:btree_splitpoint_cleanup, r=Mark-Simulacrum
authorTyler Mandry <tmandry@gmail.com>
Fri, 14 Aug 2020 21:47:01 +0000 (14:47 -0700)
committerGitHub <noreply@github.com>
Fri, 14 Aug 2020 21:47:01 +0000 (14:47 -0700)
BTreeMap: refactor splitpoint and move testing over to unit test

r? @Mark-Simulacrum

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

index b0741a2c00dadf987e100d9d06048117da4e4474,5a6a71c5fb687a0725fad418f737f499eb2a0c90..acc2ae73572ba757aa801bafde1ae2e376efc2b5
@@@ -43,6 -43,9 +43,9 @@@ use crate::boxed::Box
  const B: usize = 6;
  pub const MIN_LEN: usize = B - 1;
  pub const CAPACITY: usize = 2 * B - 1;
+ const KV_IDX_CENTER: usize = B - 1;
+ const EDGE_IDX_LEFT_OF_CENTER: usize = B - 1;
+ const EDGE_IDX_RIGHT_OF_CENTER: usize = B;
  
  /// The underlying representation of leaf nodes.
  #[repr(C)]
@@@ -413,7 -416,7 +416,7 @@@ impl<K, V> NodeRef<marker::Owned, K, V
  impl<'a, K, V, Type> NodeRef<marker::Mut<'a>, K, V, Type> {
      /// Unsafely asserts to the compiler some static information about whether this
      /// node is a `Leaf` or an `Internal`.
 -    unsafe fn cast_unchecked<NewType>(&mut self) -> NodeRef<marker::Mut<'_>, K, V, NewType> {
 +    unsafe fn cast_unchecked<NewType>(self) -> NodeRef<marker::Mut<'a>, K, V, NewType> {
          NodeRef { height: self.height, node: self.node, root: self.root, _marker: PhantomData }
      }
  
@@@ -721,7 -724,7 +724,7 @@@ impl<Node: Copy, Type> Clone for Handle
  }
  
  impl<Node, Type> Handle<Node, Type> {
 -    /// Retrieves the node that contains the edge of key/value pair this handle points to.
 +    /// Retrieves the node that contains the edge or key/value pair this handle points to.
      pub fn into_node(self) -> Node {
          self.node
      }
@@@ -834,38 -837,12 +837,12 @@@ enum InsertionPlace 
  fn splitpoint(edge_idx: usize) -> (usize, InsertionPlace) {
      debug_assert!(edge_idx <= CAPACITY);
      // Rust issue #74834 tries to explain these symmetric rules.
-     let middle_kv_idx;
-     let insertion;
-     if edge_idx <= B - 2 {
-         middle_kv_idx = B - 2;
-         insertion = InsertionPlace::Left(edge_idx);
-     } else if edge_idx == B - 1 {
-         middle_kv_idx = B - 1;
-         insertion = InsertionPlace::Left(edge_idx);
-     } else if edge_idx == B {
-         middle_kv_idx = B - 1;
-         insertion = InsertionPlace::Right(0);
-     } else {
-         middle_kv_idx = B;
-         let new_edge_idx = edge_idx - (B + 1);
-         insertion = InsertionPlace::Right(new_edge_idx);
-     }
-     let mut left_len = middle_kv_idx;
-     let mut right_len = CAPACITY - middle_kv_idx - 1;
-     match insertion {
-         InsertionPlace::Left(edge_idx) => {
-             debug_assert!(edge_idx <= left_len);
-             left_len += 1;
-         }
-         InsertionPlace::Right(edge_idx) => {
-             debug_assert!(edge_idx <= right_len);
-             right_len += 1;
-         }
+     match edge_idx {
+         0..EDGE_IDX_LEFT_OF_CENTER => (KV_IDX_CENTER - 1, InsertionPlace::Left(edge_idx)),
+         EDGE_IDX_LEFT_OF_CENTER => (KV_IDX_CENTER, InsertionPlace::Left(edge_idx)),
+         EDGE_IDX_RIGHT_OF_CENTER => (KV_IDX_CENTER, InsertionPlace::Right(0)),
+         _ => (KV_IDX_CENTER + 1, InsertionPlace::Right(edge_idx - (KV_IDX_CENTER + 1 + 1))),
      }
-     debug_assert!(left_len >= MIN_LEN);
-     debug_assert!(right_len >= MIN_LEN);
-     debug_assert!(left_len + right_len == CAPACITY);
-     (middle_kv_idx, insertion)
  }
  
  impl<'a, K, V, NodeType> Handle<NodeRef<marker::Mut<'a>, K, V, NodeType>, marker::Edge> {
@@@ -1067,16 -1044,6 +1044,16 @@@ impl<'a, K: 'a, V: 'a, NodeType> Handle
  }
  
  impl<'a, K: 'a, V: 'a, NodeType> Handle<NodeRef<marker::Mut<'a>, K, V, NodeType>, marker::KV> {
 +    pub fn into_key_mut(self) -> &'a mut K {
 +        let keys = self.node.into_key_slice_mut();
 +        unsafe { keys.get_unchecked_mut(self.idx) }
 +    }
 +
 +    pub fn into_val_mut(self) -> &'a mut V {
 +        let vals = self.node.into_val_slice_mut();
 +        unsafe { vals.get_unchecked_mut(self.idx) }
 +    }
 +
      pub fn into_kv_mut(self) -> (&'a mut K, &'a mut V) {
          unsafe {
              let (keys, vals) = self.node.into_slices_mut();
@@@ -1141,7 -1108,7 +1118,7 @@@ impl<'a, K, V> Handle<NodeRef<marker::M
      }
  
      /// 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.
 +    /// that the key/value pair collapsed into.
      pub fn remove(
          mut self,
      ) -> ((K, V), Handle<NodeRef<marker::Mut<'a>, K, V, marker::Leaf>, marker::Edge>) {
@@@ -1199,7 -1166,7 +1176,7 @@@ impl<'a, K, V> Handle<NodeRef<marker::M
      /// to by this handle, and the node immediately to the right of this handle into one new
      /// child of the underlying node, returning an edge referencing that new child.
      ///
 -    /// Assumes that this edge `.can_merge()`.
 +    /// Panics unless this edge `.can_merge()`.
      pub fn merge(
          mut self,
      ) -> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::Edge> {
          let self2 = unsafe { ptr::read(&self) };
          let mut left_node = self1.left_edge().descend();
          let left_len = left_node.len();
 -        let mut right_node = self2.right_edge().descend();
 +        let right_node = self2.right_edge().descend();
          let right_len = right_node.len();
  
 -        // necessary for correctness, but in a private module
          assert!(left_len + right_len < CAPACITY);
  
          unsafe {
  
              (*left_node.as_leaf_mut()).len += right_len as u16 + 1;
  
 -            let layout = if self.node.height > 1 {
 +            if self.node.height > 1 {
 +                // SAFETY: the height of the nodes being merged is one below the height
 +                // of the node of this edge, thus above zero, so they are internal.
 +                let mut left_node = left_node.cast_unchecked();
 +                let right_node = right_node.cast_unchecked();
                  ptr::copy_nonoverlapping(
 -                    right_node.cast_unchecked().as_internal().edges.as_ptr(),
 -                    left_node
 -                        .cast_unchecked()
 -                        .as_internal_mut()
 -                        .edges
 -                        .as_mut_ptr()
 -                        .add(left_len + 1),
 +                    right_node.reborrow().as_internal().edges.as_ptr(),
 +                    left_node.reborrow_mut().as_internal_mut().edges.as_mut_ptr().add(left_len + 1),
                      right_len + 1,
                  );
  
                  for i in left_len + 1..left_len + right_len + 2 {
 -                    Handle::new_edge(left_node.cast_unchecked().reborrow_mut(), i)
 -                        .correct_parent_link();
 +                    Handle::new_edge(left_node.reborrow_mut(), i).correct_parent_link();
                  }
  
 -                Layout::new::<InternalNode<K, V>>()
 +                Global.dealloc(right_node.node.cast(), Layout::new::<InternalNode<K, V>>());
              } else {
 -                Layout::new::<LeafNode<K, V>>()
 -            };
 -            Global.dealloc(right_node.node.cast(), layout);
 +                Global.dealloc(right_node.node.cast(), Layout::new::<LeafNode<K, V>>());
 +            }
  
              Handle::new_edge(self.node, self.idx)
          }
          unsafe {
              let (k, v, edge) = self.reborrow_mut().left_edge().descend().pop();
  
 -            let k = mem::replace(self.reborrow_mut().into_kv_mut().0, k);
 -            let v = mem::replace(self.reborrow_mut().into_kv_mut().1, v);
 +            let k = mem::replace(self.kv_mut().0, k);
 +            let v = mem::replace(self.kv_mut().1, v);
  
              match self.reborrow_mut().right_edge().descend().force() {
                  ForceResult::Leaf(mut leaf) => leaf.push_front(k, v),
          unsafe {
              let (k, v, edge) = self.reborrow_mut().right_edge().descend().pop_front();
  
 -            let k = mem::replace(self.reborrow_mut().into_kv_mut().0, k);
 -            let v = mem::replace(self.reborrow_mut().into_kv_mut().1, v);
 +            let k = mem::replace(self.kv_mut().0, k);
 +            let v = mem::replace(self.kv_mut().1, v);
  
              match self.reborrow_mut().left_edge().descend().force() {
                  ForceResult::Leaf(mut leaf) => leaf.push(k, v),
                  let left_kv = left_node.reborrow_mut().into_kv_pointers_mut();
                  let right_kv = right_node.reborrow_mut().into_kv_pointers_mut();
                  let parent_kv = {
 -                    let kv = self.reborrow_mut().into_kv_mut();
 +                    let kv = self.kv_mut();
                      (kv.0 as *mut K, kv.1 as *mut V)
                  };
  
                  let left_kv = left_node.reborrow_mut().into_kv_pointers_mut();
                  let right_kv = right_node.reborrow_mut().into_kv_pointers_mut();
                  let parent_kv = {
 -                    let kv = self.reborrow_mut().into_kv_mut();
 +                    let kv = self.kv_mut();
                      (kv.0 as *mut K, kv.1 as *mut V)
                  };
  
@@@ -1600,3 -1571,6 +1577,6 @@@ unsafe fn slice_remove<T>(slice: &mut [
          ret
      }
  }
+ #[cfg(test)]
+ mod tests;