Skip to content

Commit

Permalink
Remove AsBytes bound from most places
Browse files Browse the repository at this point in the history
**Motivation**
The `AsBytes` bound is only required in code path where the key type
needs to be converted into `&[u8]`. Having it in other locations
clutters the set of trait bounds required and causes it to
propagate other places.

**Testing Done**
`./scripts/full-testing.sh nightly`
  • Loading branch information
declanvk committed Jul 8, 2024
1 parent ed05330 commit a00708d
Show file tree
Hide file tree
Showing 16 changed files with 159 additions and 198 deletions.
43 changes: 26 additions & 17 deletions src/collections/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ pub use entry_ref::*;
pub use iterators::*;

/// An ordered map based on an adaptive radix tree.
pub struct TreeMap<K: AsBytes, V, const PREFIX_LEN: usize = 16> {
pub struct TreeMap<K, V, const PREFIX_LEN: usize = 16> {
/// The number of entries present in the tree.
num_entries: usize,
/// A pointer to the tree root, if present.
pub(crate) root: Option<OpaqueNodePtr<K, V, PREFIX_LEN>>,
}

impl<K: AsBytes, V> TreeMap<K, V> {
impl<K, V> TreeMap<K, V> {
/// Create a new, empty [`crate::TreeMap`] with the default number of prefix
/// bytes (16).
///
Expand All @@ -45,7 +45,7 @@ impl<K: AsBytes, V> TreeMap<K, V> {
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
/// Create a new, empty [`crate::TreeMap`].
///
/// This function will not pre-allocate anything.
Expand Down Expand Up @@ -559,10 +559,7 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
fn apply_delete_point(
&mut self,
delete_point: DeletePoint<K, V, PREFIX_LEN>,
) -> DeleteResult<K, V, PREFIX_LEN>
where
K: AsBytes,
{
) -> DeleteResult<K, V, PREFIX_LEN> {
// SAFETY: The root is sure to not be `None`, since the we somehow got a
// `DeletePoint`. So the caller must have checked this
let delete_result = delete_point.apply(unsafe { self.root.unwrap_unchecked() });
Expand Down Expand Up @@ -1116,7 +1113,10 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
///
/// assert_eq!(p, vec![(&c"abcde", &0), (&c"abcdexxx", &0), (&c"abcdexxy", &0)]);
/// ```
pub fn prefix<'a, 'b>(&'a self, prefix: &'b [u8]) -> Prefix<'a, 'b, K, V, PREFIX_LEN> {
pub fn prefix<'a, 'b>(&'a self, prefix: &'b [u8]) -> Prefix<'a, 'b, K, V, PREFIX_LEN>
where
K: AsBytes,
{
Prefix::new(self, prefix)
}

Expand All @@ -1139,10 +1139,10 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
///
/// assert_eq!(p, vec![(&c"abcde", &mut 0), (&c"abcdexxx", &mut 0), (&c"abcdexxy", &mut 0)]);
/// ```
pub fn prefix_mut<'a, 'b>(
&'a mut self,
prefix: &'b [u8],
) -> PrefixMut<'a, 'b, K, V, PREFIX_LEN> {
pub fn prefix_mut<'a, 'b>(&'a mut self, prefix: &'b [u8]) -> PrefixMut<'a, 'b, K, V, PREFIX_LEN>
where
K: AsBytes,
{
PrefixMut::new(self, prefix)
}

Expand All @@ -1164,7 +1164,10 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
///
/// assert_eq!(p, vec![&c"abcde", &c"abcdexxx", &c"abcdexxy"]);
/// ```
pub fn prefix_keys<'a, 'b>(&'a self, prefix: &'b [u8]) -> PrefixKeys<'a, 'b, K, V, PREFIX_LEN> {
pub fn prefix_keys<'a, 'b>(&'a self, prefix: &'b [u8]) -> PrefixKeys<'a, 'b, K, V, PREFIX_LEN>
where
K: AsBytes,
{
PrefixKeys::new(self, prefix)
}

Expand All @@ -1189,7 +1192,10 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
pub fn prefix_values<'a, 'b>(
&'a self,
prefix: &'b [u8],
) -> PrefixValues<'a, 'b, K, V, PREFIX_LEN> {
) -> PrefixValues<'a, 'b, K, V, PREFIX_LEN>
where
K: AsBytes,
{
PrefixValues::new(self, prefix)
}

Expand All @@ -1215,7 +1221,10 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
pub fn prefix_values_mut<'a, 'b>(
&'a mut self,
prefix: &'b [u8],
) -> PrefixValuesMut<'a, 'b, K, V, PREFIX_LEN> {
) -> PrefixValuesMut<'a, 'b, K, V, PREFIX_LEN>
where
K: AsBytes,
{
PrefixValuesMut::new(self, prefix)
}

Expand Down Expand Up @@ -1251,7 +1260,7 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
/// Tries to get the given key’s corresponding entry in the map for in-place
/// manipulation.
pub fn try_entry(&mut self, key: K) -> Result<Entry<K, V, PREFIX_LEN>, InsertPrefixError>
Expand Down Expand Up @@ -1352,7 +1361,7 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> TreeMap<K, V, PREFIX_LEN> {
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> Drop for TreeMap<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> Drop for TreeMap<K, V, PREFIX_LEN> {
fn drop(&mut self) {
self.clear();
}
Expand Down
10 changes: 2 additions & 8 deletions src/collections/map/entry_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@ use crate::{AsBytes, DeletePoint, InsertPoint, LeafNode, NodePtr, OpaqueNodePtr,

/// A view into an occupied entry in a [`TreeMap`]. It is part of the
/// [`EntryRef`] enum.
pub struct OccupiedEntryRef<'a, K, V, const PREFIX_LEN: usize>
where
K: AsBytes,
{
pub struct OccupiedEntryRef<'a, K, V, const PREFIX_LEN: usize> {
pub(crate) leaf_node_ptr: NodePtr<PREFIX_LEN, LeafNode<K, V, PREFIX_LEN>>,

/// Used for the removal
Expand All @@ -18,10 +15,7 @@ where
pub(crate) parent_ptr_and_child_key_byte: Option<(OpaqueNodePtr<K, V, PREFIX_LEN>, u8)>,
}

impl<'a, K, V, const PREFIX_LEN: usize> OccupiedEntryRef<'a, K, V, PREFIX_LEN>
where
K: AsBytes,
{
impl<'a, K, V, const PREFIX_LEN: usize> OccupiedEntryRef<'a, K, V, PREFIX_LEN> {
/// Gets a reference to the value in the entry.
pub fn get(&self) -> &V {
// SAFETY: This is safe because `Self` has an mutable reference
Expand Down
1 change: 1 addition & 0 deletions src/collections/map/iterators/fuzzy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ trait FuzzySearch<K: AsBytes, V, const PREFIX_LEN: usize> {
) -> bool
where
Self: InnerNode<PREFIX_LEN>,
Self::Key: AsBytes,
{
// We can use the fact that the first entry in the old_row holds,
// the length of how many bytes we used so far, so this becomes de depth
Expand Down
26 changes: 13 additions & 13 deletions src/collections/map/iterators/into_iter.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{AsBytes, TreeMap};
use crate::TreeMap;

/// An owning iterator over the entries of a `TreeMap`.
///
Expand All @@ -7,15 +7,15 @@ use crate::{AsBytes, TreeMap};
///
/// [`into_iter`]: IntoIterator::into_iter
/// [`IntoIterator`]: core::iter::IntoIterator
pub struct IntoIter<K: AsBytes, V, const PREFIX_LEN: usize>(TreeMap<K, V, PREFIX_LEN>);
pub struct IntoIter<K, V, const PREFIX_LEN: usize>(TreeMap<K, V, PREFIX_LEN>);

impl<K: AsBytes, V, const PREFIX_LEN: usize> IntoIter<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> IntoIter<K, V, PREFIX_LEN> {
pub(crate) fn new(tree: TreeMap<K, V, PREFIX_LEN>) -> Self {
IntoIter(tree)
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> Iterator for IntoIter<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> Iterator for IntoIter<K, V, PREFIX_LEN> {
type Item = (K, V);

fn next(&mut self) -> Option<Self::Item> {
Expand All @@ -29,7 +29,7 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> Iterator for IntoIter<K, V, PREFIX_
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> DoubleEndedIterator for IntoIter<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> DoubleEndedIterator for IntoIter<K, V, PREFIX_LEN> {
fn next_back(&mut self) -> Option<Self::Item> {
self.0.pop_last()
}
Expand All @@ -39,23 +39,23 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> DoubleEndedIterator for IntoIter<K,
///
/// This `struct` is created by the [`crate::TreeMap::into_keys`] method on
/// `TreeMap`. See its documentation for more.
pub struct IntoKeys<K: AsBytes, V, const PREFIX_LEN: usize>(IntoIter<K, V, PREFIX_LEN>);
pub struct IntoKeys<K, V, const PREFIX_LEN: usize>(IntoIter<K, V, PREFIX_LEN>);

impl<K: AsBytes, V, const PREFIX_LEN: usize> IntoKeys<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> IntoKeys<K, V, PREFIX_LEN> {
pub(crate) fn new(tree: TreeMap<K, V, PREFIX_LEN>) -> Self {
IntoKeys(IntoIter::new(tree))
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> Iterator for IntoKeys<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> Iterator for IntoKeys<K, V, PREFIX_LEN> {
type Item = K;

fn next(&mut self) -> Option<Self::Item> {
Some(self.0.next()?.0)
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> DoubleEndedIterator for IntoKeys<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> DoubleEndedIterator for IntoKeys<K, V, PREFIX_LEN> {
fn next_back(&mut self) -> Option<Self::Item> {
Some(self.0.next_back()?.0)
}
Expand All @@ -67,23 +67,23 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> DoubleEndedIterator for IntoKeys<K,
/// See its documentation for more.
///
/// [`into_values`]: crate::TreeMap::into_values
pub struct IntoValues<K: AsBytes, V, const PREFIX_LEN: usize>(IntoIter<K, V, PREFIX_LEN>);
pub struct IntoValues<K, V, const PREFIX_LEN: usize>(IntoIter<K, V, PREFIX_LEN>);

impl<K: AsBytes, V, const PREFIX_LEN: usize> IntoValues<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> IntoValues<K, V, PREFIX_LEN> {
pub(crate) fn new(tree: TreeMap<K, V, PREFIX_LEN>) -> Self {
IntoValues(IntoIter::new(tree))
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> Iterator for IntoValues<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> Iterator for IntoValues<K, V, PREFIX_LEN> {
type Item = V;

fn next(&mut self) -> Option<Self::Item> {
Some(self.0.next()?.1)
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> DoubleEndedIterator for IntoValues<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> DoubleEndedIterator for IntoValues<K, V, PREFIX_LEN> {
fn next_back(&mut self) -> Option<Self::Item> {
Some(self.0.next_back()?.1)
}
Expand Down
19 changes: 7 additions & 12 deletions src/collections/map/iterators/iterator.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
use crate::{AsBytes, ConcreteNodePtr, InnerNode, NodePtr, OpaqueNodePtr, TreeMap};
use crate::{ConcreteNodePtr, InnerNode, NodePtr, OpaqueNodePtr, TreeMap};
use std::{collections::VecDeque, iter::FusedIterator};

macro_rules! gen_iter {
($name:ident, $tree:ty, $ret:ty, $op:ident) => {
/// An iterator over all the `LeafNode`s
pub struct $name<'a, K: AsBytes, V, const PREFIX_LEN: usize> {
pub struct $name<'a, K, V, const PREFIX_LEN: usize> {
nodes: VecDeque<OpaqueNodePtr<K, V, PREFIX_LEN>>,
size: usize,
_tree: $tree,
}

impl<'a, K: AsBytes, V, const PREFIX_LEN: usize> $name<'a, K, V, PREFIX_LEN> {
impl<'a, K, V, const PREFIX_LEN: usize> $name<'a, K, V, PREFIX_LEN> {
/// Create a new iterator that will visit all leaf nodes descended from the
/// given node.
pub(crate) fn new(tree: $tree) -> Self {
Expand Down Expand Up @@ -51,7 +51,7 @@ macro_rules! gen_iter {
}
}

impl<'a, K: AsBytes, V, const PREFIX_LEN: usize> Iterator for $name<'a, K, V, PREFIX_LEN> {
impl<'a, K, V, const PREFIX_LEN: usize> Iterator for $name<'a, K, V, PREFIX_LEN> {
type Item = $ret;

fn next(&mut self) -> Option<Self::Item> {
Expand Down Expand Up @@ -83,7 +83,7 @@ macro_rules! gen_iter {
}
}

impl<'a, K: AsBytes, V, const PREFIX_LEN: usize> DoubleEndedIterator
impl<'a, K, V, const PREFIX_LEN: usize> DoubleEndedIterator
for $name<'a, K, V, PREFIX_LEN>
{
fn next_back(&mut self) -> Option<Self::Item> {
Expand All @@ -104,14 +104,9 @@ macro_rules! gen_iter {
}
}

impl<'a, K: AsBytes, V, const PREFIX_LEN: usize> FusedIterator
for $name<'a, K, V, PREFIX_LEN>
{
}
impl<'a, K, V, const PREFIX_LEN: usize> FusedIterator for $name<'a, K, V, PREFIX_LEN> {}

impl<'a, K: AsBytes, V, const PREFIX_LEN: usize> ExactSizeIterator
for $name<'a, K, V, PREFIX_LEN>
{
impl<'a, K, V, const PREFIX_LEN: usize> ExactSizeIterator for $name<'a, K, V, PREFIX_LEN> {
fn len(&self) -> usize {
self.size
}
Expand Down
2 changes: 1 addition & 1 deletion src/collections/map/iterators/prefix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ macro_rules! gen_add_children {
macro_rules! gen_iter {
($name:ident, $tree:ty, $ret:ty, $op:ident) => {
/// An iterator over all the `LeafNode`s with a specific prefix
pub struct $name<'a, 'b, K: AsBytes, V, const PREFIX_LEN: usize> {
pub struct $name<'a, 'b, K, V, const PREFIX_LEN: usize> {
nodes: VecDeque<(OpaqueNodePtr<K, V, PREFIX_LEN>, usize)>,
size: usize,
_tree: $tree,
Expand Down
6 changes: 3 additions & 3 deletions src/nodes/operations.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Trie node lookup and manipulation

use crate::{AsBytes, ConcreteNodePtr, InnerNode, NodePtr, OpaqueNodePtr};
use crate::{ConcreteNodePtr, InnerNode, NodePtr, OpaqueNodePtr};

mod insert;
pub(crate) use insert::*;
Expand All @@ -21,10 +21,10 @@ pub(crate) use delete::*;
/// # Safety
/// - This function must only be called once for this root node and all
/// descendants, otherwise a double-free could result.
pub unsafe fn deallocate_tree<K: AsBytes, V, const PREFIX_LEN: usize>(
pub unsafe fn deallocate_tree<K, V, const PREFIX_LEN: usize>(
root: OpaqueNodePtr<K, V, PREFIX_LEN>,
) {
fn deallocate_inner_node<K: AsBytes, V, N, const PREFIX_LEN: usize>(
fn deallocate_inner_node<K, V, N, const PREFIX_LEN: usize>(
stack: &mut Vec<OpaqueNodePtr<K, V, PREFIX_LEN>>,
inner_ptr: NodePtr<PREFIX_LEN, N>,
) where
Expand Down
14 changes: 7 additions & 7 deletions src/nodes/operations/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ unsafe fn remove_child_from_inner_node_and_compress<
/// have any other mutable references.
/// - `leaf_node_ptr` must be a unique pointer to the node and not have any
/// other mutable references.
unsafe fn inner_delete_non_root_unchecked<K: AsBytes, V, const PREFIX_LEN: usize>(
unsafe fn inner_delete_non_root_unchecked<K, V, const PREFIX_LEN: usize>(
leaf_node_ptr: NodePtr<PREFIX_LEN, LeafNode<K, V, PREFIX_LEN>>,
(parent_node_ptr, parent_key_byte): (OpaqueNodePtr<K, V, PREFIX_LEN>, u8),
grandparent_node_ptr: Option<(OpaqueNodePtr<K, V, PREFIX_LEN>, u8)>,
Expand Down Expand Up @@ -193,7 +193,7 @@ unsafe fn inner_delete_non_root_unchecked<K: AsBytes, V, const PREFIX_LEN: usize

/// The results of a successful delete operation
#[derive(Debug)]
pub struct DeleteResult<K: AsBytes, V, const PREFIX_LEN: usize> {
pub struct DeleteResult<K, V, const PREFIX_LEN: usize> {
/// The new root node for the tree, after the delete has been applied.
///
/// If `None`, that means the tree is now empty.
Expand All @@ -202,7 +202,7 @@ pub struct DeleteResult<K: AsBytes, V, const PREFIX_LEN: usize> {
pub deleted_leaf: LeafNode<K, V, PREFIX_LEN>,
}

pub struct DeletePoint<K: AsBytes, V, const PREFIX_LEN: usize> {
pub struct DeletePoint<K, V, const PREFIX_LEN: usize> {
/// The grandparent node of the leaf that will be deleted and the key byte
/// that was used to continue search.
///
Expand All @@ -219,7 +219,7 @@ pub struct DeletePoint<K: AsBytes, V, const PREFIX_LEN: usize> {
pub leaf_node_ptr: NodePtr<PREFIX_LEN, LeafNode<K, V, PREFIX_LEN>>,
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> std::fmt::Debug for DeletePoint<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> std::fmt::Debug for DeletePoint<K, V, PREFIX_LEN> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("DeleteSearchResult")
.field(
Expand All @@ -232,7 +232,7 @@ impl<K: AsBytes, V, const PREFIX_LEN: usize> std::fmt::Debug for DeletePoint<K,
}
}

impl<K: AsBytes, V, const PREFIX_LEN: usize> DeletePoint<K, V, PREFIX_LEN> {
impl<K, V, const PREFIX_LEN: usize> DeletePoint<K, V, PREFIX_LEN> {
/// Handle the logic of deleting a leaf node from the tree, after it has
/// been found.
///
Expand Down Expand Up @@ -368,7 +368,7 @@ where
/// on `root` or any child node of `root`. This function will arbitrarily
/// read to any child in the given tree.
#[inline(always)]
pub unsafe fn find_minimum_to_delete<K: AsBytes, V, const PREFIX_LEN: usize>(
pub unsafe fn find_minimum_to_delete<K, V, const PREFIX_LEN: usize>(
root: OpaqueNodePtr<K, V, PREFIX_LEN>,
) -> DeletePoint<K, V, PREFIX_LEN> {
let mut current_grandparent = None;
Expand Down Expand Up @@ -406,7 +406,7 @@ pub unsafe fn find_minimum_to_delete<K: AsBytes, V, const PREFIX_LEN: usize>(
/// on `root` or any child node of `root`. This function will arbitrarily
/// read to any child in the given tree.
#[inline(always)]
pub unsafe fn find_maximum_to_delete<K: AsBytes, V, const PREFIX_LEN: usize>(
pub unsafe fn find_maximum_to_delete<K, V, const PREFIX_LEN: usize>(
root: OpaqueNodePtr<K, V, PREFIX_LEN>,
) -> DeletePoint<K, V, PREFIX_LEN> {
let mut current_grandparent = None;
Expand Down
Loading

0 comments on commit a00708d

Please sign in to comment.