diff --git a/src/collections/map.rs b/src/collections/map.rs index 528d4736..2d727b96 100644 --- a/src/collections/map.rs +++ b/src/collections/map.rs @@ -138,7 +138,7 @@ impl TreeMap { // means there can only exist other immutable references aside from this one, // and no mutable references. That means that no mutating operations can occur // on the root node or any child of the root node. - let search_result = unsafe { search_unchecked(root, key)? }; + let search_result = unsafe { search_unchecked(root, key.as_bytes())? }; // SAFETY: The lifetime chosen the value reference is bounded by the lifetime of // the immutable reference to the `TreeMap`. The memory of the value will not be @@ -196,7 +196,7 @@ impl TreeMap { // means there cannot exist any other reference (mutable or immutable) to the // same `TreeMap`. Which means that no other mutating operations could be // happening during the `search_unchecked` call. - let search_result = unsafe { search_unchecked(root, key)? }; + let search_result = unsafe { search_unchecked(root, key.as_bytes())? }; // SAFETY: The lifetime chosen the value reference is bounded by the lifetime of // the mutable reference to the `TreeMap`. The value pointed to by the returned @@ -639,7 +639,7 @@ impl TreeMap { // that there are no other references (mutable or immutable) to this same // object. Meaning that our access to the root node is unique and there are no // other accesses to any node in the tree. - let insert_point = unsafe { search_for_insert_point(root, &key)? }; + let insert_point = unsafe { search_for_insert_point(root, key.as_bytes())? }; let insert_result = self.apply_insert_point(insert_point, key, value); Ok(insert_result.existing_leaf.map(|leaf| leaf.into_entry().1)) } else { @@ -673,7 +673,7 @@ impl TreeMap { // that there are no other references (mutable or immutable) to this same // object. Meaning that our access to the root node is unique and there are no // other accesses to any node in the tree. - let delete_point = unsafe { search_for_delete_point(root, key)? }; + let delete_point = unsafe { search_for_delete_point(root, key.as_bytes())? }; let delete_result = self.apply_delete_point(delete_point); Some(delete_result.deleted_leaf.into_entry()) } else { @@ -1273,7 +1273,7 @@ impl TreeMap { // that there are no other references (mutable or immutable) to this same // object. Meaning that our access to the root node is unique and there are no // other accesses to any node in the tree. - let insert_point = unsafe { search_for_insert_point(root, &key)? }; + let insert_point = unsafe { search_for_insert_point(root, key.as_bytes())? }; match insert_point.insert_type { Exact { leaf_node_ptr } => Entry::Occupied(OccupiedEntry { map: self, @@ -1314,7 +1314,7 @@ impl TreeMap { // that there are no other references (mutable or immutable) to this same // object. Meaning that our access to the root node is unique and there are no // other accesses to any node in the tree. - let insert_point = unsafe { search_for_insert_point(root, key)? }; + let insert_point = unsafe { search_for_insert_point(root, key.as_bytes())? }; match insert_point.insert_type { Exact { leaf_node_ptr } => EntryRef::Occupied(OccupiedEntryRef { map: self, diff --git a/src/nodes/operations/delete.rs b/src/nodes/operations/delete.rs index 088d82ee..f27664c0 100644 --- a/src/nodes/operations/delete.rs +++ b/src/nodes/operations/delete.rs @@ -1,5 +1,3 @@ -use std::borrow::Borrow; - use crate::{ nodes::operations::lookup, AsBytes, ConcreteNodePtr, InnerNode, LeafNode, NodePtr, OpaqueNodePtr, @@ -294,13 +292,12 @@ impl DeletePoint { /// - This function cannot be called concurrently with any mutating operation /// on `root` or any child node of `root`. This function will arbitrarily /// read to any child in the given tree. -pub unsafe fn search_for_delete_point( +pub unsafe fn search_for_delete_point( root: OpaqueNodePtr, - key: &Q, + key_bytes: &[u8], ) -> Option> where - K: Borrow + AsBytes, - Q: AsBytes + ?Sized, + K: AsBytes, { let mut current_grandparent = None; let mut current_parent = None; @@ -312,29 +309,29 @@ where ConcreteNodePtr::Node4(inner_ptr) => unsafe { // SAFETY: The safety requirement is covered by the safety requirement on the // containing function - lookup::check_prefix_lookup_child(inner_ptr, key, &mut current_depth) + lookup::check_prefix_lookup_child(inner_ptr, key_bytes, &mut current_depth) }, ConcreteNodePtr::Node16(inner_ptr) => unsafe { // SAFETY: The safety requirement is covered by the safety requirement on the // containing function - lookup::check_prefix_lookup_child(inner_ptr, key, &mut current_depth) + lookup::check_prefix_lookup_child(inner_ptr, key_bytes, &mut current_depth) }, ConcreteNodePtr::Node48(inner_ptr) => unsafe { // SAFETY: The safety requirement is covered by the safety requirement on the // containing function - lookup::check_prefix_lookup_child(inner_ptr, key, &mut current_depth) + lookup::check_prefix_lookup_child(inner_ptr, key_bytes, &mut current_depth) }, ConcreteNodePtr::Node256(inner_ptr) => unsafe { // SAFETY: The safety requirement is covered by the safety requirement on the // containing function - lookup::check_prefix_lookup_child(inner_ptr, key, &mut current_depth) + lookup::check_prefix_lookup_child(inner_ptr, key_bytes, &mut current_depth) }, ConcreteNodePtr::LeafNode(leaf_node_ptr) => { let leaf_node = leaf_node_ptr.read(); // Specifically we are matching the leaf node stored key against the full search // key to confirm that it is the right value. - if leaf_node.matches_full_key(key) { + if leaf_node.matches_full_key(key_bytes) { return Some(DeletePoint { grandparent_ptr_and_parent_key_byte: current_grandparent, parent_ptr_and_child_key_byte: current_parent, @@ -352,7 +349,7 @@ where ); // This should not panic because the current_depth will be greater than zero - let last_key_byte = key.as_bytes()[current_depth - 1]; + let last_key_byte = key_bytes[current_depth - 1]; current_grandparent = current_parent; current_parent = Some((current_node, last_key_byte)); diff --git a/src/nodes/operations/insert.rs b/src/nodes/operations/insert.rs index ae4222ec..199797ef 100644 --- a/src/nodes/operations/insert.rs +++ b/src/nodes/operations/insert.rs @@ -3,7 +3,7 @@ use crate::{ AsBytes, ConcreteNodePtr, InnerNode, InnerNode4, LeafNode, MatchPrefixResult, Mismatch, NodePtr, OpaqueNodePtr, }; -use std::{borrow::Borrow, error::Error, fmt, marker::PhantomData, ops::ControlFlow}; +use std::{error::Error, fmt, marker::PhantomData, ops::ControlFlow}; /// The results of a successful tree insert #[derive(Debug)] @@ -439,13 +439,12 @@ pub enum InsertSearchResultType { /// # Errors /// - If the given `key` is a prefix of an existing key, this function will /// return an error. -pub unsafe fn search_for_insert_point( +pub unsafe fn search_for_insert_point( root: OpaqueNodePtr, - key: &Q, + key_bytes: &[u8], ) -> Result, InsertPrefixError> where - K: AsBytes + Borrow, - Q: AsBytes + ?Sized, + K: AsBytes, { fn test_prefix_identify_insert( inner_ptr: NodePtr, @@ -500,7 +499,6 @@ where let mut current_parent = None; let mut current_node = root; let mut current_depth = 0; - let key_bytes = key.as_bytes(); loop { let lookup_result = match current_node.to_node_ptr() { @@ -519,7 +517,7 @@ where ConcreteNodePtr::LeafNode(leaf_node_ptr) => { let leaf_node = leaf_node_ptr.read(); - if leaf_node.matches_full_key(key) { + if leaf_node.matches_full_key(key_bytes) { return Ok(InsertPoint { key_bytes_used: current_depth, grandparent_ptr_and_parent_key_byte: current_grandparent, diff --git a/src/nodes/operations/insert/tests.rs b/src/nodes/operations/insert/tests.rs index 3378e35f..b8bfecb0 100644 --- a/src/nodes/operations/insert/tests.rs +++ b/src/nodes/operations/insert/tests.rs @@ -1,8 +1,8 @@ use crate::{ deallocate_tree, search_unchecked, tests_common::{generate_keys_skewed, insert_unchecked, setup_tree_from_entries}, - AsBytes, InnerNode, InnerNode4, InnerNodeCompressed, InsertPrefixError, LeafNode, NodePtr, - NodeType, OpaqueNodePtr, + InnerNode, InnerNode4, InnerNodeCompressed, InsertPrefixError, LeafNode, NodePtr, NodeType, + OpaqueNodePtr, }; #[test] @@ -271,7 +271,7 @@ fn insert_existing_key_overwrite() { let current_root: OpaqueNodePtr, char, 16> = setup_tree_from_entries(entries_it); - unsafe fn get_value( + unsafe fn get_value( n: NodePtr>, ) -> V { unsafe { *n.as_ref().value_ref() } diff --git a/src/nodes/operations/lookup.rs b/src/nodes/operations/lookup.rs index 8fa7ac00..0d1ff44f 100644 --- a/src/nodes/operations/lookup.rs +++ b/src/nodes/operations/lookup.rs @@ -1,5 +1,3 @@ -use std::borrow::Borrow; - use crate::{ AsBytes, ConcreteNodePtr, InnerNode, LeafNode, MatchPrefixResult, NodePtr, OpaqueNodePtr, }; @@ -10,13 +8,12 @@ use crate::{ /// - This function cannot be called concurrently with any mutating operation /// on `root` or any child node of `root`. This function will arbitrarily /// read to any child in the given tree. -pub unsafe fn search_unchecked( +pub unsafe fn search_unchecked( root: OpaqueNodePtr, - key: &Q, + key_bytes: &[u8], ) -> Option>> where - K: Borrow + AsBytes, - Q: AsBytes + ?Sized, + K: AsBytes, { let mut current_node = root; let mut current_depth = 0; @@ -26,29 +23,29 @@ where ConcreteNodePtr::Node4(inner_ptr) => unsafe { // SAFETY: The safety requirement is covered by the safety requirement on the // containing function - check_prefix_lookup_child(inner_ptr, key, &mut current_depth) + check_prefix_lookup_child(inner_ptr, key_bytes, &mut current_depth) }, ConcreteNodePtr::Node16(inner_ptr) => unsafe { // SAFETY: The safety requirement is covered by the safety requirement on the // containing function - check_prefix_lookup_child(inner_ptr, key, &mut current_depth) + check_prefix_lookup_child(inner_ptr, key_bytes, &mut current_depth) }, ConcreteNodePtr::Node48(inner_ptr) => unsafe { // SAFETY: The safety requirement is covered by the safety requirement on the // containing function - check_prefix_lookup_child(inner_ptr, key, &mut current_depth) + check_prefix_lookup_child(inner_ptr, key_bytes, &mut current_depth) }, ConcreteNodePtr::Node256(inner_ptr) => unsafe { // SAFETY: The safety requirement is covered by the safety requirement on the // containing function - check_prefix_lookup_child(inner_ptr, key, &mut current_depth) + check_prefix_lookup_child(inner_ptr, key_bytes, &mut current_depth) }, ConcreteNodePtr::LeafNode(leaf_node_ptr) => { let leaf_node = leaf_node_ptr.read(); // Specifically we are matching the leaf node stored key against the full search // key to confirm that it is the right value. - if leaf_node.matches_full_key(key) { + if leaf_node.matches_full_key(key_bytes) { return Some(leaf_node_ptr); } else { return None; @@ -67,30 +64,29 @@ where /// # Safety /// - No other access or mutation to the `inner_ptr` Node can happen while this /// function runs. -pub(crate) unsafe fn check_prefix_lookup_child( +pub(crate) unsafe fn check_prefix_lookup_child( inner_ptr: NodePtr, - key: &Q, + key_bytes: &[u8], current_depth: &mut usize, ) -> Option> where N: InnerNode, - K: Borrow + AsBytes, - Q: AsBytes + ?Sized, + K: AsBytes, { // SAFETY: The lifetime produced from this is bounded to this scope and does not // escape. Further, no other code mutates the node referenced, which is further // enforced the "no concurrent reads or writes" requirement on the // `search_unchecked` function. let inner_node = unsafe { inner_ptr.as_ref() }; - let match_prefix = inner_node.match_prefix(key.as_bytes(), *current_depth); + let match_prefix = inner_node.match_prefix(key_bytes, *current_depth); match match_prefix { MatchPrefixResult::Mismatch { .. } => None, MatchPrefixResult::Match { matched_bytes } => { // Since the prefix matched, advance the depth by the size of the prefix *current_depth += matched_bytes; - let next_key_fragment = if *current_depth < key.as_bytes().len() { - key.as_bytes()[*current_depth] + let next_key_fragment = if *current_depth < key_bytes.len() { + key_bytes[*current_depth] } else { // the key has insufficient bytes, it is a prefix of an existing key. Thus, the // key must not exist in the tree by the requirements of the insert function diff --git a/src/nodes/representation.rs b/src/nodes/representation.rs index dd52e2a4..7efdcbcb 100644 --- a/src/nodes/representation.rs +++ b/src/nodes/representation.rs @@ -2,7 +2,6 @@ use crate::{rust_nightly_apis::assume, tagged_pointer::TaggedPointer, AsBytes}; use std::{ - borrow::Borrow, fmt, hash::Hash, iter::FusedIterator, @@ -275,7 +274,7 @@ pub enum ConcreteNodePtr { LeafNode(NodePtr>), } -impl fmt::Debug for ConcreteNodePtr { +impl fmt::Debug for ConcreteNodePtr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::Node4(arg0) => f.debug_tuple("Node4").field(arg0).finish(), @@ -830,12 +829,11 @@ impl LeafNode { } /// Check that the provided full key is the same one as the stored key. - pub fn matches_full_key(&self, possible_key: &Q) -> bool + pub fn matches_full_key(&self, possible_key: &[u8]) -> bool where - K: Borrow + AsBytes, - Q: AsBytes + ?Sized, + K: AsBytes, { - self.key.borrow().as_bytes().eq(possible_key.as_bytes()) + self.key.as_bytes().eq(possible_key) } } diff --git a/src/nodes/visitor/pretty_printer.rs b/src/nodes/visitor/pretty_printer.rs index 1fe27ea7..342116f5 100644 --- a/src/nodes/visitor/pretty_printer.rs +++ b/src/nodes/visitor/pretty_printer.rs @@ -1,6 +1,6 @@ use crate::{ visitor::{Visitable, Visitor}, - AsBytes, InnerNode, NodeType, OpaqueNodePtr, TreeMap, + InnerNode, NodeType, OpaqueNodePtr, TreeMap, }; use std::{ fmt::{Debug, Display}, @@ -32,7 +32,7 @@ impl DotPrinter { settings: DotPrinterSettings, ) -> Option> where - K: Display + AsBytes, + K: Display, V: Display, { tree.root.map(|root| { @@ -53,7 +53,7 @@ impl DotPrinter { settings: DotPrinterSettings, ) -> io::Result<()> where - K: Display + AsBytes, + K: Display, V: Display, { let mut visitor = DotPrinter { @@ -87,7 +87,7 @@ impl DotPrinter { inner_node: &N, ) -> io::Result where - K: Display + AsBytes, + K: Display, T: Display, N: InnerNode, { @@ -144,7 +144,7 @@ impl DotPrinter { impl Visitor for DotPrinter where - K: Display + AsBytes, + K: Display, T: Display, O: Write, { @@ -204,7 +204,7 @@ where #[cfg(test)] mod tests { - use crate::deallocate_tree; + use crate::{deallocate_tree, AsBytes}; use super::*; diff --git a/src/nodes/visitor/tree_stats.rs b/src/nodes/visitor/tree_stats.rs index 3ece93ab..5ea78ec7 100644 --- a/src/nodes/visitor/tree_stats.rs +++ b/src/nodes/visitor/tree_stats.rs @@ -32,12 +32,12 @@ impl TreeStatsCollector { } /// Iterate through the given tree and return the number of leaf nodes. - pub fn count_leaf_nodes( + pub fn count_leaf_nodes( tree: &TreeMap, ) -> usize { struct LeafNodeCounter; - impl Visitor for LeafNodeCounter { + impl Visitor for LeafNodeCounter { type Output = usize; fn default_output(&self) -> Self::Output { @@ -91,7 +91,7 @@ pub struct InnerNodeStats { } impl InnerNodeStats { - fn aggregate_data(&mut self, t: &N) + fn aggregate_data(&mut self, t: &N) where N: InnerNode, { diff --git a/src/nodes/visitor/well_formed.rs b/src/nodes/visitor/well_formed.rs index 0a1983e3..37d2a58f 100644 --- a/src/nodes/visitor/well_formed.rs +++ b/src/nodes/visitor/well_formed.rs @@ -30,7 +30,8 @@ impl PartialEq<[u8; LEN]> for KeyPrefix { /// An issue with the well-formed-ness of the tree. See the documentation on /// [`WellFormedChecker`] for more context. -pub enum MalformedTreeError { +#[derive(PartialEq, Eq)] +pub enum MalformedTreeError { /// A loop was observed between nodes LoopFound { /// The node that was observed more than once while traversing the tree @@ -155,10 +156,7 @@ where } } -impl Clone for MalformedTreeError -where - K: Clone, -{ +impl Clone for MalformedTreeError { fn clone(&self) -> Self { match self { Self::LoopFound { @@ -191,61 +189,6 @@ where } } -impl PartialEq for MalformedTreeError -where - K: Eq, -{ - fn eq(&self, other: &Self) -> bool { - match (self, other) { - ( - Self::LoopFound { - node_ptr: l_node_ptr, - first_observed: l_first_observed, - later_observed: l_later_observed, - }, - Self::LoopFound { - node_ptr: r_node_ptr, - first_observed: r_first_observed, - later_observed: r_later_observed, - }, - ) => { - l_node_ptr == r_node_ptr - && l_first_observed == r_first_observed - && l_later_observed == r_later_observed - }, - ( - Self::WrongChildrenCount { - key_prefix: l_key_prefix, - inner_node_type: l_inner_node_type, - num_children: l_num_children, - }, - Self::WrongChildrenCount { - key_prefix: r_key_prefix, - inner_node_type: r_inner_node_type, - num_children: r_num_children, - }, - ) => { - l_key_prefix == r_key_prefix - && l_inner_node_type == r_inner_node_type - && l_num_children == r_num_children - }, - ( - Self::PrefixMismatch { - expected_prefix: l_expected_prefix, - entire_key: l_entire_key, - }, - Self::PrefixMismatch { - expected_prefix: r_expected_prefix, - entire_key: r_entire_key, - }, - ) => l_expected_prefix == r_expected_prefix && l_entire_key == r_entire_key, - _ => false, - } - } -} - -impl Eq for MalformedTreeError {} - impl Error for MalformedTreeError {} /// A visitor of the radix tree which checks that the tree is well-formed. @@ -265,7 +208,7 @@ impl Error for MalformedTreeError { +pub struct WellFormedChecker { current_key_prefix: Vec, seen_nodes: HashMap, KeyPrefix>, } diff --git a/src/tests_common.rs b/src/tests_common.rs index b3414ea9..c57f72aa 100644 --- a/src/tests_common.rs +++ b/src/tests_common.rs @@ -327,7 +327,7 @@ where { use crate::search_for_insert_point; - let insert_point = unsafe { search_for_insert_point(root, &key)? }; + let insert_point = unsafe { search_for_insert_point(root, key.as_bytes())? }; Ok(insert_point.apply(key, value)) }