From 349c20d9e15196d5aa143d836362b365cf77b511 Mon Sep 17 00:00:00 2001 From: sharnoff Date: Mon, 19 Sep 2022 20:53:41 -0700 Subject: [PATCH 1/2] Drain pt 1: `tree/iter` refactor Add `bounded_search_step` as shared component of iterator bound search logic, in 'src/tree/mod.rs'. Also move `panic_internal_error_or_bad_index` into 'src/tree/mod.rs' so that it can be used by `Drain` as well. --- src/tree/iter.rs | 147 +++++++++++------------------------------------ src/tree/mod.rs | 72 +++++++++++++++++++++++ 2 files changed, 106 insertions(+), 113 deletions(-) diff --git a/src/tree/iter.rs b/src/tree/iter.rs index 72913e0..e3d214f 100644 --- a/src/tree/iter.rs +++ b/src/tree/iter.rs @@ -14,7 +14,10 @@ use crate::MaybeDebug; use std::fmt::{self, Formatter}; use super::node::{borrow, ty, NodeHandle, SliceHandle, Type}; -use super::{search_step, ChildOrKey, SliceEntry, DEFAULT_MIN_KEYS}; +use super::{ + bounded_search_step, panic_internal_error_or_bad_index, ChildOrKey, SliceEntry, + DEFAULT_MIN_KEYS, +}; /// An iterator over a range of slices and their positions in an [`RleTree`] /// @@ -32,7 +35,8 @@ where P: RleTreeConfig, { start: I, - end: IncludedOrExcludedBound, + end: I, + end_bound_is_exclusive: bool, state: Option>, } @@ -176,21 +180,6 @@ impl<'t, I, S, P: RleTreeConfig, const M: usize> Debug for IterStack<'t } } -#[derive(Debug, Copy, Clone)] -enum IncludedOrExcludedBound { - Included(I), - Excluded(I), -} - -#[track_caller] -fn panic_internal_error_or_bad_index() -> ! { - if I::TRUSTED { - panic!("internal error"); - } else { - panic!("internal error or bad `Index` implementation"); - } -} - impl<'t, C, I, S, P, const M: usize> Iter<'t, C, I, S, P, M> where C: Cursor, @@ -221,13 +210,15 @@ where StartBound::Unbounded => I::ZERO, }; - let (end, strict_end) = match end_bound { - EndBound::Included(i) => (IncludedOrExcludedBound::Included(i), EndBound::Included(i)), - EndBound::Excluded(i) => (IncludedOrExcludedBound::Excluded(i), EndBound::Excluded(i)), - EndBound::Unbounded => ( - IncludedOrExcludedBound::Excluded(tree_size), - EndBound::Excluded(tree_size), - ), + let (end, end_bound_is_exclusive) = match end_bound { + EndBound::Included(i) => (i, false), + EndBound::Excluded(i) => (i, true), + EndBound::Unbounded => (tree_size, true), + }; + + let strict_end = match end_bound_is_exclusive { + true => EndBound::Excluded(end), + false => EndBound::Included(end), }; if range.starts_after_end() { @@ -242,18 +233,14 @@ where // Special case for empty iterators, so we don't attempt to traverse the tree when we won't // end up with anything. - let bounded_end_bound = match end { - IncludedOrExcludedBound::Included(i) => EndBound::Included(i), - IncludedOrExcludedBound::Excluded(i) => EndBound::Excluded(i), - }; - - if (StartBound::Included(start), bounded_end_bound).is_empty_naive() { + if (StartBound::Included(start), strict_end).is_empty_naive() { root = None; } Iter { start, end, + end_bound_is_exclusive, state: root.map(|(root, store)| IterState { store, root, @@ -267,80 +254,18 @@ where fn make_stack( root: NodeHandle, I, S, P, M>, cursor: Option, - target: IncludedOrExcludedBound, + mut target: I, + excluded: bool, ) -> IterStack<'t, I, S, P, M> { let mut stack = Vec::new(); let mut cursor = cursor.map(|c| c.into_path()); let mut head_node = root; let mut head_pos = I::ZERO; - let (mut target, excluded) = match target { - IncludedOrExcludedBound::Included(i) => (i, false), - IncludedOrExcludedBound::Excluded(i) => (i, true), - }; // Search downward with the cursor loop { let hint = cursor.as_mut().and_then(|c| c.next()); - - let result = if target != head_node.leaf().subtree_size() { - match search_step(head_node, hint, target) { - ChildOrKey::Key((k_idx, k_pos)) if excluded && k_pos == target => { - match head_node.into_typed() { - Type::Leaf(n) => { - if k_idx == 0 { - panic_internal_error_or_bad_index::(); - } - - // SAFETY: key indexes returned from `search_step` are guaranteed - // to be in bounds; we checked above that `k_idx != 0`, so - // subtracting one won't overlflow: `k_idx - 1` is still in bounds. - let lhs_k_pos = unsafe { n.leaf().key_pos(k_idx - 1) }; - ChildOrKey::Key((k_idx - 1, lhs_k_pos)) - } - Type::Internal(n) => { - let c_idx = k_idx; - // SAFETY: key indexes returned from `search_step` are guaranteed - // to be in bounds, so a child at the same index (i.e. to its left) - // will also be valid. - let c_pos = k_pos.sub_right(unsafe { n.child_size(c_idx) }); - ChildOrKey::Child((c_idx, c_pos)) - } - } - } - ChildOrKey::Child((c_idx, c_pos)) if excluded && c_pos == target => { - if c_idx == 0 { - panic_internal_error_or_bad_index::(); - } - - let k_idx = c_idx - 1; - // SAFETY: child indexes returned from `search_step` are guaranteed to be - // valid children, so the key to their left (at `c_idx - 1`) is a valid key - let k_pos = unsafe { head_node.leaf().key_pos(k_idx) }; - ChildOrKey::Key((k_idx, k_pos)) - } - res => res, - } - } else { - let len = head_node.leaf().len(); - - // Use the key or child immediately to the left. - match head_node.into_typed() { - Type::Leaf(_) => { - let i = len - 1; - // SAFETY: `len` is guaranteed to be `>= 1`, so `len - 1` can't overflow, - // and key indexes must be `< len`. - let p = unsafe { head_node.leaf().key_pos(i) }; - ChildOrKey::Key((i, p)) - } - Type::Internal(node) => { - // The child immediately left of the key has the same index - let c_idx = len; - // SAFETY: the index `len` is always a valid child index - let c_pos = unsafe { node.child_pos(c_idx) }; - ChildOrKey::Child((c_idx, c_pos)) - } - } - }; + let result = bounded_search_step(head_node, hint, target, excluded); match result { ChildOrKey::Key((k_idx, k_pos)) => { @@ -361,8 +286,8 @@ where Type::Internal(n) => n, }; - // SAFETY: `search_step` and our processing above guarantee that `c_idx` is a - // valid child index. + // SAFETY: `bounded_search_step` guarantees that `c_idx` is a valid child + // index. let child = unsafe { node.into_child(c_idx) }; // If we have COW enabled, and the child's parent is not this node, we need to // add `head_node` to the stack. @@ -587,16 +512,11 @@ where // Update `fwd` to the next entry: let next_start = fwd.end_pos; // ... but first, check that it'll be within bounds: - match self.end { - IncludedOrExcludedBound::Included(i) if i < next_start => { - self.state = None; - return None; - } - IncludedOrExcludedBound::Excluded(i) if i <= next_start => { - self.state = None; - return None; - } - _ => (), + if (self.end_bound_is_exclusive && self.end <= next_start) + || (!self.end_bound_is_exclusive && self.end < next_start) + { + self.state = None; + return None; } fwd.fwd_step(); @@ -604,11 +524,7 @@ where (fwd, next_start) } None => { - let s = Self::make_stack( - state.root, - state.cursor.take(), - IncludedOrExcludedBound::Included(self.start), - ); + let s = Self::make_stack(state.root, state.cursor.take(), self.start, false); // We can't use `self.start` for the start position because it might be in the // middle of a slice @@ -657,7 +573,12 @@ where bkwd } None => { - let s = Self::make_stack(state.root, state.cursor.take(), self.end); + let s = Self::make_stack( + state.root, + state.cursor.take(), + self.end, + self.end_bound_is_exclusive, + ); state.bkwd.insert(s) } diff --git a/src/tree/mod.rs b/src/tree/mod.rs index bae1b8d..cecdc69 100644 --- a/src/tree/mod.rs +++ b/src/tree/mod.rs @@ -823,6 +823,78 @@ fn search_step<'t, I: Index, S, P: RleTreeConfig, const M: usize>( } } +/// Custom function for iteration that's like [`search_step`], but goes to the key or child to the +/// target's immediate left if `excluded` is true +/// +/// This is primarily used for finding iteration endpoints, where the end may be exclusive. +fn bounded_search_step<'t, I, S, P, const M: usize>( + node: NodeHandle, I, S, P, M>, + hint: Option, + target: I, + excluded: bool, +) -> ChildOrKey<(u8, I), (u8, I)> +where + I: Index, + P: RleTreeConfig, +{ + let result = if target == node.leaf().subtree_size() { + debug_assert!(excluded); + ChildOrKey::Key((node.leaf().len(), target)) + } else { + search_step(node, hint, target) + }; + + if !excluded { + return result; + } + + // Because we have an excluded bound, check for key/child positions exactly matching `target`, + // and move to the previous key or child + match result { + ChildOrKey::Child((c_idx, c_pos)) if c_pos == target => { + if c_idx == 0 { + panic_internal_error_or_bad_index::(); + } + + let k_idx = c_idx - 1; + // SAFETY: child indexes returned from `search_step` are guaranteed to be valid + // children, so the key to their left (at `c_idx - 1`) is a valid key + let k_pos = unsafe { node.leaf().key_pos(k_idx) }; + ChildOrKey::Key((k_idx, k_pos)) + } + ChildOrKey::Key((k_idx, k_pos)) if k_pos == target => match node.typed_ref() { + Type::Leaf(n) => { + if k_idx == 0 { + panic_internal_error_or_bad_index::(); + } + + // SAFETY: key indexes returned from `search_step` are guaranteed to be in bounds, + // and our special case is also non-zero; we checked above that `k_idx != 0`, so + // subtracting one won't overlflow: `k_idx - 1` is still in bounds. + let lhs_k_pos = unsafe { n.leaf().key_pos(k_idx - 1) }; + ChildOrKey::Key((k_idx - 1, lhs_k_pos)) + } + Type::Internal(n) => { + let c_idx = k_idx; + // SAFETY: key indexes returned from `search_step` are guaranteed to be in bounds, + // so a child at the same index (i.e. to its left) will also be valid. + let c_pos = k_pos.sub_right(unsafe { n.child_size(c_idx) }); + ChildOrKey::Child((c_idx, c_pos)) + } + }, + r => r, + } +} + +#[track_caller] +fn panic_internal_error_or_bad_index() -> ! { + if I::TRUSTED { + panic!("internal error"); + } else { + panic!("internal error or bad `Index` implementation"); + } +} + macro_rules! define_node_box { ( $(#[$attrs:meta])* From 5be4ce8fa5610c2e1311cbd31471e200beba08d2 Mon Sep 17 00:00:00 2001 From: sharnoff Date: Wed, 5 Oct 2022 21:46:52 -0700 Subject: [PATCH 2/2] Drain pt 2: implement `Drain::{next,next_back}` This commit also adds the `DrainMut` borrow type for `{Node,Slice}Handle`s, to allow access to nodes in a way that doesn't need to uphold the initialization guarantees of slice values and `RefId`s (i.e., `Leaf.vals` and `Leaf.refs`). Hopefully the documentation contained there should be enough to understand it as is. --- src/tree/drain.rs | 863 +++++++++++++++++++++++++++++++++++++++++++++- src/tree/mod.rs | 19 +- src/tree/node.rs | 232 ++++++++++++- 3 files changed, 1090 insertions(+), 24 deletions(-) diff --git a/src/tree/drain.rs b/src/tree/drain.rs index 7ac7c70..39084aa 100644 --- a/src/tree/drain.rs +++ b/src/tree/drain.rs @@ -1,11 +1,18 @@ //! Wrapper module for [`RleTree`](crate::RleTree)'s destructive iterator -- [`Drain`] and related //! types -use crate::param::RleTreeConfig; -use crate::{Cursor, Index, Slice}; +use crate::param::{BorrowState, RleTreeConfig, SliceRefStore, SupportsInsert}; +use crate::public_traits::{Index, Slice}; +use crate::range::RangeBounds; +use crate::Cursor; +use std::mem::{self, ManuallyDrop}; use std::ops::Range; -use super::{Root, DEFAULT_MIN_KEYS}; +use super::node::{borrow, ty, NodeHandle, SliceHandle, Type}; +use super::{ + bounded_search_step, panic_internal_error_or_bad_index, ChildOrKey, Root, Side, + DEFAULT_MIN_KEYS, +}; /// A destructive iterator over a range in an [`RleTree`] /// @@ -22,43 +29,647 @@ pub struct Drain<'t, C, I, S, P, const M: usize = DEFAULT_MIN_KEYS> where P: RleTreeConfig, { + /// Total range of values returned by the iterator range: Range, - root: Option>, - initial_cursor: Option, - // The entire tree gets stored here, so that the + /// Stored reference to the tree, held so that we can put the root back once we're done + /// + /// ## Safety + /// + /// `tree_ref` must not be accessed before a later access to `state` or a value derived from + /// it. tree_ref: &'t mut Option>, + /// Copy of the tree's root, so that if the `Drain` is dropped, we don't leave the tree in an + /// invalid state + root: Option>, + /// The current state of iteration, if it has been started. Else, the initial cursor. The full + /// type will be `None` when `range` is empty. + /// + /// The lifetime `'t` here is technically invalid; we're actually borrowing `root`, and we have + /// to be careful in the destructor that we don't access `state` after dropping `root` or + /// moving it back into `tree_ref`. + state: ManuallyDrop, C>>>, +} + +/// (*Internal*) Result of a call to [`Drain::initial_search`] +/// +/// In general, the caller (`next` or `next_back`) is responsible for processing this search +/// result, adding to an existing or initial [`DrainState`] +struct DrainInitialSearch<'t, I, S, P: RleTreeConfig, const M: usize> { + slice: SliceHandle, I, S, P, M>, + /// Absolute position of `slice` + slice_pos: I, +} + +/// (*Internal*) Current state of the iterator, *after* it has started, and with a non-empty range +struct DrainState<'t, I, S, P: RleTreeConfig, const M: usize> { + start: Option>, + end: Option>, +} + +impl<'t, I, S, P: RleTreeConfig, const M: usize> DrainState<'t, I, S, P, M> { + /// Returns `true` iff iteration has been completed + /// + /// Iteration has only been completed when the cursors in `self.start` and `self.end` are + /// equal, an invariant that we guarantee during calls to `next` and `next_back`. + fn done(&self) -> bool { + if let (Some(start), Some(end)) = (&self.start, &self.end) { + start.cursor.slice == end.cursor.slice + } else { + false + } + } +} + +/// (*Internal*) One side of a [`DrainState`], encompassing both drained range bound +/// ([`DrainEdge`]) and current iteration position ([`DrainSideCursor`]) +struct DrainSide<'t, I, S, P: RleTreeConfig, const M: usize> { + edge: DrainEdge<'t, I, S, P, M>, + cursor: DrainSideCursor<'t, I, S, P, M>, +} + +/// (*Internal*) Static state associated with one side of the drained range +/// +/// The state tracking the progress of the iteration itself is given by [`DrainSideCursor`]. +struct DrainEdge<'t, I, S, P: RleTreeConfig, const M: usize> { + /// The slice containing the outermost key (for this edge) overapping with the drained edge + outermost_removed: SliceHandle, I, S, P, M>, + /// Absolute position of `outermost_removed` within the tree + slice_pos: I, + /// If the matching bound on `Drain.range` is in the middle of `outermost_removed`, `writeback` + /// stores the *size* and *value* of the key extracted from the outer side of the slice as it was + /// split to produce the outermost value for this side + /// + /// The value will be written back into `outermost_removed` (if possible; some edge cases + /// make this harder) when the tree is dropped + writeback: Option<(I, S)>, +} + +/// (*Internal*) Dynamic state (tracks iteration progress) of one side of the drained range +/// +/// The static state is stored in a [`DrainEdge`]. +/// +/// Broadly speaking, a `DrainSideCursor` stores a reference to the current position in our +/// iteration. Semantics are slightly different for the "start" vs "end" cursor: When a +/// `DrainSideCursor` is part of [`DrainState.start`] +/// +/// [`DrainState.start`]: DrainState#field.start +struct DrainSideCursor<'t, I, S, P: RleTreeConfig, const M: usize> { + /// Cursor's current position in the tree. Refer to top-level docs for semantics + slice: SliceHandle, I, S, P, M>, + + /// Absolute start position of `slice` + slice_pos: I, } impl<'t, C, I, S, P, const M: usize> Drain<'t, C, I, S, P, M> where - P: RleTreeConfig, + C: Cursor, + I: Index, + S: Slice, + P: RleTreeConfig + SupportsInsert, { + /// (*Internal*) Internal constructor for a `Drain`, panicking if the range is out of bounds + #[track_caller] pub(super) fn new(root: &'t mut Option>, range: Range, cursor: C) -> Self { + if range.starts_after_end() { + panic!("invalid range `{range:?}`"); + } else if range.start < I::ZERO { + panic!("range `{range:?}` out of bounds below zero"); + } + + let size = root + .as_ref() + .map(|r| r.handle.leaf().subtree_size()) + .unwrap_or(I::ZERO); + if range.end > size || range.start >= size { + panic!("range `{range:?}` out of bounds for tree size {size:?}"); + } + let (tree_ref, root) = { let r = root.take(); (root, r) }; + // If the range is empty, then we shouldn't remove anything. + let state = match range.start == range.end { + true => None, + false => Some(Err(cursor)), + }; + + // Mutably borrow the root. We'll release this borrow on drop. + if let Some(r) = root.as_ref() { + if let Err(conflict) = r.refs_store.acquire_mutable() { + panic!("{conflict}"); + } + } + Drain { range, root, - initial_cursor: Some(cursor), tree_ref, + state: ManuallyDrop::new(state), + } + } + + /// (*Internal*) Traverses the tree to produce a handle on the slice containing(-ish) the + /// target + /// + /// `excluded` is true for the upper bound only. + fn initial_search( + root: &mut Root, + cursor: Option, + other_side: Option<(Side, &DrainSideCursor<'t, I, S, P, M>)>, + mut target: I, + excluded: bool, + ) -> DrainInitialSearch<'t, I, S, P, M> { + let node: NodeHandle, I, S, P, M> = + root.handle.make_unique().borrow_mut().into_drain(); + // SAFETY: We're extending the lifetime on the root node in a way that is not valid by + // itself. We need to do this in order for the compiler to let us store references to + // `root` in `Drain` itself. We don't need the `Drain` to be pinned because the borrowed + // contents are of heap-allocated nodes, which won't move when `Drain` does. + let mut node: NodeHandle, I, S, P, M> = + unsafe { mem::transmute(node) }; + + let mut node_pos = I::ZERO; + let mut cursor = cursor.map(|c| c.into_path()); + + loop { + let hint = cursor.as_mut().and_then(|c| c.next()); + // SAFETY: `borrow_unchecked` requires that we never use the handle to produce a + // reference to a `Slice`, which `bounded_search_step` doesn't do. + let borrowed = unsafe { node.borrow_unchecked() }; + let result = bounded_search_step(borrowed, hint, target, excluded); + + match result { + ChildOrKey::Key((k_idx, k_pos)) => { + // SAFETY: `bounded_search_step` guarantees that `k_idx` is within the bounds + // of the node. + let slice = unsafe { node.into_slice_handle(k_idx) }; + if let Some((side, drain_cursor)) = other_side { + drain_check_valid_search_result(slice.copy_handle(), side, drain_cursor); + } + + let slice_pos = node_pos.add_right(k_pos); + return DrainInitialSearch { slice, slice_pos }; + } + ChildOrKey::Child((c_idx, c_pos)) => { + let this = match node.into_typed() { + Type::Leaf(_) => panic_internal_error_or_bad_index::(), + Type::Internal(n) => n, + }; + + // SAFETY: `bounded_search_step` guarantees that `c_idx` is a valid child + // index. + let child = unsafe { this.into_child(c_idx) }; + target = target.sub_left(c_pos); + node = child; + node_pos = node_pos.add_right(c_pos); + } + } + } + } + + /// Return the [`SliceHandle`] corresponding to the next (position-wise) slice in the tree + /// + /// ## Panics + /// + /// This methd panics by calling [`panic_internal_error_or_bad_index`] if this is the last + /// `SliceHandle` in the tree. + fn traverse_next( + slice: SliceHandle, I, S, P, M>, + ) -> SliceHandle, I, S, P, M> { + match slice.into_typed() { + Type::Leaf(mut h) if h.idx + 1 < h.node.leaf().len() => { + h.idx += 1; + return h.erase_type(); + } + Type::Leaf(h) => { + let mut node = h.node.erase_type(); + + // Recurse upwards to the right + loop { + let (parent, child_idx) = match node.into_parent() { + Err(_) => panic_internal_error_or_bad_index::(), + Ok(p) => p, + }; + node = parent.erase_type(); + + // If this is the last child, continue recursing + if node.leaf().len() == child_idx { + continue; + } + + // Otherwise, we pick the key immediately after the child, which has the + // same index as the child. + // + // SAFETY: `into_slice_handle` requires `child_idx <= parent.leaf().len()`, + // which is guaranteed by the check above (it cannot be greater). + return unsafe { node.into_slice_handle(child_idx) }; + } + } + Type::Internal(h) => { + let mut node = h.node; + let mut c_idx = h.idx + 1; // Child to the right is key index + 1 + + // Recurse downwards to the right + loop { + // SAFETY: `into_child` requires `c_idx <= node.leaf().len()`, which is + // guaranteed on every iteration after the first (`c_idx = 0`). For the + // first iteration, we're using `h.idx + 1`, which is ok because `h.idx` is + // guaranteed `< node.leaf().len()`. + let child = unsafe { node.into_child(c_idx) }; + match child.into_typed() { + Type::Leaf(h) => { + let n = h.erase_type(); + // SAFETY: valid trees are guaranteed to have len > 0 on each node, + // so getting key index 0 is ok. + return unsafe { n.into_slice_handle(0) }; + } + Type::Internal(n) => { + node = n; + c_idx = 0; + } + } + } + } + } + } + + /// Return the [`SliceHandle`] corresponding to the previous (position-wise) slice in the tree + /// + /// ## Panics + /// + /// This methd panics by calling [`panic_internal_error_or_bad_index`] if this is the last + /// `SliceHandle` in the tree. + fn traverse_next_back( + slice: SliceHandle, I, S, P, M>, + ) -> SliceHandle, I, S, P, M> { + match slice.into_typed() { + Type::Leaf(mut h) if h.idx > 0 => { + h.idx -= 1; + return h.erase_type(); + } + Type::Leaf(h) => { + // Note: h.idx == 0. + let mut node = h.node.erase_type(); + + // Recurse upwards to the left + loop { + let (parent, child_idx) = match node.into_parent() { + Err(_) => panic_internal_error_or_bad_index::(), + Ok(p) => p, + }; + node = parent.erase_type(); + + // If this is the first child, continue recursing + if child_idx == 0 { + continue; + } + + // Otherwise, we pick the key immediately before the child, which is at the + // child's index minus one + // + // SAFETY: `into_slice_handle` requires `child_idx <= parent.leaf().len()`, + // which is guaranteed because `child_idx != 0` means the subtraction won't + // overflow, and `child_idx <= parent.leaf().len()` already. + return unsafe { node.into_slice_handle(child_idx - 1) }; + } + } + Type::Internal(h) => { + let mut node = h.node; + let mut c_idx = h.idx; // Child to the left is equal to key index + + // Recurse downwards to the left + loop { + // SAFETY: `into_child` requires `c_idx <= node.leaf().len()`, which is + // guaranteed on every iteration after the first (`c_idx = node.leaf().len()`). + // For the first iteration, we're using `h.idx`, which is ok because `h.idx` is + // guaranteed `< node.leaf().len()`. + let child = unsafe { node.into_child(c_idx) }; + match child.into_typed() { + Type::Leaf(h) => { + let n = h.erase_type(); + let len = n.leaf().len(); + // SAFETY: `len != 0` is guaranteed for any vaild node + unsafe { weak_assert!(len != 0) }; + // SAFETY: `into_slice_handle` requires `idx < len`, which is true for + // `len - 1` as long as it doesn't overflow (which we checked above). + return unsafe { n.into_slice_handle(len - 1) }; + } + Type::Internal(n) => { + node = n; + c_idx = node.leaf().len(); + } + } + } + } } } } +/// (*Internal*) Check that a [`SliceHandle`] returned by [`initial_search`] is valid, given the +/// other side's iteration +/// +/// This method is necessary because it's possible for a bad `Index` implementation to yield a +/// second search result inside of or before what we've already returned from the iterator. +/// +/// This method is called before any return from [`initial_search`]. +/// +/// [`initial_search`]: Drain::initial_search +// We'd like this to be an associated function on `Drain`, but due to rust-lang/rust#102611 this +// has to be separated out into a standalone function. +// +// (link: https://github.com/rust-lang/rust/issues/102611) +fn drain_check_valid_search_result<'t, I, S, P, const M: usize>( + slice: SliceHandle, I, S, P, M>, + cursor_side: Side, + drain_cursor: &DrainSideCursor<'t, I, S, P, M>, +) where + I: Index, + P: RleTreeConfig, +{ + // There's two possibilities to be concerned about here: (a) `slice` is positionally within a + // range that's already been returned, or (b) `slice` is positionally in the opposite direction + // to the other bound of the drained range. + // + // These are both handled by checking if `slice` is on the "wrong" side of `drain_cursor`, + // basically: if its direction relative to `drain_cursor` is equal to `cursor_side`. + // + // --- + // + // Algorithm: + // + // We traverse up the tree from `slice` and `drain_cursor.node` until we get to a common + // ancestor, at which point we make the comparison with their positions and `cursor_side` to + // determine if `slice` is on the correct side. + + let mut slice_node = slice.node.copy_handle(); + let mut cursor_node = drain_cursor.slice.node.copy_handle(); + + let mut slice_idx = ChildOrKey::Key(slice.idx); + let mut cursor_idx = ChildOrKey::Key(drain_cursor.slice.idx); + + // Traverse up the tree: + loop { + if slice_node.ptr() == cursor_node.ptr() { + // SAFETY: Guaranteed by tree invariants. + unsafe { weak_assert!(slice_node.height() == cursor_node.height()) }; + break; + } + + // Move `slice_node` or `cursor_node` up the tree, depending on which one is further down. + // If they're at the same height, move both up because we know they're not the same node + // (from the check above). + + if slice_node.height() <= cursor_node.height() { + match slice_node.into_parent() { + Ok((h, c_idx)) => { + slice_node = h.erase_type(); + slice_idx = ChildOrKey::Child(c_idx); + } + // SAFETY: If this node has no parent, then either `cursor_node` is at a higher + // height than the root of the tree (tree invariant has been broken) or it is at + // the same height but must be a different node (invariant is still broken). So + // because we rely on tree invariants for soundness, it is no less sound to create + // UB here on failure. We're guaranteed to have "correct" parents from the downward + // traversal that created the `DrainMut` + Err(_) => unsafe { weak_unreachable!() }, + } + } + + if cursor_node.height() <= slice_node.height() { + match cursor_node.into_parent() { + Ok((h, c_idx)) => { + cursor_node = h.erase_type(); + cursor_idx = ChildOrKey::Child(c_idx); + } + // SAFETY: see not above for `slice_node.into_parent()`. + Err(_) => unsafe { weak_unreachable!() }, + } + } + } + + macro_rules! check { + ($cond:expr) => {{ + if $cond { + return; + } else { + panic_internal_error_or_bad_index::(); + } + }}; + } + + // Check for validity + match cursor_side { + // Cursor's to the left, so `slice` must be to its right. + Side::Left => match (slice_idx, cursor_idx) { + (ChildOrKey::Key(s), ChildOrKey::Key(c)) => check!(s >= c), + (ChildOrKey::Key(s), ChildOrKey::Child(c)) => check!(s >= c), + (ChildOrKey::Child(s), ChildOrKey::Key(c)) => check!(s > c), + (ChildOrKey::Child(s), ChildOrKey::Child(c)) => check!(s >= c), + }, + // Cursor's to the right, so `slice` must be to its left. + Side::Right => match (slice_idx, cursor_idx) { + (ChildOrKey::Key(s), ChildOrKey::Key(c)) => check!(s < c), + (ChildOrKey::Key(s), ChildOrKey::Child(c)) => check!(s < c), + (ChildOrKey::Child(s), ChildOrKey::Key(c)) => check!(s <= c), + (ChildOrKey::Child(s), ChildOrKey::Child(c)) => check!(s < c), + }, + } +} + impl<'t, C, I, S, P, const M: usize> Iterator for Drain<'t, C, I, S, P, M> where C: Cursor, I: Index, S: 't + Slice, - P: RleTreeConfig, + P: RleTreeConfig + SupportsInsert, { type Item = (Range, S); fn next(&mut self) -> Option { - todo!() + let root = self.root.as_mut()?; + let state_result = self.state.as_mut()?; + + // Helper value to abbreviate logic below. + let empty_state = DrainState { + start: None, + end: None, + }; + + let (start_side, end_side) = 'state: loop { + // ^ treat the loop as a way to short-circuit needing to find the `start` side of the + // range we're iterating over (i.e. skip it if we don't need to). + let (maybe_cursor, state) = match state_result.as_mut() { + Ok(state) if state.done() => return None, + Ok(state) => match state.start.as_mut() { + Some(s) => break 'state (s, &mut state.end), + None => { + assert!(state.end.is_some(), "internal error"); + (None, state) + } + }, + // If we haven't started yet, temporarily replace `state_result` with a fully-empty + // version of `DrainState` and then extract references to it. + Err(_) => match mem::replace(state_result, Ok(empty_state)) { + Err(cursor) => match state_result.as_mut() { + Ok(s) => (Some(cursor), s), + // SAFETY: we just set this to `Ok(_)` + Err(_) => unsafe { weak_unreachable!() }, + }, + // SAFETY: we just observed this as an `Err(_)` + Ok(_) => unsafe { weak_unreachable!() }, + }, + }; + + // Either we still have the cursor (nothing's been done yet) or `state.start` is None. + // However, `state` always needs to have at least one non-None side, so `state.end` + // must be `Some`. ( `!=` is xor ) + debug_assert!(maybe_cursor.is_some() != state.end.is_some()); + + let other_side = state.end.as_ref().map(|s| (Side::Right, &s.cursor)); + let mut search = + Self::initial_search(root, maybe_cursor, other_side, self.range.start, false); + let split_pos = search.slice_pos.sub_left(self.range.start); + + let mut writeback = None; + if split_pos != I::ZERO { + // We don't need to worry here about the special case where the two ends of the + // drained range are within the same slice; that's handled further in the call to + // `next`, where we limit the range of what we're returning to be bounded by the + // end of the drained range. + // + // SAFETY: `with_slice_unchecked` requires that the slice hasn't already been + // removed by a call to `remove_slice_unchecked`, which is guaranteed by our + // pre-return call to `check_valid_search_result` in `initial_search`. + unsafe { + search.slice.with_slice_unchecked(|s| { + // The left-hand side is the writeback. In order to return that, we have to + // put the split right-hand side back into the slice. + let rhs = s.split_at(split_pos); + let lhs = mem::replace(s, rhs); + writeback = Some(lhs); + }); + } + } + + let start_side = DrainSide { + edge: DrainEdge { + outermost_removed: search.slice.copy_handle(), + slice_pos: search.slice_pos, + writeback: writeback.map(|s| (split_pos, s)), + }, + cursor: DrainSideCursor { + slice: search.slice, + slice_pos: search.slice_pos, + }, + }; + break (state.start.insert(start_side), &mut state.end); + }; + + // At this point, we've been given `start_side` and `end_side`, and are free to produce the + // next item in the iteration (with certain checks to ensure we don't double-yield). + // + // --- + // + // Algorithm: + // + // Fetch immediate slice information from `start_side.cursor`. Define: + // * `range: Range` as the slice's range + // * `value: S` as the slice's value + // If `end_side` is `None` and `range.end >= self.range.end`, then (approx.) set `end_side` + // to `Some(start_side.cursor)` and appropriately hande its writeback. Otherwise, progress + // `start_side.cursor` to the next slice in the tree (performing whatever traversal is + // necessary). + + // slice_range: full range of the slice + let slice_range = { + let start = start_side.cursor.slice_pos; + let size = start_side.cursor.slice.slice_size(); + start..start.add_right(size) + }; + // range: range of the slice, with bounds clamped to `self.range` + let range = Range { + start: slice_range.start.max(self.range.start), + end: slice_range.end.min(self.range.end), + }; + + // SAFETY: `remove_slice_unchecked` requires that we haven't previously called that method + // for the same slice. This is guaranteed by the checks we use to ensure we don't + // double-yield a value, with `if state.is_done() => return None` above and everything in + // the remainder of this function. + let mut value = unsafe { start_side.cursor.slice.remove_slice_unchecked() }; + // SAFETY: `remove_refid_unchecked` has the same restrictions as `remove_slice_unchecked`, + // and our call here is sound for the same reason. + let ref_id = unsafe { start_side.cursor.slice.remove_refid_unchecked() }; + root.refs_store.remove(ref_id); + + if end_side.is_none() && slice_range.end >= self.range.end { + // From above: "If `end_side` is `None` and `slice_range.end >= self.range.end`, then + // set `end_side` to `Some(start_side.cursor)` and appropriately handle its writeback." + + let end_ref = end_side.insert(DrainSide { + edge: DrainEdge { + outermost_removed: start_side.cursor.slice.copy_handle(), + slice_pos: slice_range.start, + writeback: None, // <- we'll set this below if we need to + }, + cursor: DrainSideCursor { + slice: start_side.cursor.slice.copy_handle(), + slice_pos: slice_range.start, + }, + }); + + // equal to: slice_range.end > self.range.end + if slice_range.end != range.end { + // [diagram note: area marked as '~' will be removed.] + // + // Removal not contained within the slice: + // + // slice: |~~~~~~~~~~~~~~~~~~~~~~~~~/-----------| + // | self.range.end + // slice_range.start + // + // Removal contained within the slice: + // + // slice: |----------/~~~~~~~~~~~~~~/-----------| + // | | self.range.end + // | self.range.start + // slice_range.start + // + // With the slice ranging between the two '|'s, we can see that the split position + // to use *after* any necessary left-hand split has been done will be given by + // `range.end - range.start`, because: `range.start` is the maximum of `slice_pos` + // and `self.range.start`; and `range.end` is currently equal to `self.range.end`. + + let splitpoint = range.end.sub_left(range.start); + let split_size = slice_range.end.sub_left(range.end); + let split_val = value.split_at(splitpoint); + end_ref.edge.writeback = Some((split_size, split_val)); + } + } else if cfg!(debug_assertions) && slice_range.end != range.end { + // Above condition implies end_side.is_some() and range.end > self.range.end, which + // shouldn't be true. + panic_internal_error_or_bad_index::(); + } else { + // From above: "... Otherwise, progress `start_side.cursor` to the next slice in the + // tree (performing whatever traversal is necessary)." + // + // For concerns about double-yielding a value: We already extracted `value`, which is + // guaranteed to be fine because of the invariants of `DrainSideCursor`. The traversal + // here exists to ensure that we appropriately update `start_side` so that a call to + // `next_back` doesn't find the same value (if it otherwise would have, then + // `start_side.cursor == end_side.cursor` by the end of this). + // + // For concerns about walking off the end of the tree: If we get to the end of the + // tree, something went wrong -- either our fault or a bad index impl (we panic). + + start_side.cursor.slice = Self::traverse_next(start_side.cursor.slice.copy_handle()); + start_side.cursor.slice_pos = slice_range.end; + } + + Some((range, value)) } } @@ -67,9 +678,237 @@ where C: Cursor, I: Index, S: 't + Slice, - P: RleTreeConfig, + P: RleTreeConfig + SupportsInsert, { fn next_back(&mut self) -> Option { - todo!() + let root = self.root.as_mut()?; + let state_result = self.state.as_mut()?; + + // Helper value to abbreviate logic below. + let empty_state = DrainState { + start: None, + end: None, + }; + + let (needs_traversal, start_side, end_side) = 'state: loop { + // ^ treat the loop as a way to short-circuit needing to find the `start` side of the + // range we're iterating over (i.e. skip it if we don't need to). + let (maybe_cursor, state) = match state_result.as_mut() { + Ok(state) if state.done() => return None, + Ok(state) => match state.end.as_mut() { + Some(s) => break 'state (true, &mut state.start, s), + None => { + assert!(state.start.is_some(), "internal error"); + (None, state) + } + }, + // If we haven't started yet, temporarily replace `state_result` with a fully-empty + // version of `DrainState` and then extract references to it. + Err(_) => match mem::replace(state_result, Ok(empty_state)) { + Err(cursor) => match state_result.as_mut() { + Ok(s) => (Some(cursor), s), + // SAFETY: we just set this to `Ok(_)` + Err(_) => unsafe { weak_unreachable!() }, + }, + // SAFETY: we just observed this as an `Err(_)` + Ok(_) => unsafe { weak_unreachable!() }, + }, + }; + + // Either we still have the cursor (nothing's been done yet) or `state.end` is None. + // However, `state` always needs to have at least one non-None side, so `state.start` + // must be `Some`. ( `!=` is xor ) + debug_assert!(maybe_cursor.is_some() != state.start.is_some()); + + let other_side = state.start.as_ref().map(|s| (Side::Left, &s.cursor)); + let mut search = + Self::initial_search(root, maybe_cursor, other_side, self.range.end, true); + let split_pos = self.range.end.sub_left(search.slice_pos); + let slice_size = search.slice.slice_size(); + + let mut writeback = None; + if split_pos != slice_size { + // We don't need to worry here about the special case where the two ends of the + // drained range are within the same slice; that's handled further in the call to + // `next_back`, where we limit the range of what we're returning to be bounded by + // the end of the drained range. + // + // SAFETY: `with_slice_unchecked` requires that the slice hasn't already been + // removed by a call to `remove_slice_unchecked`, which is guaranteed by our + // pre-return call to `check_valid_search_result` in `initial_search`. + let rhs = unsafe { search.slice.with_slice_unchecked(|s| s.split_at(split_pos)) }; + let writeback_size = slice_size.sub_left(split_pos); + writeback = Some((split_pos, rhs)); + } + + let end_side = DrainSide { + edge: DrainEdge { + outermost_removed: search.slice.copy_handle(), + slice_pos: search.slice_pos, + writeback, + }, + cursor: DrainSideCursor { + slice: search.slice, + slice_pos: search.slice_pos, + }, + }; + + break (false, &mut state.start, state.end.insert(end_side)); + }; + + // At this point, we've been given `start_side` and `end_side`, and are free to produce the + // next item in the iteration (with certain checks to ensure we don't double-yield). + // + // --- + // + // Algorithm: + // + // If we didn't just create `end_side`, progress `end_side.cursor` to the previous slice in + // the tree (performing whatever traversal is necessary). Update `end_side.cursor.slice_pos` + // Fetch immediate slice information from `end_side.cursor`. Define: + // * `range: Range` as the slice's range + // * `value: S` as the slice's value + // If `start_side` is `None` and `slice_range.start <= self.range.start`, then (approx.) + // set `start_side` to `Some(end_side.cursor)` and appropriately handle its writeback. + + // slice_range: full range of the slice. Also handle traversal here. Also update + // `end_side.cursor.slice_pos` + let slice_range = if needs_traversal { + end_side.cursor.slice = Self::traverse_next_back(end_side.cursor.slice.copy_handle()); + + let end = end_side.cursor.slice_pos; + let size = end_side.cursor.slice.slice_size(); + let start = end.sub_right(size); + + end_side.cursor.slice_pos = start; + start..end + } else { + let start = end_side.cursor.slice_pos; + let size = end_side.cursor.slice.slice_size(); + start..start.add_right(size) + }; + + // range: range of the slice, with bounds clamped to `self.range` + let range = Range { + start: slice_range.start.max(self.range.start), + end: slice_range.end.min(self.range.end), + }; + + // SAFETY: `remove_slice_unchecked` requires that we haven't previously called that method + // for the same slice. This is guaranteed by the checks we use to ensure we don't + // double-yield a value, with `if state.is_done() => return None` above and everything in + // the remainder of this function (plus traversal above). + let mut value = unsafe { end_side.cursor.slice.remove_slice_unchecked() }; + // SAFETY: `remove_refid_unchecked` has the same restrictions as `remove_slice_unchecked`, + // and our call here is sound for the same reason. + let ref_id = unsafe { end_side.cursor.slice.remove_refid_unchecked() }; + root.refs_store.remove(ref_id); + + if start_side.is_none() && slice_range.start <= self.range.start { + // From above: "If `start_side` is `None` and `slice_range.start <= self.range.start`, + // then set `start_side` to `Some(end_side.cursor)` and appropriately handle its + // writeback." + + let start_ref = start_side.insert(DrainSide { + edge: DrainEdge { + outermost_removed: end_side.cursor.slice.copy_handle(), + slice_pos: slice_range.start, + writeback: None, // <- we'll set this below if we need to + }, + cursor: DrainSideCursor { + slice: end_side.cursor.slice.copy_handle(), + slice_pos: slice_range.start, + }, + }); + + // equal to: slice_range.start < self.range.start + if slice_range.start != range.start { + // [diagram note: area marked as '~' will be removed] + // + // Removal not contained within the slice: + // + // slice: |----------/~~~~~~~~~~~~~~~~~~~~~~~~~~| + // self.range.start | + // slice_range.end + // + // Removal contained within the slice: + // + // slice: |----------/~~~~~~~~~~~~~~/-----------| + // self.range.start | | + // self.range.end | + // slice_range.end + // + // With the slice ranging between teh two '|'s, we can see that the split position + // to use *after* any necessary right-hand split has been done will be equal to + // `range.start - slice_range.start`, because `range.start` is the maximum of + // `slice_pos` and `self.range.start` (and `slice_pos < self.range.start`). + let splitpoint = range.start; + let rhs = value.split_at(splitpoint); + let split_val = mem::replace(&mut value, rhs); + start_ref.edge.writeback = Some((splitpoint, split_val)); + } + } else if cfg!(debug_assertions) && slice_range.start != range.start { + // Above condition implies start_side.is_some() and range.start < self.range.start, + // which shouldn't be true. + panic_internal_error_or_bad_index::(); + } + + // All done :) + Some((range, value)) + } +} + +#[cfg(not(feature = "nightly"))] +impl<'t, C, I, S, P, const M: usize> Drop for Drain<'t, C, I, S, P, M> +where + P: RleTreeConfig, +{ + fn drop(&mut self) { + // SAFETY: `do_drop` requires that the function is called as the the destructor + unsafe { self.do_drop() } + } +} + +// Note: `I` cannot dangle because we fix the tree in the destructor, which requires operations +// using `I`. +#[cfg(feature = "nightly")] +unsafe impl<'t, #[may_dangle] C, I, #[may_dangle] S, P, const M: usize> Drop + for Drain<'t, I, S, P, M> +where + P: RleTreeConfig, +{ + fn drop(&mut self) { + // SAFETY: `do_drop` requires that the function is called as the the destructor + unsafe { self.do_drop() } + } +} + +impl<'t, C, I, S, P: RleTreeConfig, const M: usize> Drain<'t, C, I, S, P, M> { + /// Implementation of `Drain`'s destructor, factored out so that we can have different + /// attributes for `#[cfg(feature = "nightly")]` + /// + /// ## Safety + /// + /// `do_drop` can only be called as the implementation of `Drain`'s destructor + unsafe fn do_drop(&mut self) { + // SAFETY: Guaranteed by the caller that this is called only during the destructor, so + // this is called exactly once. We don't drop `self.state` elsewhere in this function, so + // we're good to `take` from the ManuallyDrop. + let state = unsafe { ManuallyDrop::take(&mut self.state) }; + + match state { + None => (), + Some(Err(cursor)) => drop(cursor), + Some(Ok(_)) => todo!(), + } + + // Release the mutable borrow initially acquired in `Drain::new` + if let Some(root) = self.root.as_ref() { + root.refs_store.release_mutable(); + } + + // SAFETY: We're not allowed to do anything with `tree_ref` until we're absolutely done + // with `state`, which we are. + *self.tree_ref = self.root.take(); } } diff --git a/src/tree/mod.rs b/src/tree/mod.rs index cecdc69..fa984d9 100644 --- a/src/tree/mod.rs +++ b/src/tree/mod.rs @@ -538,11 +538,23 @@ where /// the range will be removed during `Drain`'s destructor. If the destructor is never called, /// the tree will be left in an unspecified (but safe) state. It may leak memory. /// - /// This method is equivalent to calling [`drain_with_cursor`] with [`NoCursor`]. + /// **Note:** `Drain` requires that `S: Clone` for COW-enabled trees. + /// + /// Also: This method is equivalent to calling [`drain_with_cursor`] with [`NoCursor`]. + /// + /// ## Panics + /// + /// This method panics if the range is out of bounds or invalid: if `range.start > range.end`, + /// `range.start < I::ZERO`, or `range.end > self.size()`. /// /// [`drain_with_cursor`]: Self::drain_with_cursor - pub fn drain(&mut self, range: Range) -> Drain<'_, NoCursor, I, S, P, M> { - self.drain_with_cursor(NoCursor, range) + pub fn drain(&mut self, range: Range) -> Drain<'_, NoCursor, I, S, P, M> + where + P: SupportsInsert, + { + // Directly call `Drain::new`, even though we *could* use `drain_with_cursor`, so panic + // locations are better + Drain::new(&mut self.root, range, NoCursor) } /// Like [`drain`], but uses a [`Cursor`] to provide a hint on the first call to `next` or @@ -557,6 +569,7 @@ where pub fn drain_with_cursor(&mut self, cursor: C, range: Range) -> Drain<'_, C, I, S, P, M> where C: Cursor, + P: SupportsInsert, { Drain::new(&mut self.root, range, cursor) } diff --git a/src/tree/node.rs b/src/tree/node.rs index bb08f28..951276a 100644 --- a/src/tree/node.rs +++ b/src/tree/node.rs @@ -129,6 +129,7 @@ where impl Debug for NodeHandle where Ty: TypeHint, + B: borrow::Valid, P: RleTreeConfig, { fn fmt(&self, f: &mut Formatter) -> fmt::Result { @@ -152,6 +153,7 @@ where impl Debug for SliceHandle where Ty: TypeHint, + B: borrow::Valid, P: RleTreeConfig, { fn fmt(&self, f: &mut Formatter) -> fmt::Result { @@ -309,6 +311,12 @@ pub(super) mod borrow { /// that they don't produce aliasing issues. `miri` (i.e. Stacked Borrows) only accepts it /// because the entire thing uses raw pointers. pub struct Mut<'a>(PhantomData<&'a mut ()>); + /// Like [`Mut`], but for use within the implementation of `Drain` + /// + /// This borrow type is notable because it does *not* need to abide by the normal + /// initialization rules for `Leaf.vals` or `Leaf.refs`. As such, there are a few methods that + /// are not available for `DrainMut` that are available for other types of borrows. + pub struct DrainMut<'a>(PhantomData<&'a mut ()>); /// Borrow that's used only for dropping the tree, produced by [`NodeHandle::try_drop`] or /// [`NodeHandle::into_drop`] /// @@ -316,11 +324,29 @@ pub(super) mod borrow { /// [`NodeHandle::into_drop`]: super::NodeHandle::into_drop pub struct Dropping; + /// Marker trait for borrow types that guarantee the tree they provide access to is valid + /// + /// After the end of each operation - even if it panics and ends prematurely - the tree will + /// remain valid. + /// + /// Note that "valid" here does not have the same meaning as in `RleTree::validate`. Here, + /// validity refers only to the initialization guarantees for keys, `RefId`s, and slices. + pub trait Valid {} + impl Valid for Owned {} + impl Valid for UniqueOwned {} + impl Valid for SliceRef {} + impl<'a> Valid for Immut<'a> {} + impl<'a> Valid for Mut<'a> {} + // Technically speaking, panic during drop can leave the tree in an invalid state, but because + // it's unsound to double-drop, that invalid state can't be observed, so this impl is still ok + impl<'a> Valid for Dropping {} + /// Marker trait for borrow types that are not [`Owned`], [`UniqueOwned`], or [`Dropping`] pub trait NotOwned {} impl NotOwned for SliceRef {} impl<'a> NotOwned for Immut<'a> {} impl<'a> NotOwned for Mut<'a> {} + impl<'a> NotOwned for DrainMut<'a> {} /// Marker for borrow types that can be immutably borrowed pub trait AsImmut {} @@ -374,6 +400,16 @@ pub(super) mod borrow { { type Param = P; + fn cast_ref_if_mut(r: &mut NodePtr) -> Option<&mut NodePtr> { + Some(r) + } + } + impl<'a, I, S, P, const M: usize> SupportsInsertIfMut for DrainMut<'a> + where + P: RleTreeConfig + SupportsInsert, + { + type Param = P; + fn cast_ref_if_mut(r: &mut NodePtr) -> Option<&mut NodePtr> { Some(r) } @@ -806,7 +842,7 @@ where // * into_parent // * into_slice_handle (where B: borrow::NotOwned) // * try_child_size (where I: Copy) -// * shallow_clone (where I: Index, P: SupportsInsert) +// * shallow_clone (where B: borrow::Valid + borrow::Immut, I: Index, P: SupportsInsert) // * increase_strong_count_and_clone impl NodeHandle where @@ -826,6 +862,10 @@ where } /// Returns a reference to the inner `Leaf` + /// + /// This method does not require `B: borrow::Valid` only because the methods on `Leaf` don't + /// permit access that would be unsound if the node was invalid (in the ways allowed by + /// [`borrow::Valid`]). pub(super) fn leaf(&self) -> &Leaf { // SAFETY: references to `Internal` nodes can be cast to `Leaf`s, as described in the // "safety" comment for `Internal` -- so regardless of the type of the node, we can read it @@ -1026,7 +1066,7 @@ where /// This function requires that `P = AllowCow`, and *will* trigger UB if that is not true. pub unsafe fn shallow_clone(&self) -> NodeHandle where - B: borrow::AsImmut, + B: borrow::Valid + borrow::AsImmut, I: Index, P: SupportsInsert, { @@ -1589,6 +1629,47 @@ where } } +// any type, borrow::DrainMut +// * borrow_unchecked +// * copy_handle +impl<'t, Ty, I, S, P, const M: usize> NodeHandle, I, S, P, M> +where + Ty: TypeHint, + P: RleTreeConfig, +{ + /// Immutably borrows the `DrainMut` node + /// + /// ## Safety + /// + /// Unlike [`NodeHandle::borrow`], which is not unsafe, this method requires that the caller + /// make sure that no references to any slices are produced. + pub unsafe fn borrow_unchecked<'h>(&'h self) -> NodeHandle, I, S, P, M> { + // Unfortunately, there isn't really a good way to check this. We're kind of left to our + // best bet of `miri`, creative test cases, and a bit of fuzzing. + NodeHandle { + ptr: self.ptr, + height: self.height, + borrow: PhantomData, + } + } + + /// Copies the handle on the node + /// + /// This method is safe because all of the other methods on `borrow::Mut` or `borrow::Drain` + /// *already* assume that there may be multiple mutable handles active at the same time. For + /// example, the safety requirements for [`with_mut`] explicitly require that references are + /// not aliased. + /// + /// [`with_mut`]: Self::with_mut + pub fn copy_handle(&self) -> Self { + NodeHandle { + ptr: self.ptr, + height: self.height, + borrow: self.borrow, + } + } +} + // ty::Unknown, borrow::Owned // * try_drop // * make_unique (where I: Index, P: SupportsInsert) @@ -2124,6 +2205,7 @@ where // ty::Unknown, borrow::Mut // * split_slice_handle +// * into_drain impl<'t, Ty, I, S, P, const M: usize> NodeHandle, I, S, P, M> where Ty: TypeHint, @@ -2147,6 +2229,15 @@ where idx: key_idx, } } + + /// Converts this mutable borrow into one that does not need the node it references to be valid + pub fn into_drain(self) -> NodeHandle, I, S, P, M> { + NodeHandle { + ptr: self.ptr, + height: self.height, + borrow: PhantomData, + } + } } // ty::Leaf, borrow::Mut @@ -2892,18 +2983,18 @@ where // Any params // * erase_type +// * into_typed // * key_pos // * slice_size // * is_hole // * clone_slice_ref // * clone_immut -// * take_refid -// * replace_refid -// * redirect_to +// * take_refid (where B: borrow::Valid) +// * replace_refid (where B: borrow::Valid) +// * redirect_to (where B: borrow::Valid) impl SliceHandle where Ty: TypeHint, - B: borrow::AsImmut, P: RleTreeConfig, { /// Converts this `SliceHandle` into one with `ty::Unknown` instead of the current type tag @@ -2914,6 +3005,20 @@ where } } + /// Converts this `SliceHandle` into one where the type has been resolved + pub fn into_typed(self) -> Type<::Leaf, ::Internal> { + match self.node.into_typed() { + Type::Leaf(node) => Type::Leaf(SliceHandle { + node, + idx: self.idx, + }), + Type::Internal(node) => Type::Internal(SliceHandle { + node, + idx: self.idx, + }), + } + } + /// Returns the position of the slice within its node pub fn key_pos(&self) -> I where @@ -2978,7 +3083,10 @@ where } /// Removes the `RefId` associated with the slice and returns it, if there is one - pub fn take_refid(&self) -> resolve![P::SliceRefStore::OptionRefId] { + pub fn take_refid(&self) -> resolve![P::SliceRefStore::OptionRefId] + where + B: borrow::Valid, + { let default = ::default(); self.replace_refid(default) } @@ -2987,7 +3095,10 @@ where pub fn replace_refid( &self, new_ref: resolve![P::SliceRefStore::OptionRefId], - ) -> resolve![P::SliceRefStore::OptionRefId] { + ) -> resolve![P::SliceRefStore::OptionRefId] + where + B: borrow::Valid, + { unsafe { let unsafecell = self .node @@ -3000,7 +3111,10 @@ where } /// Informs the `SliceRefStore` to redirect any references to `self` to `other` - pub fn redirect_to(&self, other: &Self, store: &mut resolve![P::SliceRefStore]) { + pub fn redirect_to(&self, other: &Self, store: &mut resolve![P::SliceRefStore]) + where + B: borrow::Valid, + { // SAFETY: in general, producing a reference to something in `refs` is only valid as long // as we don't reentrantly call anything that would also modify the value, which we happen // to know `SliceRefStore` won't do for the call to `redirect`. @@ -3150,6 +3264,106 @@ where } } +// any type, borrow::DrainMut +// * copy_handle +// * with_slice_unchecked +// * remove_slice_unchecked +// * remove_refid_unchecked +impl<'t, Ty, I, S, P, const M: usize> SliceHandle, I, S, P, M> +where + Ty: TypeHint, + P: RleTreeConfig, +{ + /// Copies the handle on the slice + /// + /// See also: [`NodeHandle::copy_handle`] + pub fn copy_handle(&self) -> Self { + SliceHandle { + node: self.node.copy_handle(), + idx: self.idx, + } + } + + /// Version of [`with_slice`] for `DrainMut`, where arbitrary slices within a node may be + /// uninitialized without being marked as such + /// + /// ## Safety + /// + /// The slice pointed to by this `SliceHandle` *must not* have already been removed by a call + /// to [`remove_slice_unchecked`]. + /// + /// ## Panics + /// + /// Exactly the same panics as [`with_slice`]. + /// + /// [`with_slice`]: Self::with_slice + /// [`remove_slice_unchecked`]: Self::remove_slice_unchecked + pub unsafe fn with_slice_unchecked(&mut self, func: impl FnOnce(&mut S) -> R) -> R { + // Create a `borrow::Mut` handle equivalent to `self` for us to use. + let mut h: SliceHandle = SliceHandle { + idx: self.idx, + node: NodeHandle { + ptr: self.node.ptr, + height: self.node.height, + borrow: PhantomData, + }, + }; + + h.with_slice(func) + } + + /// Like [`take_value_and_leave_hole`], but does not leave a hole behind + /// + /// ## Safety + /// + /// This method may produce UB if the slice has already been removed with a prior call to + /// `remove_slice_unchecked` for the same slice. + /// + /// ## Panics + /// + /// This method panics if `self.is_hole()`. + /// + /// [`take_value_and_leave_hole`]: Self::take_value_and_leave_hole + pub unsafe fn remove_slice_unchecked(&mut self) -> S { + assert!(!self.is_hole()); + + // Create a `borrow::Mut` handle equivalent to `self.node` for us to use: + let mut h: NodeHandle = NodeHandle { + ptr: self.node.ptr, + height: self.node.height, + borrow: PhantomData, + }; + + unsafe { + h.with_mut(|leaf| { + leaf.vals + .get_unchecked(self.idx as usize) + .assume_init_read() + }) + } + } + + /// Like [`take_refid`], but leaves the `RefId`'s slot uninitialized + /// + /// ## Safety + /// + /// This method may produce UB if the `RefId` has already been removed with a prior call to + /// `remove_refid_unchecked` for the same slice. + /// + /// [`take_refid`]: SliceHandle::take_refid + pub unsafe fn remove_refid_unchecked(&mut self) -> resolve![P::SliceRefStore::OptionRefId] { + unsafe { + let unsafecell = self + .node + .leaf() + .refs + .get_unchecked(self.idx as usize) + .assume_init_read(); + unsafecell.into_inner() + } + } +} + // ty::Unknown, borrow::SliceRef // * borrow_slice // * range