Restore Equivalent/Comparable

Renamed OrdComparator to BasicComparator and made compatible with the
Equivalent/Comparable traits

Author: matthew-mcallister

crossbeam-skiplist/src/base.rs

@@ -1,6 +1,6 @@
 //! A lock-free skip list. See [`SkipList`].
 
-use crate::comparator::{Comparator, OrdComparator};
+use crate::comparator::{Comparator, BasicComparator};
 
 use alloc::alloc::{alloc, dealloc, handle_alloc_error, Layout};
 use core::cmp;
@@ -323,7 +323,7 @@ struct HotData {
 // As a further future optimization, if `!mem::needs_drop::<K>() && !mem::needs_drop::<V>()`
 // (neither key nor the value have destructors), there's no point in creating a new local
 // collector, so we should simply use the global one.
-pub struct SkipList<K, V, C = OrdComparator> {
+pub struct SkipList<K, V, C = BasicComparator> {
     /// The head of the skip list (just a dummy node, not a real entry).
     head: Head<K, V>,
 
@@ -1333,7 +1333,7 @@ impl<K, V, C> IntoIterator for SkipList<K, V, C> {
 /// The lifetimes of the key and value are the same as that of the `Guard`
 /// used when creating the `Entry` (`'g`). This lifetime is also constrained to
 /// not outlive the `SkipList`.
-pub struct Entry<'a: 'g, 'g, K, V, C = OrdComparator> {
+pub struct Entry<'a: 'g, 'g, K, V, C = BasicComparator> {
     parent: &'a SkipList<K, V, C>,
     node: &'g Node<K, V>,
     guard: &'g Guard,
@@ -1476,7 +1476,7 @@ where
 ///
 /// You *must* call `release` to free this type, otherwise the node will be
 /// leaked. This is because releasing the entry requires a `Guard`.
-pub struct RefEntry<'a, K, V, C = OrdComparator> {
+pub struct RefEntry<'a, K, V, C = BasicComparator> {
     parent: &'a SkipList<K, V, C>,
     node: &'a Node<K, V>,
 }
@@ -1651,7 +1651,7 @@ where
 }
 
 /// An iterator over the entries of a `SkipList`.
-pub struct Iter<'a: 'g, 'g, K, V, C = OrdComparator> {
+pub struct Iter<'a: 'g, 'g, K, V, C = BasicComparator> {
     parent: &'a SkipList<K, V, C>,
     head: Option<&'g Node<K, V>>,
     tail: Option<&'g Node<K, V>>,
@@ -1728,7 +1728,7 @@ where
 }
 
 /// An iterator over reference-counted entries of a `SkipList`.
-pub struct RefIter<'a, K, V, C = OrdComparator> {
+pub struct RefIter<'a, K, V, C = BasicComparator> {
     parent: &'a SkipList<K, V, C>,
     head: Option<RefEntry<'a, K, V, C>>,
     tail: Option<RefEntry<'a, K, V, C>>,
@@ -1830,7 +1830,7 @@ impl<'a, K: 'a, V: 'a, C> RefIter<'a, K, V, C> {
 }
 
 /// An iterator over a subset of entries of a `SkipList`.
-pub struct Range<'a: 'g, 'g, Q, R, K, V, C = OrdComparator>
+pub struct Range<'a: 'g, 'g, Q, R, K, V, C = BasicComparator>
 where
     C: Comparator<K> + Comparator<K, Q>,
     R: RangeBounds<Q>,
@@ -1946,7 +1946,7 @@ where
 }
 
 /// An iterator over reference-counted subset of entries of a `SkipList`.
-pub struct RefRange<'a, Q, R, K, V, C = OrdComparator>
+pub struct RefRange<'a, Q, R, K, V, C = BasicComparator>
 where
     C: Comparator<K> + Comparator<K, Q>,
     R: RangeBounds<Q>,

crossbeam-skiplist/src/comparator.rs

@@ -2,6 +2,8 @@
 
 use core::{borrow::Borrow, cmp::Ordering};
 
+use crate::equivalent::{Comparable, Equivalent};
+
 /// Key equality trait.
 ///
 /// This trait allows for very flexible comparison of objects. You may
@@ -51,30 +53,29 @@ pub trait Comparator<L: ?Sized, R: ?Sized = L>: Equivalator<L, R> {
     fn compare(&self, lhs: &L, rhs: &R) -> Ordering;
 }
 
-/// This comparator uses the standard library `Borrow` and `Ord` traits to
-/// compare values. When used in a data structure, this results in the same
-/// behavior as the standard `BTreeMap` interface.
+/// This comparator uses the `Equivalent` and `Comparable` traits to perform
+/// comparisons, which themselves fall back on the standard library `Borrow`
+/// and `Ord` traits. When used in a map, this results in the same behavior as
+/// the standard `BTreeMap` interface.
 #[derive(Clone, Copy, Debug, Default)]
-pub struct OrdComparator;
+pub struct BasicComparator;
 
-impl<K: ?Sized, Q: ?Sized> Equivalator<K, Q> for OrdComparator
+impl<K: ?Sized, Q: ?Sized> Equivalator<K, Q> for BasicComparator
 where
-    K: Borrow<Q>,
-    Q: Eq,
+    K: Equivalent<Q>,
 {
     #[inline]
     fn equivalent(&self, lhs: &K, rhs: &Q) -> bool {
-        <Q as PartialEq>::eq(lhs.borrow(), rhs)
+        <K as Equivalent<Q>>::equivalent(lhs, rhs)
     }
 }
 
-impl<K: ?Sized, Q: ?Sized> Comparator<K, Q> for OrdComparator
+impl<K: ?Sized, Q: ?Sized> Comparator<K, Q> for BasicComparator
 where
-    K: Borrow<Q>,
-    Q: Ord,
+    K: Comparable<Q>,
 {
     #[inline]
     fn compare(&self, lhs: &K, rhs: &Q) -> Ordering {
-        Ord::cmp(lhs.borrow(), rhs)
+        <K as Comparable<Q>>::compare(lhs, rhs)
     }
 }

crossbeam-skiplist/src/equivalent.rs

@@ -0,0 +1,54 @@
+// These traits are based on `equivalent` crate, but `K` and `Q` are flipped to avoid type inference issues:
+// https://github.com/indexmap-rs/equivalent/issues/5
+
+//! Traits for key comparison in maps.
+
+use core::{borrow::Borrow, cmp::Ordering};
+
+/// Key equivalence trait.
+///
+/// This trait allows hash table lookup to be customized. It has one blanket
+/// implementation that uses the regular solution with `Borrow` and `Eq`, just
+/// like `HashMap` does, so that you can pass `&str` to lookup into a map with
+/// `String` keys and so on.
+///
+/// # Contract
+///
+/// The implementor **must** hash like `Q`, if it is hashable.
+pub trait Equivalent<Q: ?Sized> {
+    /// Compare self to `key` and return `true` if they are equal.
+    fn equivalent(&self, key: &Q) -> bool;
+}
+
+impl<K: ?Sized, Q: ?Sized> Equivalent<Q> for K
+where
+    K: Borrow<Q>,
+    Q: Eq,
+{
+    #[inline]
+    fn equivalent(&self, key: &Q) -> bool {
+        PartialEq::eq(self.borrow(), key)
+    }
+}
+
+/// Key ordering trait.
+///
+/// This trait allows ordered map lookup to be customized. It has one blanket
+/// implementation that uses the regular solution with `Borrow` and `Ord`, just
+/// like `BTreeMap` does, so that you can pass `&str` to lookup into a map with
+/// `String` keys and so on.
+pub trait Comparable<Q: ?Sized>: Equivalent<Q> {
+    /// Compare self to `key` and return their ordering.
+    fn compare(&self, key: &Q) -> Ordering;
+}
+
+impl<K: ?Sized, Q: ?Sized> Comparable<Q> for K
+where
+    K: Borrow<Q>,
+    Q: Ord,
+{
+    #[inline]
+    fn compare(&self, key: &Q) -> Ordering {
+        Ord::cmp(self.borrow(), key)
+    }
+}

crossbeam-skiplist/src/lib.rs

@@ -260,3 +260,4 @@ pub mod set;
 pub use crate::{map::SkipMap, set::SkipSet};
 
 pub mod comparator;
+pub mod equivalent;

crossbeam-skiplist/src/map.rs

@@ -7,8 +7,8 @@ use std::ptr;
 
 use crate::{
     base::{self, try_pin_loop},
+    comparator::BasicComparator,
     comparator::Comparator,
-    comparator::OrdComparator,
 };
 use crossbeam_epoch as epoch;
 
@@ -18,7 +18,7 @@ use crossbeam_epoch as epoch;
 /// concurrent access across multiple threads.
 ///
 /// [`BTreeMap`]: std::collections::BTreeMap
-pub struct SkipMap<K, V, C = OrdComparator> {
+pub struct SkipMap<K, V, C = BasicComparator> {
     inner: base::SkipList<K, V, C>,
 }
 
@@ -45,9 +45,9 @@ impl<K, V, C> SkipMap<K, V, C> {
     /// # Example
     ///
     /// ```
-    /// use crossbeam_skiplist::{SkipMap, comparator::OrdComparator};
+    /// use crossbeam_skiplist::{SkipMap, comparator::BasicComparator};
     ///
-    /// let map: SkipMap<i32, &str> = SkipMap::with_comparator(OrdComparator);
+    /// let map: SkipMap<i32, &str> = SkipMap::with_comparator(BasicComparator);
     /// ```
     pub fn with_comparator(comparator: C) -> Self {
         Self {
@@ -589,7 +589,7 @@ where
 }
 
 /// A reference-counted entry in a map.
-pub struct Entry<'a, K, V, C = OrdComparator> {
+pub struct Entry<'a, K, V, C = BasicComparator> {
     inner: ManuallyDrop<base::RefEntry<'a, K, V, C>>,
 }
 
@@ -709,7 +709,7 @@ impl<K, V> fmt::Debug for IntoIter<K, V> {
 }
 
 /// An iterator over the entries of a `SkipMap`.
-pub struct Iter<'a, K, V, C = OrdComparator> {
+pub struct Iter<'a, K, V, C = BasicComparator> {
     inner: base::RefIter<'a, K, V, C>,
 }
 
@@ -749,7 +749,7 @@ impl<K, V, C> Drop for Iter<'_, K, V, C> {
 }
 
 /// An iterator over a subset of entries of a `SkipMap`.
-pub struct Range<'a, Q, R, K, V, C = OrdComparator>
+pub struct Range<'a, Q, R, K, V, C = BasicComparator>
 where
     C: Comparator<K> + Comparator<K, Q>,
     R: RangeBounds<Q>,

crossbeam-skiplist/src/set.rs

@@ -5,7 +5,7 @@ use std::ops::Deref;
 use std::ops::{Bound, RangeBounds};
 
 use crate::{
-    comparator::{Comparator, OrdComparator},
+    comparator::{BasicComparator, Comparator},
     map,
 };
 
@@ -15,7 +15,7 @@ use crate::{
 /// concurrent access across multiple threads.
 ///
 /// [`BTreeSet`]: std::collections::BTreeSet
-pub struct SkipSet<T, C = OrdComparator> {
+pub struct SkipSet<T, C = BasicComparator> {
     inner: map::SkipMap<T, (), C>,
 }
 
@@ -42,9 +42,9 @@ impl<T, C> SkipSet<T, C> {
     /// # Example
     ///
     /// ```
-    /// use crossbeam_skiplist::{SkipSet, comparator::OrdComparator};
+    /// use crossbeam_skiplist::{SkipSet, comparator::BasicComparator};
     ///
-    /// let map: SkipSet<i32> = SkipSet::with_comparator(OrdComparator);
+    /// let map: SkipSet<i32> = SkipSet::with_comparator(BasicComparator);
     /// ```
     pub fn with_comparator(comparator: C) -> Self {
         Self {
@@ -469,7 +469,7 @@ where
 }
 
 /// A reference-counted entry in a set.
-pub struct Entry<'a, T, C = OrdComparator> {
+pub struct Entry<'a, T, C = BasicComparator> {
     inner: map::Entry<'a, T, (), C>,
 }
 
@@ -574,7 +574,7 @@ impl<T> fmt::Debug for IntoIter<T> {
 }
 
 /// An iterator over the entries of a `SkipSet`.
-pub struct Iter<'a, T, C = OrdComparator> {
+pub struct Iter<'a, T, C = BasicComparator> {
     inner: map::Iter<'a, T, (), C>,
 }
 
@@ -605,7 +605,7 @@ impl<T, C> fmt::Debug for Iter<'_, T, C> {
 }
 
 /// An iterator over a subset of entries of a `SkipSet`.
-pub struct Range<'a, Q, R, T, C = OrdComparator>
+pub struct Range<'a, Q, R, T, C = BasicComparator>
 where
     C: Comparator<T> + Comparator<T, Q>,
     R: RangeBounds<Q>,

crossbeam-skiplist/tests/base.rs

@@ -902,8 +902,8 @@ fn drops() {
 }
 
 #[test]
-fn comparator() {
-    use crossbeam_skiplist::comparator::{Comparator, Equivalator};
+fn comparable_get() {
+    use crossbeam_skiplist::equivalent::{Comparable, Equivalent};
 
     #[derive(PartialEq, Eq, PartialOrd, Ord)]
     struct Foo {
@@ -916,9 +916,6 @@ fn comparator() {
         data: &'a [u8],
     }
 
-    #[derive(Clone, Copy, Default)]
-    struct FooComparator;
-
     impl PartialOrd for FooRef<'_> {
         fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
             Some(self.cmp(other))
@@ -938,35 +935,23 @@ fn comparator() {
         }
     }
 
-    impl<'a> Equivalator<Foo> for FooComparator {
-        fn equivalent(&self, lhs: &Foo, rhs: &Foo) -> bool {
-            lhs == rhs
-        }
-    }
-
-    impl<'a> Equivalator<Foo, FooRef<'a>> for FooComparator {
-        fn equivalent(&self, foo: &Foo, key: &FooRef<'a>) -> bool {
+    impl<'a> Equivalent<FooRef<'a>> for Foo {
+        fn equivalent(&self, key: &FooRef<'a>) -> bool {
             let a = u64::from_be_bytes(key.data[..8].try_into().unwrap());
             let b = u32::from_be_bytes(key.data[8..].try_into().unwrap());
-            a == foo.a && b == foo.b
+            a == self.a && b == self.b
         }
     }
 
-    impl<'a> Comparator<Foo> for FooComparator {
-        fn compare(&self, lhs: &Foo, rhs: &Foo) -> std::cmp::Ordering {
-            Ord::cmp(lhs, rhs)
-        }
-    }
-
-    impl<'a> Comparator<Foo, FooRef<'a>> for FooComparator {
-        fn compare(&self, foo: &Foo, key: &FooRef<'a>) -> std::cmp::Ordering {
+    impl<'a> Comparable<FooRef<'a>> for Foo {
+        fn compare(&self, key: &FooRef<'a>) -> std::cmp::Ordering {
             let a = u64::from_be_bytes(key.data[..8].try_into().unwrap());
             let b = u32::from_be_bytes(key.data[8..].try_into().unwrap());
-            Foo { a, b }.cmp(foo)
+            Foo { a, b }.cmp(self)
         }
     }
 
-    let s = SkipList::with_comparator(epoch::default_collector().clone(), FooComparator);
+    let s = SkipList::new(epoch::default_collector().clone());
     let foo = Foo { a: 1, b: 2 };
 
     let g = &epoch::pin();

crossbeam-skiplist/tests/set.rs

@@ -766,6 +766,9 @@ fn comparator() {
     assert!(s.contains(b"xy"));
     assert!(s.contains(b""));
 
-    let elems: Vec<_> = s.into_iter().collect();
+    let elems: Vec<_> = s.iter().map(|x| &x.value()[..]).collect();
     assert_eq!(elems, [&b""[..], b"a", b"ab"]);
+
+    s.remove(b"p");
+    assert!(!s.contains(b"a"));
 }