Skip to content

impl PartialOrd<[U]> for [T] #129039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
clarfonthey opened this issue Aug 13, 2024 · 0 comments · May be fixed by #129870
Open

impl PartialOrd<[U]> for [T] #129039

clarfonthey opened this issue Aug 13, 2024 · 0 comments · May be fixed by #129870
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@clarfonthey
Copy link
Contributor

clarfonthey commented Aug 13, 2024

This has an accepted ACP, but I'm going to file a separate issue here since I was having issues implementing it and don't want to forget about it. (Like I have for the past year.)

Essentially, the primary issue is the way we go about specializing PartialOrd for slices, which is not symmetric with the way we go about specializing PartialEq for slices.

Right now, PartialEq<[U]> for [T] relies on a corresponding SlicePartialEq<U> for [T], and there's no issue.

However, because Ord doesn't have a generic parameter, SlicePartialOrd also doesn't, and this means that we can't actually specialize PartialOrd<[U]> for [T]. And I don't really know how to fix things without breaking the existing optimizations in the standard library, which is why I haven't touched this yet.

All the relevant code is in library/core/src/slice/cmp.rs:

impl<T, U> PartialEq<[U]> for [T]
where
    T: PartialEq<U>,
{
    fn eq(&self, other: &[U]) -> bool {
        SlicePartialEq::equal(self, other)
    }

    fn ne(&self, other: &[U]) -> bool {
        SlicePartialEq::not_equal(self, other)
    }
}

trait SlicePartialEq<B> {
    fn equal(&self, other: &[B]) -> bool;

    fn not_equal(&self, other: &[B]) -> bool {
        !self.equal(other)
    }
}

impl<T: PartialOrd> PartialOrd for [T] {
    fn partial_cmp(&self, other: &[T]) -> Option<Ordering> {
        SlicePartialOrd::partial_compare(self, other)
    }
}

trait SlicePartialOrd: Sized {
    fn partial_compare(left: &[Self], right: &[Self]) -> Option<Ordering>;
}

The main issue is this impl here:

trait AlwaysApplicableOrd: SliceOrd + Ord {}
impl<A: AlwaysApplicableOrd> SlicePartialOrd for A {
    fn partial_compare(left: &[A], right: &[A]) -> Option<Ordering> {
        Some(SliceOrd::compare(left, right))
    }
}

Which essentially requires that SlicePartialOrd not have a generic parameter, since SliceOrd and Ord don't.

@clarfonthey clarfonthey added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant