Experiment: add unstable RHS type to Ord, impl PartialOrd<[U]> for [T] #129870
+121
−109
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an unstable
Rhs
parameter toOrd
in an attempt to implementPartialOrd<[U]> for [T]
without disrupting specialization. Doing so requires reducing the number of types implementingAlwaysApplicableOrd
, which could disrupt specialization, but this (hopefully) won't affect performance.In particular,
*const T
,*mut T
, andOption<T>
no longer are marked asAlwaysApplicableOrd
because there is no equivalent implementations ofPartialOrd
for them. In particular, there are not general implementations of the following, and adding them would be a larger API change than the implementation that was accepted by the ACP:Closes #129039, which tracks the implementation of the ACP rust-lang/libs-team#285.
This requires an FCP since it technically adds immediately-stable impls, although it's unclear whether these impls can actually affect users. See below for notes.
r? libs-api
Details
Currently, this expands the following impls:
into the following:
Which is technically a new impl in the sense that
for<'a, 'b> Ord<&'a T> for &'b T
now exists, although it's unclear whether this actually would be observable by users in practice. The reason for this impl is to ensure thatAlwaysApplicableOrd
can apply to references, since aPartialOrd
implementation exists for them, but it's unclear whether this is truly needed. No matter what, users won't be able to name these specific impls in bounds, but they will be able to use them in functions anonymously by callingOrd::cmp
directly.However, adding this type parameter helps make specialization possible for using
Ord
on slices whenever possible, since we no longer have to rely on the lifetimes of two parameters being exactly equal to swap inOrd
inPartialOrd
impls.Personally, I think that longer-term, it could be useful to have an
Rhs
type parameter forOrd
since it actually makes sense logically, even though it doesn't forEq
(which just adds reflexivity toPartialEq
, and thus doesn't depend on the RHS). However, this PR is only adding the type parameter to makePartialOrd
work as expected, and that would require its own ACP.