Should PartialOrd provide impl for Ord to avoid boilerplate?

[oblitum]

Currently std gives the following example in "How can I implement Ord?":

use std::cmp::Ordering;

#[derive(Eq)]
struct Person {
    id: u32,
    name: String,
    height: u32,
}

impl Ord for Person {
    fn cmp(&self, other: &Self) -> Ordering {
        self.height.cmp(&other.height)
    }
}

impl PartialOrd for Person {
    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
        Some(self.cmp(other))
    }
}

impl PartialEq for Person {
    fn eq(&self, other: &Self) -> bool {
        self.height == other.height
    }
}

I was wondering why this boilerplate couldn't be reduced, because it seems clear that in almost all cases of Ord implementation, PartialOrd implementation can be done in terms of the former.

I suspect this is in its current state because it exists prior to specialization? If you have the following:

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

Am I right that the std example could be updated to:

use std::cmp::Ordering;

#[derive(Eq)]
struct Person {
    id: u32,
    name: String,
    height: u32,
}

impl Ord for Person {
    fn cmp(&self, other: &Self) -> Ordering {
        self.height.cmp(&other.height)
    }
}

impl PartialEq for Person {
    fn eq(&self, other: &Self) -> bool {
        self.height == other.height
    }
}

?

[shingtaklam1324]

This conflicts with the existing blanket impl in core.

error[E0119]: conflicting implementations of trait `std::cmp::PartialOrd<&_>` for type `&_`:
 --> src/lib.rs:3:1
  |
3 | impl<T: Ord> PartialOrd for T {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: conflicting implementation in crate `core`:
          - impl<A, B> std::cmp::PartialOrd<&B> for &A
            where A: std::cmp::PartialOrd<B>, A: ?Sized, B: ?Sized;

(There was a previous discussion on reddit r/cpp)

The idea is a good one, since the existing method of providing custom impl of PartialOrd and Ord isn't great, with the same impl for PartialOrd for every type. ( Some(self.cmp(&other)) ). However I'm not sure this is possible right now, especially without Specialisation.

[oblitum]

@shingtaklam1324 what this is basically saying is that given impl<T: Ord> is too general that it also covers references (Ord refs), and there's a impl for references already, then the two collide?

[shingtaklam1324]

Pretty much.

Essentially what rustc is complaining about is that there are now two implementations of the same trait on the same type

PartialOrd<Rhs=Self>, so replace B with A in the error.

Consider a type A: Ord

1. First way
   1. since Ord: PartialOrd
   2. A must impl PartialOrd
   3. impl PartialOrd<&A> for &A from core's impl<A, B> std::cmp::PartialOrd<&B> for &A where A: PartialOrd<B>
2. Second Way
   1. impl Ord<&A> for &A from core's impl<'_, A> Ord for &'_ A
   2. impl PartialOrd<&A> for &A from your impl<T: Ord> PartialOrd for T

[scottmcm]

This has been a wish for a long time (such as rust-lang/rfcs#1028). I think it's better tracked in the RFCs repo, since it depends on languages changes, and is plausibly just a requirement to be addressed in a specialization RFC. (Or a different change, which would also take an RFC.)