Skip to content

Should PartialOrd and Ord impls be equivalent? #41270

Closed
@frankmcsherry

Description

@frankmcsherry

Nightly introduces a change that alters the behavior of sorting, by using the PartialOrd methods lt, le and such, rather than Ords cmp method.

This has implications for me, on account of I am a weirdo with types that implement both PartialOrd and Ord, with the property that Ord is a strict refinement of PartialOrd. That is, whenever partial_cmp returns a non-None value then cmp returns the same value, but there are incomparable elements of the partial order that are ordered by Ord. This sounds like a mess, but is important for doing things like efficient deduplication (via sort(), dedup()). For example, I have pairs of integers that are partially ordered (via product order), and I need to be able to deduplicate them.

I think this is a "breaking change" in that code that previously did one thing now does another thing. It may be that it is intended, and that all implementations of PartialOrd not equivalent to their Ord implementation were in error and get no love, but (i) it would be great to have someone actually state that, and (ii) the PartialOrd docs could use some sprucing up (they currently say that PartialOrd is for defining sort orders, which is ...).

I can fix all of my code, by moving all the PartialOrd functionality into the Lattice trait I already have, but I don't see why an Ord implementation should imply a total PartialOrd implementation (you should add a default implementation of cmp that unwraps partial_cmp in that case).

Edit bluss pointed me at #12517 which discusses the naming and stuff, but I couldn't see that the discussion came to a conclusion on whether PartialOrd must be equivalent to Ord when it exists.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-docsArea: Documentation for any part of the project, including the compiler, standard library, and toolsE-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.P-mediumMedium priorityT-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.regression-from-stable-to-betaPerformance or correctness regression from stable to beta.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions