Skip to content

Commit

Permalink
Merge #31: Remove Eq trait bound
Browse files Browse the repository at this point in the history
3280db4 Remove Eq trait bound (Tobin C. Harding)

Pull request description:

  The `PartialOrd` trait is object safe but `Ord` is not. We can make `ArbitraryOrd` object safe by softening the bounds and only requiring `PartialEq` - we still provide a blanket implementation of `Ord` if `Eq` is implemented for `T`.

  Add a unit test that verifies using `Box<dyn T>` - props to Poelstra for the idea.

ACKs for top commit:
  apoelstra:
    ACK 3280db4; successfully ran local tests; nice! Yeah, we never would have gotten this right by hand

Tree-SHA512: 865b2f2b8689d843b0771924552819e4d0e60ea53a3affcaa68c66bdeccb1cb9708073ee8baa25d641c4af647a2c1017a7791508f3b826f414ba411ccc58e1c7
  • Loading branch information
apoelstra committed Dec 30, 2024
2 parents 6a5b6f8 + 3280db4 commit dd3ecef
Showing 1 changed file with 17 additions and 4 deletions.
21 changes: 17 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ use core::ops::{Deref, DerefMut};
/// }
/// }
/// ```
pub trait ArbitraryOrd<Rhs = Self>: Eq + PartialEq<Rhs> {
pub trait ArbitraryOrd<Rhs = Self>: PartialEq<Rhs> {
/// Implements a meaningless, arbitrary ordering.
fn arbitrary_cmp(&self, other: &Rhs) -> Ordering;
}
Expand Down Expand Up @@ -154,11 +154,11 @@ impl<T: ArbitraryOrd> ArbitraryOrd for &T {
}

impl<T: ArbitraryOrd> PartialOrd for Ordered<T> {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some(self.cmp(other)) }
fn partial_cmp(&self, other: &Self) -> Option<Ordering> { Some((*self).arbitrary_cmp(other)) }
}

impl<T: ArbitraryOrd> Ord for Ordered<T> {
fn cmp(&self, other: &Self) -> Ordering { self.0.arbitrary_cmp(&other.0) }
impl<T: ArbitraryOrd + Eq> Ord for Ordered<T> {
fn cmp(&self, other: &Self) -> Ordering { (*self).arbitrary_cmp(other) }
}

impl<T: fmt::Display> fmt::Display for Ordered<T> {
Expand Down Expand Up @@ -260,4 +260,17 @@ mod tests {
assert_send::<Ordered<Point>>();
assert_sync::<Ordered<Point>>();
}

#[test]
fn trait_is_object_safe() {
extern crate std;
use std::boxed::Box;

// If this test builds then `ArbitraryOrd` is object safe.
#[allow(dead_code)]
struct ObjectSafe {
p: Box<dyn ArbitraryOrd<Self>>,
q: Box<dyn PartialOrd<Self>>, // Sanity check.
}
}
}

0 comments on commit dd3ecef

Please sign in to comment.