Fix ordering in the least bad way possible maybe (fixes #38) (#39)

Author: udoprog

fixed-map-derive/src/context.rs

@@ -61,8 +61,10 @@ toks! {
         hash_t = [::core::hash::Hash],
         hasher_t = [::core::hash::Hasher],
         into_iterator_t = [::core::iter::IntoIterator],
+        iterator_cmp = [crate::macro_support::__storage_iterator_cmp],
         iterator_flat_map = [::core::iter::FlatMap],
         iterator_flatten = [::core::iter::Flatten],
+        iterator_partial_cmp = [crate::macro_support::__storage_iterator_partial_cmp],
         iterator_t = [::core::iter::Iterator],
         key_t = [crate::key::Key],
         mem = [::core::mem],

fixed-map-derive/src/unit_variants.rs

@@ -19,8 +19,10 @@ pub(crate) fn implement(cx: &Ctxt<'_>, en: &DataEnum) -> Result<TokenStream, ()>
     let hash = cx.toks.hash_t();
     let hasher = cx.toks.hasher_t();
     let into_iterator_t = cx.toks.into_iterator_t();
+    let iterator_cmp = cx.toks.iterator_cmp();
     let iterator_flat_map = cx.toks.iterator_flat_map();
     let iterator_flatten = cx.toks.iterator_flatten();
+    let iterator_partial_cmp = cx.toks.iterator_partial_cmp();
     let iterator_t = cx.toks.iterator_t();
     let key_trait = cx.toks.key_t();
     let mem = cx.toks.mem();
@@ -192,53 +194,15 @@ pub(crate) fn implement(cx: &Ctxt<'_>, en: &DataEnum) -> Result<TokenStream, ()>
             impl<V> #partial_ord_t for Storage<V> where V: #partial_ord_t {
                 #[inline]
                 fn partial_cmp(&self, other: &Self) -> Option<#ordering> {
-                    #partial_ord_t::partial_cmp(&self.data, &other.data)
-                }
-
-                #[inline]
-                fn lt(&self, other: &Self) -> bool {
-                    #partial_ord_t::lt(&self.data, &other.data)
-                }
-
-                #[inline]
-                fn le(&self, other: &Self) -> bool {
-                    #partial_ord_t::le(&self.data, &other.data)
-                }
-
-                #[inline]
-                fn gt(&self, other: &Self) -> bool {
-                    #partial_ord_t::gt(&self.data, &other.data)
-                }
-
-                #[inline]
-                fn ge(&self, other: &Self) -> bool {
-                    #partial_ord_t::ge(&self.data, &other.data)
+                    #iterator_partial_cmp(&self.data, &other.data)
                 }
             }
 
             #[automatically_derived]
             impl<V> #ord_t for Storage<V> where V: #ord_t {
                 #[inline]
                 fn cmp(&self, other: &Self) -> #ordering {
-                    #ord_t::cmp(self, other)
-                }
-
-                #[inline]
-                fn max(self, other: Self) -> Self {
-                    Self { data: #ord_t::max(self.data, other.data) }
-                }
-
-                #[inline]
-                fn min(self, other: Self) -> Self {
-                    Self { data: #ord_t::min(self.data, other.data) }
-                }
-
-                #[inline]
-                fn clamp(self, min: Self, max: Self) -> Self
-                where
-                    Self: #partial_ord_t<Self>
-                {
-                    Self { data: #ord_t::clamp(self.data, min.data, max.data) }
+                    #iterator_cmp(&self.data, &other.data)
                 }
             }
 

src/key.rs

@@ -52,6 +52,73 @@ use crate::storage::{BooleanStorage, OptionStorage, SingletonStorage, Storage};
 ///     Second,
 /// }
 /// ```
+///
+/// ## Ordering
+///
+/// Keys provide their own ordering semantics instead of relying on the
+/// [`PartialOrd`] and [`Ord`] traits.
+///
+/// Therefore keys when stored in a collection such as [`Map`] and [`Set`] are
+/// always ordered in *declaration order*. This allows those containers
+/// themselves to be ordered if the underlying key supports, it similarly to how
+/// [`BTreeMap`] and [`BTreeSet`] works.
+///
+/// ```
+/// use fixed_map::{Key, Set};
+///
+/// #[derive(Clone, Copy, Key)]
+/// enum Key {
+///     First,
+///     Second,
+///     Third,
+/// }
+///
+/// let mut a = Set::new();
+/// a.insert(Key::First);
+///
+/// let mut b = Set::new();
+/// b.insert(Key::Third);
+///
+/// let mut c = Set::new();
+/// c.insert(Key::First);
+/// c.insert(Key::Third);
+///
+/// assert!(a < b);
+/// assert!(c < b);
+/// assert!(a < c);
+/// ```
+///
+/// The same example with [`BTreeSet`]:
+///
+/// ```
+/// use std::collections::BTreeSet;
+///
+/// #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+/// enum Key {
+///     First,
+///     Second,
+///     Third,
+/// }
+///
+/// let mut a = BTreeSet::new();
+/// a.insert(Key::First);
+///
+/// let mut b = BTreeSet::new();
+/// b.insert(Key::Third);
+///
+/// let mut c = BTreeSet::new();
+/// c.insert(Key::First);
+/// c.insert(Key::Third);
+///
+/// assert!(a < b);
+/// assert!(c < b);
+/// assert!(a < c);
+/// ```
+///
+/// [`BTreeMap`]: std::collections::BTreeMap
+/// [`BTreeSet`]: std::collections::BTreeSet
+/// [`Map`]: crate::Map
+/// [`Set`]: crate::Set
 pub trait Key: Copy {
     /// The `Storage` implementation to use for the key implementing this trait.
     type Storage<V>: Storage<Self, V>;

src/lib.rs

@@ -347,3 +347,6 @@ pub mod storage;
 #[doc(hidden)]
 #[cfg(feature = "entry")]
 pub mod option_bucket;
+
+#[doc(hidden)]
+pub mod macro_support;

src/macro_support.rs

@@ -0,0 +1,43 @@
+//! ## PRIVATE API
+//!
+//! This API is private, for use only in the `derive(Key)` macro. Usage for
+//! other purposes is not supported, and this API will not abide by semver
+//! stability guarantees.
+
+use core::cmp::Ordering;
+
+#[inline]
+fn flatten<T>(value: (usize, &Option<T>)) -> Option<(usize, &T)> {
+    match value {
+        (index, Some(value)) => Some((index, value)),
+        _ => None,
+    }
+}
+
+/// `partial_cmp` implementation over iterators which ensures that storage
+/// ordering between `None` and `Some` is handled in a reasonable manner.
+#[allow(clippy::missing_inline_in_public_items)]
+pub fn __storage_iterator_partial_cmp<'a, A, B, T: 'a>(a: A, b: B) -> Option<Ordering>
+where
+    A: IntoIterator<Item = &'a Option<T>>,
+    B: IntoIterator<Item = &'a Option<T>>,
+    T: PartialOrd<T>,
+{
+    let a = a.into_iter().enumerate().filter_map(flatten);
+    let b = b.into_iter().enumerate().filter_map(flatten);
+    a.partial_cmp(b)
+}
+
+/// `cmp` implementation over iterators which ensures that storage ordering
+/// between `None` and `Some` is handled in a reasonable manner.
+#[allow(clippy::missing_inline_in_public_items)]
+pub fn __storage_iterator_cmp<'a, A, B, T: 'a>(a: A, b: B) -> Ordering
+where
+    A: IntoIterator<Item = &'a Option<T>>,
+    B: IntoIterator<Item = &'a Option<T>>,
+    T: Ord,
+{
+    let a = a.into_iter().enumerate().filter_map(flatten);
+    let b = b.into_iter().enumerate().filter_map(flatten);
+    a.cmp(b)
+}

src/map.rs

@@ -1088,6 +1088,8 @@ where
 
 /// [`PartialOrd`] implementation for a [`Map`].
 ///
+/// For more details on ordering, see the [`Key`] documentation.
+///
 /// # Examples
 ///
 /// ```
@@ -1097,18 +1099,20 @@ where
 /// enum Key {
 ///     First,
 ///     Second,
+///     Third,
 /// }
 ///
 /// let mut a = Map::new();
 /// a.insert(Key::First, 1);
 ///
 /// let mut b = Map::new();
-/// b.insert(Key::Second, 1);
+/// b.insert(Key::Third, 1);
 ///
-/// assert!(a > b);
-/// assert!(a >= b);
-/// assert!(!(a < b));
-/// assert!(!(a <= b));
+/// assert!(a < b);
+///
+/// let mut empty = Map::new();
+/// assert!(empty < a);
+/// assert!(empty < b);
 /// ```
 ///
 /// Using a composite key:
@@ -1129,7 +1133,7 @@ where
 /// b.insert(Key::Second, 1);
 ///
 /// // TODO: support this
-/// // assert!(a > b);
+/// // assert!(a < b);
 /// ```
 impl<K, V> PartialOrd for Map<K, V>
 where
@@ -1164,6 +1168,8 @@ where
 
 /// [`Ord`] implementation for a [`Map`].
 ///
+/// For more details on ordering, see the [`Key`] documentation.
+///
 /// # Examples
 ///
 /// ```
@@ -1181,10 +1187,10 @@ where
 /// let mut b = Map::new();
 /// b.insert(Key::Second, 1);
 ///
-/// let mut list = vec![a, b];
+/// let mut list = vec![b, a];
 /// list.sort();
 ///
-/// assert_eq!(list, [b, a]);
+/// assert_eq!(list, [a, b]);
 /// ```
 ///
 /// Using a composite key:

src/set.rs

@@ -645,6 +645,8 @@ where
 
 /// [`PartialOrd`] implementation for a [`Set`].
 ///
+/// For more details on ordering, see the [`Key`] documentation.
+///
 /// # Examples
 ///
 /// ```
@@ -654,18 +656,20 @@ where
 /// enum Key {
 ///     First,
 ///     Second,
+///     Third,
 /// }
 ///
 /// let mut a = Set::new();
 /// a.insert(Key::First);
 ///
 /// let mut b = Set::new();
-/// b.insert(Key::Second);
+/// b.insert(Key::Third);
 ///
-/// assert!(a > b);
-/// assert!(a >= b);
-/// assert!(!(a < b));
-/// assert!(!(a <= b));
+/// assert!(a < b);
+///
+/// let mut empty = Set::new();
+/// assert!(empty < a);
+/// assert!(empty < b);
 /// ```
 ///
 /// Using a composite key:
@@ -686,7 +690,7 @@ where
 /// b.insert(Key::Second);
 ///
 /// // TODO: support this
-/// // assert!(a > b);
+/// // assert!(a < b);
 /// ```
 impl<K> PartialOrd for Set<K>
 where
@@ -721,6 +725,8 @@ where
 
 /// [`Ord`] implementation for a [`Set`].
 ///
+/// For more details on ordering, see the [`Key`] documentation.
+///
 /// # Examples
 ///
 /// ```
@@ -738,10 +744,10 @@ where
 /// let mut b = Set::new();
 /// b.insert(Key::Second);
 ///
-/// let mut list = vec![a, b];
+/// let mut list = vec![b, a];
 /// list.sort();
 ///
-/// assert_eq!(list, [b, a]);
+/// assert_eq!(list, [a, b]);
 /// ```
 ///
 /// Using a composite key: