From 6d9d519c719871a2ea91f7d4e595015b420c71c4 Mon Sep 17 00:00:00 2001 From: Alan Egerton Date: Sun, 1 Jan 2023 04:05:29 +0000 Subject: [PATCH] Tie lookup keys to comparators and tidy up --- Cargo.toml | 4 +- README.md | 86 +++++++----- src/btree/append.rs | 9 +- src/btree/dedup_sorted_iter.rs | 12 +- src/btree/map.rs | 136 +++++++++++------- src/btree/map/tests.rs | 241 +++++++++----------------------- src/btree/merge_iter.rs | 6 +- src/btree/navigate.rs | 30 ++-- src/btree/search.rs | 81 ++++++----- src/btree/set.rs | 213 ++++++++++++++-------------- src/btree/set/tests.rs | 156 +++++---------------- src/btree/split.rs | 14 +- src/btree/testing/crash_test.rs | 6 +- src/btree/testing/ord_chaos.rs | 11 +- src/lib.rs | 240 ++++++++++++++++--------------- src/polyfill.rs | 6 +- 16 files changed, 562 insertions(+), 689 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 510f638..4c23550 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "copse" -version = "0.1.1" +version = "0.2.0" license = "MIT OR Apache-2.0" repository = "https://github.com/eggyal/copse.git" description = "Direct ports of the standard library's B-Tree collections, but that sort according to a specified comparator rather than the `Ord` trait" @@ -25,7 +25,6 @@ unstable = [ "rustc_attrs", "slice_ptr_get", "specialization", - "type_alias_impl_trait", ] allocator_api = [] bound_map = [] @@ -42,7 +41,6 @@ new_uninit = [] rustc_attrs = [] slice_ptr_get = [] specialization = [] -type_alias_impl_trait = [] std = [] [dependencies] diff --git a/README.md b/README.md index 0764984..192aee9 100644 --- a/README.md +++ b/README.md @@ -13,45 +13,57 @@ possible because the [`Borrow`] trait stipulates that borrowed values must prese order. However, copse's collections do not use the [`Ord`] trait; instead, lookups can only ever -be performed using the [`Comparator`] supplied upon collection creation. This comparator -can only compare values of type `&T` for which it was defined, and hence such type must be -reachable from any key type `&Q` used to perform lookups in the collection. copse ensures -this via its [`Sortable`] trait, which will typically be implemented by the stored key type -`K`; its [`State`][Sortable::State] associated type then specifies the `T` in which -comparisons will be performed, and values of type `&Q` can be used as lookup keys provided -that `Q: Borrow`. +be performed using the [`Comparator`] supplied upon collection creation. This comparator +can only compare values of its [`Key`][Comparator::Key] associated type, and hence keys used +for lookups must implement [`LookupKey`] in order that the conversion can be performed. -For example, a collection using a `Comparator` comparator can store keys of type -`String` because `String` implements `Sortable`; moreover, lookups can be -performed using keys of type `&str` because `str` implements `Borrow` (due to the -reflexive blanket implementation). +# Example +```rust +// define a comparator +struct NthByteComparator { + n: usize, // runtime state +} -Implementations of [`Sortable`] are provided for primitive and some common standard library -types, but storing keys of other foreign types may require newtyping. +impl Comparator for NthByteComparator { + type Key = str; + fn cmp(&self, this: &str, that: &str) -> Ordering { + match (this.as_bytes().get(self.n), that.as_bytes().get(self.n)) { + (Some(lhs), Some(rhs)) => lhs.cmp(rhs), + (Some(_), None) => Ordering::Greater, + (None, Some(_)) => Ordering::Less, + (None, None) => Ordering::Equal, + } + } +} -# Function item types -In addition to the type parameters familiar from the standard library collections, copse's -collections are additionally parameterised by the type of the [`Comparator`]. If the -comparator type is not explicitly named, it defaults to the type of the [`Ord::cmp`] -function for `K::State`. As noted in the documentation of the [`CmpFn`] type alias, this -is only a zero-sized function item type if the unstable `type_alias_impl_trait` feature is -enabled; otherwise it is a function pointer type, with ensuing size and indirect call -implications. Collections built using the zero-sized function item type can still be -used in stable code, however; just not using the default type parameter. For example: +// define lookup key types for collections sorted by our comparator +impl LookupKey for String { + fn key(&self) -> &str { self.as_str() } +} +impl LookupKey for str { + fn key(&self) -> &str { self } +} -```rust -let mut ord_map = BTreeMap::new(Ord::cmp); +// create a collection using our comparator +let mut set = BTreeSet::new(NthByteComparator { n: 10 }); +assert!(set.insert("abcdefghij".to_string())); +assert!(!set.insert("xxxxxxxxxj".to_string())); +assert!(set.contains("jjjjjjjjjj")); ``` -However, naming this type carries the usual problems associated with anonymous types like -closures; in certain situations you may be able to use `impl Comparator` for the type -parameter, but in other situations (in stable code) the function pointer may be -unavoidable. +# Collection type parameters +In addition to the type parameters familiar from the standard library collections, copse's +collections are additionally parameterised by the type of the [`Comparator`]. If the +comparator type is not explicitly named, it defaults to the [`OrdComparator`] for the +storage key's [`DefaultComparisonKey`][OrdStoredKey::DefaultComparisonKey], which yields +behaviour analagous to the standard library collections (i.e. sorted by the `Ord` trait). +If you find yourself using these items, then you should probably ditch copse for the +standard library instead. # Crate feature flags This crate defines a number of feature flags, none of which are enabled by default: -* the `std` feature provides [`Sortable`] implementations for some libstd types +* the `std` feature provides [`OrdStoredKey`] implementations for some libstd types that are not available in libcore + liballoc, namely [`OsString`] and [`PathBuf`]; * the `unstable` feature enables all other crate features, each of which enables the @@ -61,11 +73,8 @@ This crate defines a number of feature flags, none of which are enabled by defau they are nevertheless included to ease tracking of the stdlib implementations. The most visible differences to library users will be: - * `allocator_api` enables the `new_in` methods for use of custom allocators; - * `specialization` adds the collection type name to some panic messages; - * `type_alias_impl_trait`, as mentioned above, ensures that the *default* - [`Comparator`] type parameter for the collections is the zero-sized function - item type of the `K::State::cmp` function. + * `allocator_api` enables the `new_in` methods for use of custom allocators; and + * `specialization` adds the collection type name to some panic messages. [std::collections::BTreeMap]: https://doc.rust-lang.org/std/collections/struct.BTreeMap.html [std::collections::BTreeSet]: https://doc.rust-lang.org/std/collections/struct.BTreeSet.html @@ -76,8 +85,9 @@ This crate defines a number of feature flags, none of which are enabled by defau [`OsString`]: https://doc.rust-lang.org/std/ffi/os_str/struct.OsString.html [`PathBuf`]: https://doc.rust-lang.org/std/path/struct.PathBuf.html -[`CmpFn`]: https://docs.rs/copse/latest/copse/type.CmpFn.html [`Comparator`]: https://docs.rs/copse/latest/copse/trait.Comparator.html -[`Comparator`]: https://docs.rs/copse/latest/copse/trait.Comparator.html -[`Sortable`]: https://docs.rs/copse/latest/copse/trait.Sortable.html -[Sortable::State]: https://docs.rs/copse/latest/copse/trait.Sortable.html#associatedtype.State \ No newline at end of file +[Comparator::Key]: https://docs.rs/copse/latest/copse/trait.Comparator.html#associatedtype.Key +[`LookupKey`]: https://docs.rs/copse/latest/copse/trait.LookupKey.html +[`OrdComparator`]: https://docs.rs/copse/latest/copse/struct.OrdComparator.html +[`OrdStoredKey`]: https://docs.rs/copse/latest/copse/trait.OrdStoredKey.html +[OrdStoredKey::DefaultComparisonKey]: https://docs.rs/copse/latest/copse/trait.OrdStoredKey.html#associatedtype.DefaultComparisonKey diff --git a/src/btree/append.rs b/src/btree/append.rs index fe7446a..1573bab 100644 --- a/src/btree/append.rs +++ b/src/btree/append.rs @@ -1,6 +1,7 @@ use super::merge_iter::MergeIterInner; use super::node::{self, Root}; -use crate::{polyfill::*, Comparator}; +use crate::polyfill::*; +use core::cmp::Ordering; use core::iter::FusedIterator; impl Root { @@ -20,7 +21,7 @@ impl Root { left: I, right: I, length: &mut usize, - comparator: impl Comparator<(K, V)>, + comparator: impl Fn(&(K, V), &(K, V)) -> Ordering, alloc: A, ) where I: Iterator + FusedIterator, @@ -95,14 +96,14 @@ struct MergeIter>(MergeIterInner, C); impl Iterator for MergeIter where - C: Comparator<(K, V)>, + C: Fn(&(K, V), &(K, V)) -> Ordering, I: Iterator + FusedIterator, { type Item = (K, V); /// If two keys are equal, returns the key-value pair from the right source. fn next(&mut self) -> Option<(K, V)> { - let (a_next, b_next) = self.0.nexts(&self.1); + let (a_next, b_next) = self.0.nexts(|this, that| self.1(this, that)); b_next.or(a_next) } } diff --git a/src/btree/dedup_sorted_iter.rs b/src/btree/dedup_sorted_iter.rs index c3f1e7f..481fb4a 100644 --- a/src/btree/dedup_sorted_iter.rs +++ b/src/btree/dedup_sorted_iter.rs @@ -1,6 +1,6 @@ use core::iter::Peekable; -use crate::{Comparator, Sortable}; +use crate::{Comparator, LookupKey}; /// A iterator for deduping the key of a sorted iterator. /// When encountering the duplicated key, only the last key-value pair is yielded. @@ -30,8 +30,8 @@ where impl Iterator for DedupSortedIter<'_, K, V, C, I> where - K: Sortable, - C: Comparator, + K: LookupKey, + C: Comparator, I: Iterator, { type Item = (K, V); @@ -48,11 +48,7 @@ where None => return Some(next), }; - if self - .comparator - .cmp(next.0.borrow(), peeked.0.borrow()) - .is_ne() - { + if self.comparator.cmp(next.0.key(), peeked.0.key()).is_ne() { return Some(next); } } diff --git a/src/btree/map.rs b/src/btree/map.rs index ada914e..d9e6cb1 100644 --- a/src/btree/map.rs +++ b/src/btree/map.rs @@ -1,6 +1,5 @@ use alloc::vec::Vec; use cfg_if::cfg_if; -use core::borrow::Borrow; use core::cmp::Ordering; use core::fmt::{self, Debug}; use core::hash::{Hash, Hasher}; @@ -10,7 +9,7 @@ use core::mem::{self, ManuallyDrop}; use core::ops::{Index, RangeBounds}; use core::ptr; -use crate::{polyfill::*, Comparator}; +use crate::{polyfill::*, Comparator, LookupKey, OrdComparator, OrdStoredKey}; use super::borrow::DormantMutRef; use super::dedup_sorted_iter::DedupSortedIter; @@ -18,7 +17,6 @@ use super::navigate::{LazyLeafRange, LeafRange}; use super::node::{self, marker, ForceResult::*, Handle, NodeRef, Root}; use super::search::SearchResult::*; use super::set_val::SetValZST; -use crate::{std_ord::Ord as _, MinCmpFn, Sortable}; mod entry; @@ -128,7 +126,7 @@ pub(super) const MIN_LEN: usize = node::MIN_LEN_AFTER_SPLIT; /// ``` /// use copse::BTreeMap; /// -/// let solar_distance = BTreeMap::from([ +/// let solar_distance = BTreeMap::<_, _>::from([ /// ("Mercury", 0.4), /// ("Venus", 0.7), /// ("Earth", 1.0), @@ -146,7 +144,7 @@ pub(super) const MIN_LEN: usize = node::MIN_LEN_AFTER_SPLIT; /// /// // type inference lets us omit an explicit type signature (which /// // would be `BTreeMap<&str, u8>` in this example). -/// let mut player_stats = BTreeMap::default(); +/// let mut player_stats = BTreeMap::<_, _>::default(); /// /// fn random_stat_buff() -> u8 { /// // could actually return some random value here - let's just return @@ -174,7 +172,7 @@ pub(super) const MIN_LEN: usize = node::MIN_LEN_AFTER_SPLIT; pub struct BTreeMap< K, V, - C = MinCmpFn, + C = OrdComparator<::DefaultComparisonKey>, // #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator + Clone = Global, > { @@ -314,9 +312,9 @@ impl Clone for BTreeMap super::Recover for BTreeMap where - K: Sortable, - Q: Borrow, - C: Comparator, + K: LookupKey, + Q: LookupKey, + C: Comparator, { type Key = K; @@ -615,12 +613,41 @@ impl BTreeMap { /// Basic usage: /// /// ``` - /// use copse::BTreeMap; - /// - /// let mut map = BTreeMap::default(); + /// # use core::cmp::Ordering; + /// # use copse::{Comparator, LookupKey, BTreeMap}; + /// # + /// // define a comparator + /// struct NthByteComparator { + /// n: usize, // runtime state + /// } + /// + /// impl Comparator for NthByteComparator { + /// // etc + /// # type Key = str; + /// # fn cmp(&self, this: &str, that: &str) -> Ordering { + /// # match (this.as_bytes().get(self.n), that.as_bytes().get(self.n)) { + /// # (Some(lhs), Some(rhs)) => lhs.cmp(rhs), + /// # (Some(_), None) => Ordering::Greater, + /// # (None, Some(_)) => Ordering::Less, + /// # (None, None) => Ordering::Equal, + /// # } + /// # } + /// } + /// + /// // define lookup key types for collections sorted by our comparator + /// impl LookupKey for String { + /// // etc + /// # fn key(&self) -> &str { self.as_str() } + /// } + /// # impl LookupKey for str { + /// # fn key(&self) -> &str { self } + /// # } + /// + /// // create a map using our comparator + /// let mut map = BTreeMap::new(NthByteComparator { n: 10 }); /// /// // entries can now be inserted into the empty map - /// map.insert(1, "a"); + /// assert!(map.insert("abcdefghij".to_string(), ()).is_none()); /// ``` // #[stable(feature = "rust1", since = "1.0.0")] // #[rustc_const_stable(feature = "const_btree_new", since = "1.66.0")] @@ -678,10 +705,10 @@ impl BTreeMap { /// ``` /// #![feature(allocator_api)] /// - /// use copse::BTreeMap; + /// use copse::{BTreeMap, OrdComparator}; /// use std::alloc::Global; /// - /// let mut map = BTreeMap::new_in(Ord::cmp, Global); + /// let mut map = BTreeMap::new_in(OrdComparator::default(), Global); /// /// // entries can now be inserted into the empty map /// map.insert(1, "a"); @@ -714,8 +741,8 @@ impl BTreeMap { impl BTreeMap where - K: Sortable, - C: Comparator, + K: LookupKey, + C: Comparator, { /// Returns a reference to the value corresponding to the key. /// @@ -737,7 +764,7 @@ where // #[stable(feature = "rust1", since = "1.0.0")] pub fn get(&self, key: &Q) -> Option<&V> where - Q: Borrow, + Q: LookupKey, { let root_node = self.root.as_ref()?.reborrow(); match root_node.search_tree(key, &self.comparator) { @@ -764,7 +791,7 @@ where // #[stable(feature = "map_get_key_value", since = "1.40.0")] pub fn get_key_value(&self, k: &Q) -> Option<(&K, &V)> where - Q: Borrow, + Q: LookupKey, { let root_node = self.root.as_ref()?.reborrow(); match root_node.search_tree(k, &self.comparator) { @@ -954,7 +981,7 @@ where // #[stable(feature = "rust1", since = "1.0.0")] pub fn contains_key(&self, key: &Q) -> bool where - Q: Borrow, + Q: LookupKey, { self.get(key).is_some() } @@ -982,7 +1009,7 @@ where // #[stable(feature = "rust1", since = "1.0.0")] pub fn get_mut(&mut self, key: &Q) -> Option<&mut V> where - Q: Borrow, + Q: LookupKey, { let root_node = self.root.as_mut()?.borrow_mut(); match root_node.search_tree(key, &self.comparator) { @@ -1082,7 +1109,7 @@ where // #[stable(feature = "rust1", since = "1.0.0")] pub fn remove(&mut self, key: &Q) -> Option where - Q: Borrow, + Q: LookupKey, { self.remove_entry(key).map(|(_, v)| v) } @@ -1108,7 +1135,7 @@ where // #[stable(feature = "btreemap_remove_entry", since = "1.45.0")] pub fn remove_entry(&mut self, key: &Q) -> Option<(K, V)> where - Q: Borrow, + Q: LookupKey, { let (map, dormant_map) = DormantMutRef::new(self); let root_node = map.root.as_mut()?.borrow_mut(); @@ -1215,7 +1242,7 @@ where self_iter, other_iter, &mut self.length, - |a: &(K, V), b: &(K, V)| self.comparator.cmp(a.0.borrow(), b.0.borrow()), + |a: &(K, V), b: &(K, V)| self.comparator.cmp(a.0.key(), b.0.key()), (*self.alloc).clone(), ) } @@ -1252,7 +1279,7 @@ where // #[stable(feature = "btree_range", since = "1.17.0")] pub fn range(&self, range: R) -> Range<'_, K, V> where - T: Borrow, + T: LookupKey, R: RangeBounds, { if let Some(root) = &self.root { @@ -1297,7 +1324,7 @@ where // #[stable(feature = "btree_range", since = "1.17.0")] pub fn range_mut(&mut self, range: R) -> RangeMut<'_, K, V> where - T: Borrow, + T: LookupKey, R: RangeBounds, { if let Some(root) = &mut self.root { @@ -1394,7 +1421,7 @@ where // #[stable(feature = "btree_split_off", since = "1.11.0")] pub fn split_off(&mut self, key: &Q) -> Self where - Q: Borrow, + Q: LookupKey, A: Clone, C: Clone, { @@ -2214,29 +2241,30 @@ impl<'a, K, V> DoubleEndedIterator for RangeMut<'a, K, V> { impl FusedIterator for RangeMut<'_, K, V> {} // #[stable(feature = "rust1", since = "1.0.0")] -impl FromIterator<(K, V)> for BTreeMap> +impl FromIterator<(K, V)> for BTreeMap where - K: Sortable, - K::State: Ord, + K: LookupKey, + C: Comparator + Default, { - fn from_iter>(iter: T) -> BTreeMap> { + fn from_iter>(iter: T) -> BTreeMap { let mut inputs: Vec<_> = iter.into_iter().collect(); if inputs.is_empty() { - return BTreeMap::new(K::State::cmp_fn()); + return BTreeMap::new(C::default()); } // use stable sort to preserve the insertion order. - inputs.sort_by(|a, b| Ord::cmp(a.0.borrow(), b.0.borrow())); - BTreeMap::bulk_build_from_sorted_iter(inputs, K::State::cmp_fn(), Global) + let comparator = C::default(); + inputs.sort_by(|a, b| comparator.cmp(a.0.key(), b.0.key())); + BTreeMap::bulk_build_from_sorted_iter(inputs, comparator, Global) } } // #[stable(feature = "rust1", since = "1.0.0")] impl Extend<(K, V)> for BTreeMap where - K: Sortable, - C: Comparator, + K: LookupKey, + C: Comparator, { #[inline] fn extend>(&mut self, iter: T) { @@ -2255,8 +2283,8 @@ where // #[stable(feature = "extend_ref", since = "1.2.0")] impl<'a, K: Copy, V: Copy, C, A: Allocator + Clone> Extend<(&'a K, &'a V)> for BTreeMap where - K: Sortable, - C: Comparator, + K: LookupKey, + C: Comparator, { fn extend>(&mut self, iter: I) { self.extend(iter.into_iter().map(|(&key, &value)| (key, value))); @@ -2282,14 +2310,13 @@ impl Hash for BTreeMap { } // #[stable(feature = "rust1", since = "1.0.0")] -impl Default for BTreeMap> +impl Default for BTreeMap where - K: Sortable, - K::State: Ord, + C: Comparator + Default, { - /// Creates an empty `BTreeMap`, ordered by the `Ord` trait. - fn default() -> BTreeMap> { - BTreeMap::new(K::State::cmp_fn()) + /// Creates an empty `BTreeMap`, ordered by a default `C` comparator. + fn default() -> BTreeMap { + BTreeMap::new(C::default()) } } @@ -2329,9 +2356,9 @@ impl Debug for BTreeMap // #[stable(feature = "rust1", since = "1.0.0")] impl Index<&Q> for BTreeMap where - K: Sortable, - Q: Borrow, - C: Comparator, + K: LookupKey, + Q: LookupKey, + C: Comparator, { type Output = V; @@ -2347,10 +2374,10 @@ where } // #[stable(feature = "std_collections_from_array", since = "1.56.0")] -impl From<[(K, V); N]> for BTreeMap> +impl From<[(K, V); N]> for BTreeMap where - K: Sortable, - K::State: Ord, + K: LookupKey, + C: Comparator + Default, { /// Converts a `[(K, V); N]` into a `BTreeMap<(K, V)>`. /// @@ -2363,12 +2390,13 @@ where /// ``` fn from(mut arr: [(K, V); N]) -> Self { if N == 0 { - return BTreeMap::new(K::State::cmp_fn()); + return BTreeMap::new(C::default()); } // use stable sort to preserve the insertion order. - arr.sort_by(|a, b| Ord::cmp(a.0.borrow(), b.0.borrow())); - BTreeMap::bulk_build_from_sorted_iter(arr, K::State::cmp_fn(), Global) + let comparator = C::default(); + arr.sort_by(|a, b| comparator.cmp(a.0.key(), b.0.key())); + BTreeMap::bulk_build_from_sorted_iter(arr, comparator, Global) } } @@ -2420,7 +2448,7 @@ impl BTreeMap { /// ``` /// use copse::BTreeMap; /// - /// let mut map = BTreeMap::from([ + /// let mut map = BTreeMap::<_, _>::from([ /// ("a", 1), /// ("b", 2), /// ("c", 3), diff --git a/src/btree/map/tests.rs b/src/btree/map/tests.rs index 124fdce..dc3c613 100644 --- a/src/btree/map/tests.rs +++ b/src/btree/map/tests.rs @@ -41,10 +41,7 @@ fn test_all_refs<'a, T: 'a>(dummy: &mut T, iter: impl Iterator } } -impl BTreeMap -where - K::State: Ord, -{ +impl BTreeMap { // Panics if the map (or the code navigating it) is corrupted. fn check_invariants(&self) { if let Some(root) = &self.root { @@ -74,7 +71,7 @@ where // guarantee that all keys are unique, just that adjacent keys are unique. fn check(&self) where - K::State: Debug, + K::DefaultComparisonKey: Debug, { self.check_invariants(); self.assert_strictly_ascending(); @@ -99,16 +96,16 @@ where // Panics if the keys are not in strictly ascending order. fn assert_strictly_ascending(&self) where - K::State: Debug, + K::DefaultComparisonKey: Debug, { let mut keys = self.keys(); if let Some(mut previous) = keys.next() { for next in keys { assert!( - previous.borrow() < next.borrow(), + self.comparator.cmp(previous.key(), next.key()).is_lt(), "{:?} >= {:?}", - previous.borrow(), - next.borrow() + previous.key(), + next.key() ); previous = next; } @@ -118,10 +115,7 @@ where // Transform the tree to minimize wasted space, obtaining fewer nodes that // are mostly filled up to their capacity. The same compact tree could have // been obtained by inserting keys in a shrewd order. - fn compact(&mut self) - where - K: Ord, - { + fn compact(&mut self) { let iter = mem::take(self).into_iter(); cfg_if! { @@ -412,9 +406,9 @@ fn test_iter_rev() { // Specifically tests iter_mut's ability to mutate the value of pairs in-line. fn do_test_iter_mut_mutation(size: usize) where - T: Copy + Sortable + TryFrom, + T: Copy + OrdStoredKey + TryFrom, >::Error: Debug, - T::State: Debug + Ord, + T::DefaultComparisonKey: Debug, { let zero = T::try_from(0).unwrap(); let mut map = BTreeMap::from_iter((0..size).map(|i| (T::try_from(i).unwrap(), zero))); @@ -425,22 +419,22 @@ where // Iterate forwards, trying to mutate to unique values for (i, (k, v)) in map.iter_mut().enumerate() { - assert_eq!(k.borrow(), T::try_from(i).unwrap().borrow()); - assert_eq!((*v).borrow(), zero.borrow()); + assert_eq!(k.key(), T::try_from(i).unwrap().key()); + assert_eq!((*v).key(), zero.key()); *v = T::try_from(i + 1).unwrap(); } // Iterate backwards, checking that mutations succeeded and trying to mutate again for (i, (k, v)) in map.iter_mut().rev().enumerate() { - assert_eq!(k.borrow(), T::try_from(size - i - 1).unwrap().borrow()); - assert_eq!((*v).borrow(), T::try_from(size - i).unwrap().borrow()); + assert_eq!(k.key(), T::try_from(size - i - 1).unwrap().key()); + assert_eq!((*v).key(), T::try_from(size - i).unwrap().key()); *v = T::try_from(2 * size - i).unwrap(); } // Check that backward mutations succeeded for (i, (k, v)) in map.iter_mut().enumerate() { - assert_eq!(k.borrow(), T::try_from(i).unwrap().borrow()); - assert_eq!((*v).borrow(), T::try_from(size + i + 1).unwrap().borrow()); + assert_eq!(k.key(), T::try_from(i).unwrap().key()); + assert_eq!((*v).key(), T::try_from(size + i + 1).unwrap().key()); } map.check(); } @@ -449,8 +443,8 @@ where #[repr(align(32))] struct Align32(usize); -impl Sortable for Align32 { - type State = Self; +impl OrdStoredKey for Align32 { + type DefaultComparisonKey = Self; } impl TryFrom for Align32 { @@ -603,10 +597,7 @@ fn test_iter_min_max() { a.check(); } -fn range_keys( - map: &BTreeMap Ordering>, - range: impl RangeBounds, -) -> Vec { +fn range_keys(map: &BTreeMap, range: impl RangeBounds) -> Vec { Vec::from_iter(map.range(range).map(|(&k, &v)| { assert_eq!(k, v); k @@ -911,16 +902,12 @@ fn test_range_finding_ill_order_in_range_ord() { #[derive(PartialEq, Eq, PartialOrd, Ord)] struct CompositeKey(i32, EvilTwin); - impl Borrow for CompositeKey { - fn borrow(&self) -> &EvilTwin { + impl LookupKey> for CompositeKey { + fn key(&self) -> &EvilTwin { &self.1 } } - impl Sortable for CompositeKey { - type State = EvilTwin; - } - let map = BTreeMap::from_iter((0..12).map(|i| (CompositeKey(i, EvilTwin(i)), ()))); let _ = map.range(EvilTwin(5)..=EvilTwin(7)); } @@ -1398,47 +1385,47 @@ fn test_borrow() { } #[allow(dead_code)] - fn get(v: &BTreeMap, ()>, t: &T) { + fn get(v: &BTreeMap, (), OrdComparator>, t: &T) { let _ = v.get(t); } #[allow(dead_code)] - fn get_mut(v: &mut BTreeMap, ()>, t: &T) { + fn get_mut(v: &mut BTreeMap, (), OrdComparator>, t: &T) { let _ = v.get_mut(t); } #[allow(dead_code)] - fn get_key_value(v: &BTreeMap, ()>, t: &T) { + fn get_key_value(v: &BTreeMap, (), OrdComparator>, t: &T) { let _ = v.get_key_value(t); } #[allow(dead_code)] - fn contains_key(v: &BTreeMap, ()>, t: &T) { + fn contains_key(v: &BTreeMap, (), OrdComparator>, t: &T) { let _ = v.contains_key(t); } #[allow(dead_code)] - fn range(v: &BTreeMap, ()>, t: T) { + fn range(v: &BTreeMap, (), OrdComparator>, t: T) { let _ = v.range(t..); } #[allow(dead_code)] - fn range_mut(v: &mut BTreeMap, ()>, t: T) { + fn range_mut(v: &mut BTreeMap, (), OrdComparator>, t: T) { let _ = v.range_mut(t..); } #[allow(dead_code)] - fn remove(v: &mut BTreeMap, ()>, t: &T) { + fn remove(v: &mut BTreeMap, (), OrdComparator>, t: &T) { v.remove(t); } #[allow(dead_code)] - fn remove_entry(v: &mut BTreeMap, ()>, t: &T) { + fn remove_entry(v: &mut BTreeMap, (), OrdComparator>, t: &T) { v.remove_entry(t); } #[allow(dead_code)] - fn split_off(v: &mut BTreeMap, ()>, t: &T) { + fn split_off(v: &mut BTreeMap, (), OrdComparator>, t: &T) { v.split_off(t); } } @@ -1544,8 +1531,8 @@ fn test_bad_zst() { #[derive(Clone, Copy, Debug)] struct Bad; - impl Sortable for Bad { - type State = Self; + impl OrdStoredKey for Bad { + type DefaultComparisonKey = Self; } impl PartialEq for Bad { @@ -1733,7 +1720,7 @@ fn test_clone_from() { #[allow(dead_code)] fn assert_covariance() { - fn map_key<'new>(v: BTreeMap<&'static str, ()>) -> BTreeMap<&'new str, ()> { + fn map_key<'new>(v: BTreeMap<&'static str, (), ()>) -> BTreeMap<&'new str, (), ()> { v } fn map_val<'new>(v: BTreeMap<(), &'static str>) -> BTreeMap<(), &'new str> { @@ -1792,111 +1779,66 @@ fn assert_covariance() { #[allow(dead_code)] fn assert_sync() { - fn map(v: &BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn map(v: &BTreeMap) -> impl Sync + '_ { v } - fn into_iter(v: BTreeMap) -> impl Sync - where - T::State: Ord, - { + fn into_iter(v: BTreeMap) -> impl Sync { v.into_iter() } - fn into_keys(v: BTreeMap) -> impl Sync - where - T::State: Ord, - { + fn into_keys(v: BTreeMap) -> impl Sync { v.into_keys() } - fn into_values(v: BTreeMap) -> impl Sync - where - T::State: Ord, - { + fn into_values(v: BTreeMap) -> impl Sync { v.into_values() } - fn drain_filter(v: &mut BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn drain_filter(v: &mut BTreeMap) -> impl Sync + '_ { v.drain_filter(|_, _| false) } - fn iter(v: &BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn iter(v: &BTreeMap) -> impl Sync + '_ { v.iter() } - fn iter_mut(v: &mut BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn iter_mut(v: &mut BTreeMap) -> impl Sync + '_ { v.iter_mut() } - fn keys(v: &BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn keys(v: &BTreeMap) -> impl Sync + '_ { v.keys() } - fn values(v: &BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn values(v: &BTreeMap) -> impl Sync + '_ { v.values() } - fn values_mut(v: &mut BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn values_mut(v: &mut BTreeMap) -> impl Sync + '_ { v.values_mut() } - fn range(v: &BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn range(v: &BTreeMap) -> impl Sync + '_ { v.range::(..) } - fn range_mut(v: &mut BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn range_mut(v: &mut BTreeMap) -> impl Sync + '_ { v.range_mut::(..) } - fn entry(v: &mut BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn entry(v: &mut BTreeMap) -> impl Sync + '_ { v.entry(Default::default()) } - fn occupied_entry(v: &mut BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn occupied_entry(v: &mut BTreeMap) -> impl Sync + '_ { match v.entry(Default::default()) { Occupied(entry) => entry, _ => unreachable!(), } } - fn vacant_entry(v: &mut BTreeMap) -> impl Sync + '_ - where - T::State: Ord, - { + fn vacant_entry(v: &mut BTreeMap) -> impl Sync + '_ { match v.entry(Default::default()) { Vacant(entry) => entry, _ => unreachable!(), @@ -1906,111 +1848,66 @@ fn assert_sync() { #[allow(dead_code)] fn assert_send() { - fn map(v: BTreeMap) -> impl Send - where - T::State: Ord, - { + fn map(v: BTreeMap) -> impl Send { v } - fn into_iter(v: BTreeMap) -> impl Send - where - T::State: Ord, - { + fn into_iter(v: BTreeMap) -> impl Send { v.into_iter() } - fn into_keys(v: BTreeMap) -> impl Send - where - T::State: Ord, - { + fn into_keys(v: BTreeMap) -> impl Send { v.into_keys() } - fn into_values(v: BTreeMap) -> impl Send - where - T::State: Ord, - { + fn into_values(v: BTreeMap) -> impl Send { v.into_values() } - fn drain_filter(v: &mut BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn drain_filter(v: &mut BTreeMap) -> impl Send + '_ { v.drain_filter(|_, _| false) } - fn iter(v: &BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn iter(v: &BTreeMap) -> impl Send + '_ { v.iter() } - fn iter_mut(v: &mut BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn iter_mut(v: &mut BTreeMap) -> impl Send + '_ { v.iter_mut() } - fn keys(v: &BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn keys(v: &BTreeMap) -> impl Send + '_ { v.keys() } - fn values(v: &BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn values(v: &BTreeMap) -> impl Send + '_ { v.values() } - fn values_mut(v: &mut BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn values_mut(v: &mut BTreeMap) -> impl Send + '_ { v.values_mut() } - fn range(v: &BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn range(v: &BTreeMap) -> impl Send + '_ { v.range::(..) } - fn range_mut(v: &mut BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn range_mut(v: &mut BTreeMap) -> impl Send + '_ { v.range_mut::(..) } - fn entry(v: &mut BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn entry(v: &mut BTreeMap) -> impl Send + '_ { v.entry(Default::default()) } - fn occupied_entry(v: &mut BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn occupied_entry(v: &mut BTreeMap) -> impl Send + '_ { match v.entry(Default::default()) { Occupied(entry) => entry, _ => unreachable!(), } } - fn vacant_entry(v: &mut BTreeMap) -> impl Send + '_ - where - T::State: Ord, - { + fn vacant_entry(v: &mut BTreeMap) -> impl Send + '_ { match v.entry(Default::default()) { Vacant(entry) => entry, _ => unreachable!(), @@ -2020,7 +1917,7 @@ fn assert_send() { #[test] fn test_ord_absence() { - fn map(mut map: BTreeMap) { + fn map(mut map: BTreeMap) { let _ = map.is_empty(); let _ = map.len(); map.clear(); @@ -2039,7 +1936,7 @@ fn test_ord_absence() { } } - fn map_debug(mut map: BTreeMap) { + fn map_debug(mut map: BTreeMap) { format!("{map:?}"); format!("{:?}", map.iter()); format!("{:?}", map.iter_mut()); @@ -2056,7 +1953,7 @@ fn test_ord_absence() { } } - fn map_clone(mut map: BTreeMap) { + fn map_clone(mut map: BTreeMap) { map.clone_from(&map.clone()); } @@ -2075,14 +1972,14 @@ fn test_occupied_entry_key() { assert_eq!(a.height(), None); a.insert(key, value); assert_eq!(a.len(), 1); - assert_eq!(a[key], value); + assert_eq!(a[&key], value); match a.entry(key) { Vacant(_) => panic!(), Occupied(e) => assert_eq!(key, *e.key()), } assert_eq!(a.len(), 1); - assert_eq!(a[key], value); + assert_eq!(a[&key], value); a.check(); } @@ -2101,7 +1998,7 @@ fn test_vacant_entry_key() { } } assert_eq!(a.len(), 1); - assert_eq!(a[key], value); + assert_eq!(a[&key], value); a.check(); } @@ -2506,7 +2403,7 @@ fn test_into_iter_drop_leak_height_0() { let c = CrashTestDummy::new(2); let d = CrashTestDummy::new(3); let e = CrashTestDummy::new(4); - let mut map = BTreeMap::default(); + let mut map = BTreeMap::<_, _, OrdComparator>::default(); map.insert("a", a.spawn(Panic::Never)); map.insert("b", b.spawn(Panic::Never)); map.insert("c", c.spawn(Panic::Never)); diff --git a/src/btree/merge_iter.rs b/src/btree/merge_iter.rs index 37c37cf..bc6f190 100644 --- a/src/btree/merge_iter.rs +++ b/src/btree/merge_iter.rs @@ -2,8 +2,6 @@ use core::cmp::Ordering; use core::fmt::{self, Debug}; use core::iter::FusedIterator; -use crate::Comparator; - /// Core of an iterator that merges the output of two strictly ascending iterators, /// for instance a union or a symmetric difference. pub struct MergeIterInner { @@ -63,7 +61,7 @@ impl MergeIterInner { /// return the same empty pair. pub fn nexts( &mut self, - comparator: &impl Comparator, + comparator: impl Fn(&I::Item, &I::Item) -> Ordering, ) -> (Option, Option) where I: FusedIterator, @@ -85,7 +83,7 @@ impl MergeIterInner { } } if let (Some(a1), Some(b1)) = (&a_next, &b_next) { - match comparator.cmp(a1, b1) { + match comparator(a1, b1) { Ordering::Less => self.peeked = b_next.take().map(Peeked::B), Ordering::Greater => self.peeked = a_next.take().map(Peeked::A), Ordering::Equal => (), diff --git a/src/btree/navigate.rs b/src/btree/navigate.rs index 4e3b024..38ed704 100644 --- a/src/btree/navigate.rs +++ b/src/btree/navigate.rs @@ -1,9 +1,8 @@ -use core::borrow::Borrow; use core::hint; use core::ops::RangeBounds; use core::ptr; -use crate::{Comparator, Sortable}; +use crate::{Comparator, LookupKey}; use super::node::{marker, ForceResult::*, Handle, NodeRef}; @@ -266,15 +265,16 @@ impl NodeRef( + unsafe fn find_leaf_edges_spanning_range( self, - comparator: &impl Comparator, + comparator: &C, range: R, ) -> LeafRange where - K: Sortable, - Q: ?Sized + Borrow, + K: LookupKey, + Q: ?Sized + LookupKey, R: RangeBounds, + C: Comparator, { match self.search_tree_for_bifurcation(comparator, &range) { Err(_) => LeafRange::none(), @@ -326,15 +326,16 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> /// /// The result is meaningful only if the tree is ordered by key, like the tree /// in a `BTreeMap` is. - pub fn range_search( + pub fn range_search( self, - comparator: &impl Comparator, + comparator: &C, range: R, ) -> LeafRange, K, V> where - K: Sortable, - Q: ?Sized + Borrow, + K: LookupKey, + Q: ?Sized + LookupKey, R: RangeBounds, + C: Comparator, { // SAFETY: our borrow type is immutable. unsafe { self.find_leaf_edges_spanning_range(comparator, range) } @@ -356,15 +357,16 @@ impl<'a, K: 'a, V: 'a> NodeRef, K, V, marker::LeafOrInternal> /// /// # Safety /// Do not use the duplicate handles to visit the same KV twice. - pub fn range_search( + pub fn range_search( self, - comparator: &impl Comparator, + comparator: &C, range: R, ) -> LeafRange, K, V> where - K: Sortable, - Q: ?Sized + Borrow, + K: LookupKey, + Q: ?Sized + LookupKey, R: RangeBounds, + C: Comparator, { unsafe { self.find_leaf_edges_spanning_range(comparator, range) } } diff --git a/src/btree/search.rs b/src/btree/search.rs index 07a9354..07c1da2 100644 --- a/src/btree/search.rs +++ b/src/btree/search.rs @@ -1,8 +1,7 @@ -use core::borrow::Borrow; use core::cmp::Ordering; use core::ops::{Bound, RangeBounds}; -use crate::{Comparator, Sortable}; +use crate::{Comparator, LookupKey}; use super::node::{marker, ForceResult::*, Handle, NodeRef}; @@ -48,14 +47,15 @@ impl NodeRef( + pub fn search_tree( mut self, key: &Q, - comparator: &impl Comparator, + comparator: &C, ) -> SearchResult where - K: Sortable, - Q: Borrow, + K: LookupKey, + Q: LookupKey, + C: Comparator, { loop { self = match self.search_node(key, comparator) { @@ -83,9 +83,9 @@ impl NodeRef( + pub fn search_tree_for_bifurcation<'r, Q: ?Sized, R, C>( mut self, - comparator: &impl Comparator, + comparator: &C, range: &'r R, ) -> Result< ( @@ -98,9 +98,10 @@ impl NodeRef, marker::Edge>, > where - K: Sortable, - Q: Borrow, + K: LookupKey, + Q: LookupKey, R: RangeBounds, + C: Comparator, { // Determine if map or set is being searched #[cfg(feature = "specialization")] @@ -111,7 +112,7 @@ impl NodeRef + if comparator.cmp(s.key(), e.key()).is_eq() => { cfg_if! { if #[cfg(feature = "specialization")] { @@ -126,7 +127,7 @@ impl NodeRef + if comparator.cmp(s.key(), e.key()).is_gt() => { cfg_if! { if #[cfg(feature = "specialization")] { @@ -176,14 +177,15 @@ impl NodeRef( + pub fn find_lower_bound_edge<'r, Q, C>( self, - comparator: &impl Comparator, + comparator: &C, bound: SearchBound<&'r Q>, ) -> (Handle, SearchBound<&'r Q>) where - K: Sortable, - Q: ?Sized + Borrow, + K: LookupKey, + Q: ?Sized + LookupKey, + C: Comparator, { let (edge_idx, bound) = self.find_lower_bound_index(comparator, bound); let edge = unsafe { Handle::new_edge(self, edge_idx) }; @@ -191,14 +193,15 @@ impl NodeRef( + pub fn find_upper_bound_edge<'r, Q, C>( self, - comparator: &impl Comparator, + comparator: &C, bound: SearchBound<&'r Q>, ) -> (Handle, SearchBound<&'r Q>) where - K: Sortable, - Q: ?Sized + Borrow, + K: LookupKey, + Q: ?Sized + LookupKey, + C: Comparator, { let (edge_idx, bound) = unsafe { self.find_upper_bound_index(comparator, bound, 0) }; let edge = unsafe { Handle::new_edge(self, edge_idx) }; @@ -214,14 +217,15 @@ impl NodeRef { /// /// The result is meaningful only if the tree is ordered by key, like the tree /// in a `BTreeMap` is. - pub fn search_node( + pub fn search_node( self, key: &Q, - comparator: &impl Comparator, + comparator: &C, ) -> SearchResult where - K: Sortable, - Q: Borrow, + K: LookupKey, + Q: LookupKey, + C: Comparator, { match unsafe { self.find_key_index(key, comparator, 0) } { IndexResult::KV(idx) => Found(unsafe { Handle::new_kv(self, idx) }), @@ -237,15 +241,16 @@ impl NodeRef { /// /// # Safety /// `start_index` must be a valid edge index for the node. - unsafe fn find_key_index( + unsafe fn find_key_index( &self, key: &Q, - comparator: &impl Comparator, + comparator: &C, start_index: usize, ) -> IndexResult where - K: Sortable, - Q: Borrow, + K: LookupKey, + Q: LookupKey, + C: Comparator, { let node = self.reborrow(); let keys = node.keys(); @@ -254,7 +259,7 @@ impl NodeRef { .iter() .enumerate() { - match comparator.cmp(key.borrow(), k.borrow()) { + match comparator.cmp(key.key(), k.key()) { Ordering::Greater => {} Ordering::Equal => return IndexResult::KV(start_index + offset), Ordering::Less => return IndexResult::Edge(start_index + offset), @@ -268,14 +273,15 @@ impl NodeRef { /// the matching child node, if `self` is an internal node. /// /// The result is meaningful only if the tree is ordered by key. - fn find_lower_bound_index<'r, Q>( + fn find_lower_bound_index<'r, Q, C>( &self, - comparator: &impl Comparator, + comparator: &C, bound: SearchBound<&'r Q>, ) -> (usize, SearchBound<&'r Q>) where - K: Sortable, - Q: ?Sized + Borrow, + K: LookupKey, + Q: ?Sized + LookupKey, + C: Comparator, { match bound { Included(key) => match unsafe { self.find_key_index(key, comparator, 0) } { @@ -296,15 +302,16 @@ impl NodeRef { /// /// # Safety /// `start_index` must be a valid edge index for the node. - unsafe fn find_upper_bound_index<'r, Q>( + unsafe fn find_upper_bound_index<'r, Q, C>( &self, - comparator: &impl Comparator, + comparator: &C, bound: SearchBound<&'r Q>, start_index: usize, ) -> (usize, SearchBound<&'r Q>) where - K: Sortable, - Q: ?Sized + Borrow, + K: LookupKey, + Q: ?Sized + LookupKey, + C: Comparator, { match bound { Included(key) => match unsafe { self.find_key_index(key, comparator, start_index) } { diff --git a/src/btree/set.rs b/src/btree/set.rs index c31fb18..ef78fd5 100644 --- a/src/btree/set.rs +++ b/src/btree/set.rs @@ -2,7 +2,6 @@ // to TreeMap use alloc::vec::Vec; -use core::borrow::Borrow; use core::cmp::Ordering::{self, Equal, Greater, Less}; use core::cmp::{max, min}; use core::fmt::{self, Debug}; @@ -11,7 +10,7 @@ use core::iter::{FromIterator, FusedIterator, Peekable}; use core::mem::ManuallyDrop; use core::ops::{BitAnd, BitOr, BitXor, RangeBounds, Sub}; -use crate::{std_ord::Ord as _, Comparator, MinCmpFn, Sortable}; +use crate::{Comparator, LookupKey, OrdComparator}; use super::map::{BTreeMap, Keys}; use super::merge_iter::MergeIterInner; @@ -81,7 +80,7 @@ use crate::polyfill::*; // #[cfg_attr(not(test), rustc_diagnostic_item = "BTreeSet")] pub struct BTreeSet< T, - C = MinCmpFn, + C = OrdComparator, // #[unstable(feature = "allocator_api", issue = "32838")] A: Allocator + Clone = Global, > { @@ -217,8 +216,7 @@ enum DifferenceInner<'a, T: 'a, C, A: Allocator + Clone> { // Explicit Debug impl necessary because of issue #26925 impl Debug for DifferenceInner<'_, T, C, A> where - T: Sortable, - C: Comparator, + C: Comparator, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -246,8 +244,7 @@ where // #[stable(feature = "collection_debug", since = "1.17.0")] impl fmt::Debug for Difference<'_, T, C, A> where - T: Sortable, - C: Comparator, + C: Comparator, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_tuple("Difference").field(&self.inner).finish() @@ -308,8 +305,7 @@ enum IntersectionInner<'a, T: 'a, C, A: Allocator + Clone> { // Explicit Debug impl necessary because of issue #26925 impl Debug for IntersectionInner<'_, T, C, A> where - T: Sortable, - C: Comparator, + C: Comparator, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -334,8 +330,7 @@ where // #[stable(feature = "collection_debug", since = "1.17.0")] impl Debug for Intersection<'_, T, C, A> where - T: Sortable, - C: Comparator, + C: Comparator, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_tuple("Intersection").field(&self.inner).finish() @@ -376,10 +371,41 @@ impl BTreeSet { /// # Examples /// /// ``` - /// # #![allow(unused_mut)] - /// use copse::BTreeSet; + /// # use core::cmp::Ordering; + /// # use copse::{Comparator, LookupKey, BTreeSet}; + /// # + /// // define a comparator + /// struct NthByteComparator { + /// n: usize, // runtime state + /// } + /// + /// impl Comparator for NthByteComparator { + /// // etc + /// # type Key = str; + /// # fn cmp(&self, this: &str, that: &str) -> Ordering { + /// # match (this.as_bytes().get(self.n), that.as_bytes().get(self.n)) { + /// # (Some(lhs), Some(rhs)) => lhs.cmp(rhs), + /// # (Some(_), None) => Ordering::Greater, + /// # (None, Some(_)) => Ordering::Less, + /// # (None, None) => Ordering::Equal, + /// # } + /// # } + /// } + /// + /// // define lookup key types for collections sorted by our comparator + /// impl LookupKey for String { + /// // etc + /// # fn key(&self) -> &str { self.as_str() } + /// } + /// # impl LookupKey for str { + /// # fn key(&self) -> &str { self } + /// # } + /// + /// // create a set using our comparator + /// let mut set = BTreeSet::new(NthByteComparator { n: 10 }); /// - /// let mut set: BTreeSet = BTreeSet::default(); + /// // entries can now be inserted into the empty map + /// assert!(set.insert("abcdefghij".to_string())); /// ``` // #[stable(feature = "rust1", since = "1.0.0")] // #[rustc_const_stable(feature = "const_btree_new", since = "1.66.0")] @@ -393,8 +419,8 @@ impl BTreeSet { impl BTreeSet where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { /// Makes a new `BTreeSet` with a reasonable choice of B ordered by the given `comparator`. /// @@ -403,10 +429,10 @@ where /// ``` /// #![feature(allocator_api)] /// - /// use copse::BTreeSet; + /// use copse::{BTreeSet, OrdComparator}; /// use std::alloc::Global; /// - /// let mut set: BTreeSet = BTreeSet::new_in(Ord::cmp, Global); + /// let mut set: BTreeSet = BTreeSet::new_in(OrdComparator::default(), Global); /// ``` // #[unstable(feature = "btreemap_alloc", issue = "32838")] #[cfg(feature = "allocator_api")] @@ -446,7 +472,7 @@ where // #[stable(feature = "btree_range", since = "1.17.0")] pub fn range(&self, range: R) -> Range<'_, T> where - K: Borrow, + K: LookupKey, R: RangeBounds, { Range { @@ -496,12 +522,8 @@ where }; Difference { inner: match ( - self.map - .comparator - .cmp(self_min.borrow(), other_max.borrow()), - self.map - .comparator - .cmp(self_max.borrow(), other_min.borrow()), + self.map.comparator.cmp(self_min.key(), other_max.key()), + self.map.comparator.cmp(self_max.key(), other_min.key()), ) { (Greater, _) | (_, Less) => DifferenceInner::Iterate(self.iter()), (Equal, _) => { @@ -602,12 +624,8 @@ where }; Intersection { inner: match ( - self.map - .comparator - .cmp(self_min.borrow(), other_max.borrow()), - self.map - .comparator - .cmp(self_max.borrow(), other_min.borrow()), + self.map.comparator.cmp(self_min.key(), other_max.key()), + self.map.comparator.cmp(self_max.key(), other_min.key()), ) { (Greater, _) | (_, Less) => IntersectionInner::Answer(None), (Equal, _) => IntersectionInner::Answer(Some(self_min)), @@ -677,7 +695,7 @@ where // #[stable(feature = "rust1", since = "1.0.0")] pub fn contains(&self, value: &Q) -> bool where - Q: Borrow, + Q: LookupKey, { self.map.contains_key(value) } @@ -701,7 +719,7 @@ where // #[stable(feature = "set_recovery", since = "1.9.0")] pub fn get(&self, value: &Q) -> Option<&T> where - Q: Borrow, + Q: LookupKey, { Recover::get(&self.map, value) } @@ -767,22 +785,14 @@ where return false; // other is empty }; let mut self_iter = self.iter(); - match self - .map - .comparator - .cmp(self_min.borrow(), other_min.borrow()) - { + match self.map.comparator.cmp(self_min.key(), other_min.key()) { Less => return false, Equal => { self_iter.next(); } Greater => (), } - match self - .map - .comparator - .cmp(self_max.borrow(), other_max.borrow()) - { + match self.map.comparator.cmp(self_max.key(), other_max.key()) { Greater => return false, Equal => { self_iter.next_back(); @@ -802,7 +812,7 @@ where let mut self_next = self_iter.next(); while let Some(self1) = self_next { match other_iter.next().map_or(Less, |other1| { - self.map.comparator.cmp(self1.borrow(), other1.borrow()) + self.map.comparator.cmp(self1.key(), other1.key()) }) { Less => return false, Equal => self_next = self_iter.next(), @@ -997,7 +1007,7 @@ where // #[stable(feature = "rust1", since = "1.0.0")] pub fn remove(&mut self, value: &Q) -> bool where - Q: Borrow, + Q: LookupKey, { self.map.remove(value).is_some() } @@ -1021,7 +1031,7 @@ where // #[stable(feature = "set_recovery", since = "1.9.0")] pub fn take(&mut self, value: &Q) -> Option where - Q: Borrow, + Q: LookupKey, { Recover::take(&mut self.map, value) } @@ -1120,7 +1130,7 @@ where where A: Clone, C: Clone, - Q: Borrow, + Q: LookupKey, { BTreeSet { map: self.map.split_off(value), @@ -1270,12 +1280,12 @@ impl BTreeSet { } // #[stable(feature = "rust1", since = "1.0.0")] -impl FromIterator for BTreeSet> +impl FromIterator for BTreeSet where - T: Sortable, - T::State: Ord, + T: LookupKey, + C: Comparator + Default, { - fn from_iter>(iter: I) -> BTreeSet> { + fn from_iter>(iter: I) -> BTreeSet { let mut inputs: Vec<_> = iter.into_iter().collect(); if inputs.is_empty() { @@ -1283,15 +1293,16 @@ where } // use stable sort to preserve the insertion order. - inputs.sort_by(|a, b| a.borrow().cmp(b.borrow())); - BTreeSet::from_sorted_iter(inputs.into_iter(), T::State::cmp_fn(), Global) + let comparator = C::default(); + inputs.sort_by(|a, b| comparator.cmp(a.key(), b.key())); + BTreeSet::from_sorted_iter(inputs.into_iter(), comparator, Global) } } impl BTreeSet where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { fn from_sorted_iter>( iter: I, @@ -1305,10 +1316,10 @@ where } // #[stable(feature = "std_collections_from_array", since = "1.56.0")] -impl From<[T; N]> for BTreeSet> +impl From<[T; N]> for BTreeSet where - T: Sortable, - T::State: Ord, + T: LookupKey, + C: Comparator + Default, { /// Converts a `[T; N]` into a `BTreeSet`. /// @@ -1325,9 +1336,10 @@ where } // use stable sort to preserve the insertion order. - arr.sort_by(|a, b| a.borrow().cmp(b.borrow())); + let comparator = C::default(); + arr.sort_by(|a, b| comparator.cmp(a.key(), b.key())); let iter = IntoIterator::into_iter(arr).map(|k| (k, SetValZST::default())); - let map = BTreeMap::bulk_build_from_sorted_iter(iter, T::State::cmp_fn(), Global); + let map = BTreeMap::bulk_build_from_sorted_iter(iter, comparator, Global); BTreeSet { map } } } @@ -1359,8 +1371,7 @@ impl IntoIterator for BTreeSet { // #[stable(feature = "rust1", since = "1.0.0")] impl<'a, T, C, A: Allocator + Clone> IntoIterator for &'a BTreeSet where - T: Sortable, - C: Comparator, + C: Comparator, { type Item = &'a T; type IntoIter = Iter<'a, T>; @@ -1440,8 +1451,8 @@ impl FusedIterator for DrainFilter<'_, T, F, A> wher // #[stable(feature = "rust1", since = "1.0.0")] impl Extend for BTreeSet where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { #[inline] fn extend>(&mut self, iter: Iter) { @@ -1460,8 +1471,8 @@ where // #[stable(feature = "extend_ref", since = "1.2.0")] impl<'a, T: 'a + Ord + Copy, C, A: Allocator + Clone> Extend<&'a T> for BTreeSet where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { fn extend>(&mut self, iter: I) { self.extend(iter.into_iter().cloned()); @@ -1475,22 +1486,22 @@ where } // #[stable(feature = "rust1", since = "1.0.0")] -impl Default for BTreeSet> +impl Default for BTreeSet where - T: Sortable, - T::State: Ord, + T: LookupKey, + C: Comparator + Default, { /// Creates an empty `BTreeSet` ordered by the `Ord` trait. - fn default() -> BTreeSet> { - BTreeSet::new(T::State::cmp_fn()) + fn default() -> BTreeSet { + BTreeSet::new(C::default()) } } // #[stable(feature = "rust1", since = "1.0.0")] impl Sub<&BTreeSet> for &BTreeSet where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { type Output = BTreeSet; @@ -1519,8 +1530,8 @@ where // #[stable(feature = "rust1", since = "1.0.0")] impl BitXor<&BTreeSet> for &BTreeSet where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { type Output = BTreeSet; @@ -1549,8 +1560,8 @@ where // #[stable(feature = "rust1", since = "1.0.0")] impl BitAnd<&BTreeSet> for &BTreeSet where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { type Output = BTreeSet; @@ -1579,8 +1590,8 @@ where // #[stable(feature = "rust1", since = "1.0.0")] impl BitOr<&BTreeSet> for &BTreeSet where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { type Output = BTreeSet; @@ -1757,8 +1768,8 @@ impl Clone for Difference<'_, T, C, A> { // #[stable(feature = "rust1", since = "1.0.0")] impl<'a, T, C, A: Allocator + Clone> Iterator for Difference<'a, T, C, A> where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { type Item = &'a T; @@ -1771,7 +1782,7 @@ where let mut self_next = self_iter.next()?; loop { match other_iter.peek().map_or(Less, |&other_next| { - self.comparator.cmp(self_next.borrow(), other_next.borrow()) + self.comparator.cmp(self_next.key(), other_next.key()) }) { Less => return Some(self_next), Equal => { @@ -1820,8 +1831,8 @@ where // #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Difference<'_, T, C, A> where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { } @@ -1834,16 +1845,14 @@ impl Clone for SymmetricDifference<'_, T, C> { // #[stable(feature = "rust1", since = "1.0.0")] impl<'a, T, C> Iterator for SymmetricDifference<'a, T, C> where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { type Item = &'a T; fn next(&mut self) -> Option<&'a T> { loop { - let (a_next, b_next) = self - .0 - .nexts(&|a: &&T, b: &&T| self.1.cmp((*a).borrow(), (*b).borrow())); + let (a_next, b_next) = self.0.nexts(|&a, &b| self.1.cmp(a.key(), b.key())); if a_next.and(b_next).is_none() { return a_next.or(b_next); } @@ -1866,8 +1875,8 @@ where // #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for SymmetricDifference<'_, T, C> where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { } @@ -1896,8 +1905,8 @@ impl Clone for Intersection<'_, T, C, A> { // #[stable(feature = "rust1", since = "1.0.0")] impl<'a, T, C, A: Allocator + Clone> Iterator for Intersection<'a, T, C, A> where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { type Item = &'a T; @@ -1907,7 +1916,7 @@ where let mut a_next = a.next()?; let mut b_next = b.next()?; loop { - match self.comparator.cmp(a_next.borrow(), b_next.borrow()) { + match self.comparator.cmp(a_next.key(), b_next.key()) { Less => a_next = a.next()?, Greater => b_next = b.next()?, Equal => return Some(a_next), @@ -1944,8 +1953,8 @@ where // #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Intersection<'_, T, C, A> where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { } @@ -1958,15 +1967,13 @@ impl Clone for Union<'_, T, &C> { // #[stable(feature = "rust1", since = "1.0.0")] impl<'a, T, C> Iterator for Union<'a, T, C> where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { type Item = &'a T; fn next(&mut self) -> Option<&'a T> { - let (a_next, b_next) = self - .0 - .nexts(&|a: &&T, b: &&T| self.1.cmp((*a).borrow(), (*b).borrow())); + let (a_next, b_next) = self.0.nexts(|&a, &b| self.1.cmp(a.key(), b.key())); a_next.or(b_next) } @@ -1984,8 +1991,8 @@ where // #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Union<'_, T, C> where - T: Sortable, - C: Comparator, + T: LookupKey, + C: Comparator, { } diff --git a/src/btree/set/tests.rs b/src/btree/set/tests.rs index 973a5e5..1ada739 100644 --- a/src/btree/set/tests.rs +++ b/src/btree/set/tests.rs @@ -51,11 +51,7 @@ fn test_iter_min_max() { fn check(a: &[i32], b: &[i32], expected: &[i32], f: F) where - F: FnOnce( - &BTreeSet>, - &BTreeSet>, - &mut dyn FnMut(&i32) -> bool, - ) -> bool, + F: FnOnce(&BTreeSet, &BTreeSet, &mut dyn FnMut(&i32) -> bool) -> bool, { let mut set_a = BTreeSet::default(); let mut set_b = BTreeSet::default(); @@ -298,7 +294,7 @@ fn test_is_disjoint() { // Also implicitly tests the trivial function definition of is_superset fn test_is_subset() { fn is_subset(a: &[i32], b: &[i32]) -> bool { - let set_a = BTreeSet::from_iter(a.iter()); + let set_a = BTreeSet::<_, OrdComparator>::from_iter(a.iter()); let set_b = BTreeSet::from_iter(b.iter()); set_a.is_subset(&set_b) } @@ -338,7 +334,7 @@ fn test_is_subset() { #[test] fn test_is_superset() { fn is_superset(a: &[i32], b: &[i32]) -> bool { - let set_a = BTreeSet::from_iter(a.iter()); + let set_a = BTreeSet::<_, OrdComparator>::from_iter(a.iter()); let set_b = BTreeSet::from_iter(b.iter()); set_a.is_superset(&set_b) } @@ -474,7 +470,7 @@ fn test_zip() { x.insert(12); x.insert(11); - let mut y = BTreeSet::default(); + let mut y = BTreeSet::<_, OrdComparator>::default(); y.insert("foo"); y.insert("bar"); @@ -491,10 +487,10 @@ fn test_zip() { fn test_from_iter() { let xs = [1, 2, 3, 4, 5, 6, 7, 8, 9]; - let set = BTreeSet::from_iter(xs.iter()); + let set = BTreeSet::<_, OrdComparator>::from_iter(xs.iter()); for x in &xs { - assert!(set.contains(x)); + assert!(set.contains(&x)); } } @@ -545,10 +541,6 @@ fn test_recovery() { #[derive(Debug)] struct Foo(&'static str, i32); - impl Sortable for Foo { - type State = Self; - } - impl PartialEq for Foo { fn eq(&self, other: &Self) -> bool { self.0 == other.0 @@ -593,7 +585,7 @@ fn test_recovery() { #[allow(dead_code)] fn assert_covariance() { - fn set<'new>(v: BTreeSet<&'static str>) -> BTreeSet<&'new str> { + fn set<'new>(v: BTreeSet<&'static str, ()>) -> BTreeSet<&'new str, ()> { v } fn iter<'a, 'new>(v: Iter<'a, &'static str>) -> Iter<'a, &'new str> { @@ -610,132 +602,78 @@ fn assert_covariance() { #[allow(dead_code)] fn assert_sync() { - fn set(v: &BTreeSet) -> impl Sync + '_ - where - T::State: Ord, - { + fn set(v: &BTreeSet) -> impl Sync + '_ { v } - fn iter(v: &BTreeSet) -> impl Sync + '_ - where - T::State: Ord, - { + fn iter(v: &BTreeSet) -> impl Sync + '_ { v.iter() } - fn into_iter(v: BTreeSet) -> impl Sync - where - T::State: Ord, - { + fn into_iter(v: BTreeSet) -> impl Sync { v.into_iter() } - fn range(v: &BTreeSet) -> impl Sync + '_ - where - T::State: Ord, - { + fn range(v: &BTreeSet) -> impl Sync + '_ { v.range::(..) } - fn drain_filter(v: &mut BTreeSet) -> impl Sync + '_ - where - T::State: Ord, - { + fn drain_filter(v: &mut BTreeSet) -> impl Sync + '_ { v.drain_filter(|_| false) } - fn difference(v: &BTreeSet) -> impl Sync + '_ - where - T::State: Ord, - { + fn difference(v: &BTreeSet) -> impl Sync + '_ { v.difference(v) } - fn intersection(v: &BTreeSet) -> impl Sync + '_ - where - T::State: Ord, - { + fn intersection(v: &BTreeSet) -> impl Sync + '_ { v.intersection(v) } - fn symmetric_difference(v: &BTreeSet) -> impl Sync + '_ - where - T::State: Ord, - { + fn symmetric_difference(v: &BTreeSet) -> impl Sync + '_ { v.symmetric_difference(v) } - fn union(v: &BTreeSet) -> impl Sync + '_ - where - T::State: Ord, - { + fn union(v: &BTreeSet) -> impl Sync + '_ { v.union(v) } } #[allow(dead_code)] fn assert_send() { - fn set(v: BTreeSet) -> impl Send - where - T::State: Ord, - { + fn set(v: BTreeSet) -> impl Send { v } - fn iter(v: &BTreeSet) -> impl Send + '_ - where - T::State: Ord, - { + fn iter(v: &BTreeSet) -> impl Send + '_ { v.iter() } - fn into_iter(v: BTreeSet) -> impl Send - where - T::State: Ord, - { + fn into_iter(v: BTreeSet) -> impl Send { v.into_iter() } - fn range(v: &BTreeSet) -> impl Send + '_ - where - T::State: Ord, - { + fn range(v: &BTreeSet) -> impl Send + '_ { v.range::(..) } - fn drain_filter(v: &mut BTreeSet) -> impl Send + '_ - where - T::State: Ord, - { + fn drain_filter(v: &mut BTreeSet) -> impl Send + '_ { v.drain_filter(|_| false) } - fn difference(v: &BTreeSet) -> impl Send + '_ - where - T::State: Ord, - { + fn difference(v: &BTreeSet) -> impl Send + '_ { v.difference(v) } - fn intersection(v: &BTreeSet) -> impl Send + '_ - where - T::State: Ord, - { + fn intersection(v: &BTreeSet) -> impl Send + '_ { v.intersection(v) } - fn symmetric_difference(v: &BTreeSet) -> impl Send + '_ - where - T::State: Ord, - { + fn symmetric_difference(v: &BTreeSet) -> impl Send + '_ { v.symmetric_difference(v) } - fn union(v: &BTreeSet) -> impl Send + '_ - where - T::State: Ord, - { + fn union(v: &BTreeSet) -> impl Send + '_ { v.union(v) } } @@ -744,53 +682,29 @@ fn assert_send() { // Check that the member-like functions conditionally provided by #[derive()] // are not overridden by genuine member functions with a different signature. fn assert_derives() { - fn hash(v: BTreeSet, state: &mut H) - where - T::State: Ord, - { + fn hash(v: BTreeSet, state: &mut H) { v.hash(state); // Tested much more thoroughly outside the crate in btree_set_hash.rs } - fn eq(v: BTreeSet) - where - T::State: Ord, - { + fn eq(v: BTreeSet) { let _ = v.eq(&v); } - fn ne(v: BTreeSet) - where - T::State: Ord, - { + fn ne(v: BTreeSet) { let _ = v.ne(&v); } - fn cmp(v: BTreeSet) - where - T::State: Ord, - { + fn cmp(v: BTreeSet) { let _ = v.cmp(&v); } - fn min(v: BTreeSet, w: BTreeSet) - where - T::State: Ord, - { + fn min(v: BTreeSet, w: BTreeSet) { let _ = v.min(w); } - fn max(v: BTreeSet, w: BTreeSet) - where - T::State: Ord, - { + fn max(v: BTreeSet, w: BTreeSet) { let _ = v.max(w); } - fn clamp(v: BTreeSet, w: BTreeSet, x: BTreeSet) - where - T::State: Ord, - { + fn clamp(v: BTreeSet, w: BTreeSet, x: BTreeSet) { let _ = v.clamp(w, x); } - fn partial_cmp(v: &BTreeSet) - where - T::State: Ord, - { + fn partial_cmp(v: &BTreeSet) { let _ = v.partial_cmp(v); } } @@ -805,13 +719,13 @@ fn test_ord_absence() { let _ = set.into_iter(); } - fn set_debug(set: BTreeSet) { + fn set_debug(set: BTreeSet) { format!("{set:?}"); format!("{:?}", set.iter()); format!("{:?}", set.into_iter()); } - fn set_clone(mut set: BTreeSet) { + fn set_clone(mut set: BTreeSet) { set.clone_from(&set.clone()); } diff --git a/src/btree/split.rs b/src/btree/split.rs index e28cf53..3e833ed 100644 --- a/src/btree/split.rs +++ b/src/btree/split.rs @@ -1,9 +1,8 @@ -use crate::{Comparator, Sortable}; +use crate::{Comparator, LookupKey}; use super::node::{ForceResult::*, Root}; use super::search::SearchResult::*; use crate::polyfill::*; -use core::borrow::Borrow; impl Root { /// Calculates the length of both trees that result from splitting up @@ -31,15 +30,16 @@ impl Root { /// and if the ordering of `Q` corresponds to that of `K`. /// If `self` respects all `BTreeMap` tree invariants, then both /// `self` and the returned tree will respect those invariants. - pub fn split_off( + pub fn split_off( &mut self, key: &Q, - comparator: &impl Comparator, + comparator: &C, alloc: A, ) -> Self where - K: Sortable, - Q: Borrow, + K: LookupKey, + Q: LookupKey, + C: Comparator, { let left_root = self; let mut right_root = Root::new_pillar(left_root.height(), alloc.clone()); @@ -47,7 +47,7 @@ impl Root { let mut right_node = right_root.borrow_mut(); loop { - let mut split_edge = match left_node.search_node(key.borrow(), comparator) { + let mut split_edge = match left_node.search_node(key, comparator) { // key is going to the right tree Found(kv) => kv.left_edge(), GoDown(edge) => edge, diff --git a/src/btree/testing/crash_test.rs b/src/btree/testing/crash_test.rs index 9915086..cd922d6 100644 --- a/src/btree/testing/crash_test.rs +++ b/src/btree/testing/crash_test.rs @@ -3,7 +3,7 @@ use alloc::fmt::Debug; use std::cmp::Ordering; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; -use crate::Sortable; +use crate::OrdStoredKey; /// A blueprint for crash test dummy instances that monitor particular events. /// Some instances may be configured to panic at some point. @@ -61,8 +61,8 @@ pub struct Instance<'a> { panic: Panic, } -impl Sortable for Instance<'_> { - type State = Self; +impl OrdStoredKey for Instance<'_> { + type DefaultComparisonKey = Self; } #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/src/btree/testing/ord_chaos.rs b/src/btree/testing/ord_chaos.rs index 4d2ca18..ec89714 100644 --- a/src/btree/testing/ord_chaos.rs +++ b/src/btree/testing/ord_chaos.rs @@ -1,3 +1,4 @@ +use crate::OrdStoredKey; use std::cell::Cell; use std::cmp::Ordering::{self, *}; use std::ptr; @@ -11,10 +12,8 @@ pub enum Cyclic3 { } use Cyclic3::*; -use crate::Sortable; - -impl Sortable for Cyclic3 { - type State = Self; +impl OrdStoredKey for Cyclic3 { + type DefaultComparisonKey = Self; } impl PartialOrd for Cyclic3 { @@ -65,8 +64,8 @@ impl Governor { #[derive(Debug)] pub struct Governed<'a, T>(pub T, pub &'a Governor); -impl Sortable for Governed<'_, T> { - type State = Self; +impl OrdStoredKey for Governed<'_, T> { + type DefaultComparisonKey = Self; } impl PartialOrd for Governed<'_, T> { diff --git a/src/lib.rs b/src/lib.rs index a8430da..8b09b2e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,48 +13,60 @@ //! order. //! //! However, copse's collections do not use the [`Ord`] trait; instead, lookups can only ever -//! be performed using the [`Comparator`] supplied upon collection creation. This comparator -//! can only compare values of type `&T` for which it was defined, and hence such type must be -//! reachable from any key type `&Q` used to perform lookups in the collection. copse ensures -//! this via its [`Sortable`] trait, which will typically be implemented by the stored key type -//! `K`; its [`State`][Sortable::State] associated type then specifies the `T` in which -//! comparisons will be performed, and values of type `&Q` can be used as lookup keys provided -//! that `Q: Borrow`. +//! be performed using the [`Comparator`] supplied upon collection creation. This comparator +//! can only compare values of its [`Key`][Comparator::Key] associated type, and hence keys used +//! for lookups must implement [`LookupKey`] in order that the conversion can be performed. //! -//! For example, a collection using a `Comparator` comparator can store keys of type -//! `String` because `String` implements `Sortable`; moreover, lookups can be -//! performed using keys of type `&str` because `str` implements `Borrow` (due to the -//! reflexive blanket implementation). +//! # Example +//! ```rust +//! # use core::cmp::Ordering; +//! # use copse::{Comparator, LookupKey, BTreeSet}; +//! # +//! // define a comparator +//! struct NthByteComparator { +//! n: usize, // runtime state +//! } //! -//! Implementations of [`Sortable`] are provided for primitive and some common standard library -//! types, but storing keys of other foreign types may require newtyping. +//! impl Comparator for NthByteComparator { +//! type Key = str; +//! fn cmp(&self, this: &str, that: &str) -> Ordering { +//! match (this.as_bytes().get(self.n), that.as_bytes().get(self.n)) { +//! (Some(lhs), Some(rhs)) => lhs.cmp(rhs), +//! (Some(_), None) => Ordering::Greater, +//! (None, Some(_)) => Ordering::Less, +//! (None, None) => Ordering::Equal, +//! } +//! } +//! } //! -//! # Function item types -//! In addition to the type parameters familiar from the standard library collections, copse's -//! collections are additionally parameterised by the type of the [`Comparator`]. If the -//! comparator type is not explicitly named, it defaults to the type of the [`Ord::cmp`] -//! function for `K::State`. As noted in the documentation of the [`CmpFn`] type alias, this -//! is only a zero-sized function item type if the unstable `type_alias_impl_trait` feature is -//! enabled; otherwise it is a function pointer type, with ensuing size and indirect call -//! implications. Collections built using the zero-sized function item type can still be -//! used in stable code, however; just not using the default type parameter. For example: +//! // define lookup key types for collections sorted by our comparator +//! impl LookupKey for String { +//! fn key(&self) -> &str { self.as_str() } +//! } +//! impl LookupKey for str { +//! fn key(&self) -> &str { self } +//! } //! -//! ```rust -//! # use std::cmp::Ord; -//! # use copse::BTreeMap; -//! let mut ord_map = BTreeMap::new(Ord::cmp); -//! # ord_map.insert(123, "hello"); +//! // create a collection using our comparator +//! let mut set = BTreeSet::new(NthByteComparator { n: 10 }); +//! assert!(set.insert("abcdefghij".to_string())); +//! assert!(!set.insert("xxxxxxxxxj".to_string())); +//! assert!(set.contains("jjjjjjjjjj")); //! ``` //! -//! However, naming this type carries the usual problems associated with anonymous types like -//! closures; in certain situations you may be able to use `impl Comparator` for the type -//! parameter, but in other situations (in stable code) the function pointer may be -//! unavoidable. +//! # Collection type parameters +//! In addition to the type parameters familiar from the standard library collections, copse's +//! collections are additionally parameterised by the type of the [`Comparator`]. If the +//! comparator type is not explicitly named, it defaults to the [`OrdComparator`] for the +//! storage key's [`DefaultComparisonKey`][OrdStoredKey::DefaultComparisonKey], which yields +//! behaviour analagous to the standard library collections (i.e. sorted by the `Ord` trait). +//! If you find yourself using these items, then you should probably ditch copse for the +//! standard library instead. //! //! # Crate feature flags //! This crate defines a number of feature flags, none of which are enabled by default: //! -//! * the `std` feature provides [`Sortable`] implementations for some libstd types +//! * the `std` feature provides [`OrdStoredKey`] implementations for some libstd types //! that are not available in libcore + liballoc, namely [`OsString`] and [`PathBuf`]; //! //! * the `unstable` feature enables all other crate features, each of which enables the @@ -64,12 +76,11 @@ //! they are nevertheless included to ease tracking of the stdlib implementations. //! //! The most visible differences to library users will be: -//! * `allocator_api` enables the `new_in` methods for use of custom allocators; -//! * `specialization` adds the collection type name to some panic messages; -//! * `type_alias_impl_trait`, as mentioned above, ensures that the *default* -//! [`Comparator`] type parameter for the collections is the zero-sized function -//! item type of the `K::State::cmp` function. +//! * `allocator_api` enables the `new_in` methods for use of custom allocators; and +//! * `specialization` adds the collection type name to some panic messages. //! +//! [`Borrow`]: std::borrow::Borrow +//! [`Borrow`]: std::borrow::Borrow //! [`Ord`]: std::cmp::Ord //! [`Ord::cmp`]: std::cmp::Ord::cmp //! [`OsString`]: std::ffi::OsString @@ -94,7 +105,6 @@ #![cfg_attr(feature = "rustc_attrs", feature(rustc_attrs))] #![cfg_attr(feature = "slice_ptr_get", feature(slice_ptr_get))] #![cfg_attr(feature = "specialization", feature(specialization))] -#![cfg_attr(feature = "type_alias_impl_trait", feature(type_alias_impl_trait))] // documentation controls #![cfg_attr(docsrs, feature(doc_auto_cfg, doc_cfg))] #![deny(missing_docs)] @@ -105,7 +115,7 @@ extern crate alloc; use cfg_if::cfg_if; -use core::{borrow::Borrow, cmp::Ordering}; +use core::{cmp::Ordering, marker::PhantomData}; #[macro_use] mod polyfill; @@ -117,99 +127,105 @@ pub use btree::{map, set}; pub use map::BTreeMap; pub use set::BTreeSet; -mod std_ord { - use cfg_if::cfg_if; - use core::cmp::{Ord as StdOrd, Ordering}; - - // This trait and its associated type are deliberately named to conflict with - // the `Ord::cmp` function so that the generated documentation for the [`CmpFn`] - // type alias more naturally fits with how one might expect to name a function item - // type. - pub trait Ord: StdOrd { - #[allow(non_camel_case_types)] - type cmp: Copy + Fn(&Self, &Self) -> Ordering; - fn cmp_fn() -> Self::cmp; - } - - impl Ord for O { - cfg_if! { - if #[cfg(feature = "type_alias_impl_trait")] { - // With the TAIT feature, we can obtain the (otherwise anonymous) - // true (zero-sized) function item type. - type cmp = impl Copy + Fn(&Self, &Self) -> Ordering; - } else { - // Otherwise, we just use a function pointer (with ensuing storage) - // and indirect call implications. - type cmp = fn(&Self, &Self) -> Ordering; - } - } - - fn cmp_fn() -> Self::cmp { - Self::cmp - } - } +/// A comparator defines a total order over its associated type `Key`. +pub trait Comparator { + /// The type over which this comparator defines a total order. + type Key: ?Sized; - /// Alias for the type of the `::cmp` function. + /// Compare two values and return the position of `this` relative + /// to `that` according to the total order defined by this comparator. /// - /// Note that, unless the `type_alias_impl_trait` feature is enabled, this is - /// just the relevant function pointer type (and therefore is not zero-sized). - /// However, with that (unstable) feature enabled, this is an alias for the - /// actual zero-sized function item type. - pub type CmpFn = ::cmp; + /// The comparison must satisfy both transitivity and duality. + fn cmp(&self, this: &Self::Key, that: &Self::Key) -> Ordering; } -pub use std_ord::CmpFn; +/// A zero-sized comparator that delegates to the [`Ord`] implementation +/// of its type parameter `T`. +pub struct OrdComparator(PhantomData); -/// Alias for the `Ord::cmp` function of `K::State`. -pub type MinCmpFn = CmpFn<::State>; +impl Default for OrdComparator { + fn default() -> Self { + Self(PhantomData) + } +} -/// A comparator defines a total order over its type parameter `T`. -pub trait Comparator { - /// Compare two values and return their relative positions - /// according to the total order defined by this comparator. - fn cmp(&self, this: &T, that: &T) -> Ordering; +impl Clone for OrdComparator { + fn clone(&self) -> Self { + Self(PhantomData) + } } -// Blanket `Comparator` implementation for all suitable functions and closures -impl Ordering> Comparator for F { +impl Copy for OrdComparator {} + +impl Comparator for OrdComparator { + type Key = T; fn cmp(&self, this: &T, that: &T) -> Ordering { - self(this, that) + this.cmp(that) } } -/// A sortable type. See the [crate's documentation][crate] for details. -pub trait Sortable: Borrow { - /// A type that can be borrowed from `Self` and which contains all state necessary - /// for sorting. - type State: ?Sized; +/// A type that can be used as a lookup key in collections that are sorted by comparators of +/// type `C`. +/// +/// For example, if `MyComparator` is a [`Comparator`], then you may wish to implement +/// `LookupKey` for both `String` and `str` in order that keys of those types +/// can be used to search collections ordered by a comparator of type `MyComparator`. Note +/// that you must provide such implementation even for the reflexive/no-op case, which will +/// almost always be desirable. +pub trait LookupKey { + /// Returns a reference to the comparison key type. + fn key(&self) -> &C::Key; } -///////// `Sortable` implementations for standard library types ///////// +impl LookupKey> for K +where + K: ?Sized + core::borrow::Borrow, + T: ?Sized + Ord, +{ + fn key(&self) -> &T { + core::borrow::Borrow::borrow(self) + } +} + +/// A helper trait implemented on potential storage key types, used to identify their default +/// comparison type for `Ord`-based comparisons. +/// +/// This is only really used when collections are left to select the default [`OrdComparator`] +/// comparator, which essentially converts copse's collections into those already provided by +/// the standard library. This trait is therefore a convenience, but of relatively little value. +/// +/// For example, a collection that stores `String` under the default comparator will use `str` +/// as the comparison type owing to the implementation of this trait for `String`. +pub trait OrdStoredKey: LookupKey> { + /// The comparison type to be used by collections storing keys of `Self` type and using the + /// default [`OrdComparator`] comparator. + type DefaultComparisonKey: ?Sized + Ord; +} -macro_rules! minimals { +macro_rules! ord_keys { // end of recursion () => {}; // implement type and recurse ($(#[$attrs:meta])* $({$($g:tt)+})? $t:ty => $m:ty $(, $($rest:tt)*)?) => { $(#[$attrs])* - impl$(<$($g)+>)? Sortable for $t { - type State = $m; + impl$(<$($g)+>)? OrdStoredKey for $t { + type DefaultComparisonKey = $m; } - $(minimals!($($rest)*);)? + $(ord_keys!($($rest)*);)? }; // delegate to a reflexive implementation if no State is specified ($(#[$attrs:meta])* $({$($g:tt)+})? $t:ty $(, $($rest:tt)*)?) => { - minimals!($(#[$attrs])* $({$($g)+})? $t => Self $(, $($rest)*)?); + ord_keys!($(#[$attrs])* $({$($g)+})? $t => Self $(, $($rest)*)?); }; } -minimals! { +ord_keys! { (), bool, char, - f32, f64, + // f32, f64, i8, u8, i16, u16, i32, u32, @@ -218,26 +234,26 @@ minimals! { isize, usize, alloc::string::String => str, alloc::ffi::CString => core::ffi::CStr, - {B: ?Sized + Clone} alloc::borrow::Cow<'_, B> => B, - {T: ?Sized} &T => T, - {T: ?Sized} &mut T => T, - {T: ?Sized} alloc::rc::Rc => T, - {T: ?Sized} alloc::sync::Arc => T, - {T, const N: usize} [T; N] => [T], + {B: ?Sized + Ord + Clone} alloc::borrow::Cow<'_, B> => B, + {T: ?Sized + Ord} &T => T, + {T: ?Sized + Ord} &mut T => T, + {T: ?Sized + Ord} alloc::rc::Rc => T, + {T: ?Sized + Ord} alloc::sync::Arc => T, + {T: Ord, const N: usize} [T; N] => [T], #[cfg(feature = "std")] std::ffi::OsString => std::ffi::OsStr, #[cfg(feature = "std")] std::path::PathBuf => std::path::Path, } cfg_if! { if #[cfg(feature = "allocator_api")] { - minimals! { - {T, A: alloc::alloc::Allocator} alloc::vec::Vec => [T], - {T: ?Sized, A: alloc::alloc::Allocator} alloc::boxed::Box => T, + ord_keys! { + {T: Ord, A: alloc::alloc::Allocator} alloc::vec::Vec => [T], + {T: Ord + ?Sized, A: alloc::alloc::Allocator} alloc::boxed::Box => T, } } else { - minimals! { - {T} alloc::vec::Vec => [T], - {T: ?Sized} alloc::boxed::Box => T, + ord_keys! { + {T: Ord} alloc::vec::Vec => [T], + {T: Ord + ?Sized} alloc::boxed::Box => T, } } } diff --git a/src/polyfill.rs b/src/polyfill.rs index 19f916f..0388dd4 100644 --- a/src/polyfill.rs +++ b/src/polyfill.rs @@ -114,8 +114,8 @@ mod definitions { #[inline] fn new_uninit_in(alloc: A) -> Box!(MaybeUninit, A) { cfg_if! { - if #[cfg(feature = "new_uninit")] { - Self::new_uninit_in(alloc) + if #[cfg(all(feature = "allocator_api", feature = "new_uninit"))] { + ::new_uninit_in(alloc) } else { unsafe { let layout = Layout::new::>(); @@ -132,7 +132,7 @@ mod definitions { fn new_uninit() -> Box!(MaybeUninit) { cfg_if! { if #[cfg(feature = "new_uninit")] { - Self::new_uninit() + ::new_uninit() } else { ::new_uninit_in(Global) }