Skip to content

Add Discriminant::unstable_cmp - #164231

Open
bushrat011899 wants to merge 1 commit into
rust-lang:mainfrom
bushrat011899:discriminant_unstable_cmp
Open

bushrat011899 wants to merge 1 commit into
rust-lang:mainfrom
bushrat011899:discriminant_unstable_cmp

Conversation

@bushrat011899

Copy link
Copy Markdown
Contributor

Tracking Issue: Not Yet Created
Supersedes: #106418
Zulip Discussion: #t-libs > Impl Partial/Ord for Discriminant

Based on this concern on the implementation of Partial/Ord for Discriminant, I'm proposing we instead offer an unstable_cmp function. This mirrors the fN::total_cmp function used to provide Ord-like functionality to f16/32/64/128 without actually guaranteeing the type itself is Ord.

Notes

  • No AI tooling of any kind was used during the creation of this PR.
  • The name unstable_cmp has been chosen to explicitly document at each usage that the ordering of Discriminant values is not stable and may change.
  • I have not created a tracking issue or ACP, but I'd be happy to upon request!
  • The function could be trivially made const, but that requires either making DiscriminantKind::Discriminant: const Ord or adding a fairly verbose where <T as DiscriminantKind>::Discriminant: [const] Ord to unstable_cmp. As such I'm deferring its inclusion. It would remain unstable until const_cmp is stabilized anyway.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 12, 2026
@rustbot

rustbot commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: libs
  • libs expanded to 12 candidates
  • Random selection from JohnTitor, Mark-Simulacrum

Comment thread library/core/src/mem/mod.rs Outdated
@rust-log-analyzer

This comment has been minimized.

@bushrat011899
bushrat011899 force-pushed the discriminant_unstable_cmp branch from efbd650 to 3dbf96a Compare October 12, 2026 23:33
Comment thread library/core/src/marker.rs Outdated
Comment thread library/core/src/mem/mod.rs Outdated
///
/// // Inequivalent arms will compare not-equal
/// assert_ne!(discriminant(&Foo::A(1)).unstable_cmp(&discriminant(&Foo::B(1))), Ordering::Equal);
/// ```

@asquared31415 asquared31415 Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a good way to make a non-example? Like

// !!! DONT DO THIS, this assert may or may not pass, no guarantees are provided
assert_eq!(b_discriminant.unstable_cmp(a_discriminant), Ordering::Less);

(note: i intentionally used an assert that will not pass with the current obvious implementation)

I know we have some UB non-examples in the standard library, but is there a standard for other non-examples? and if so, would it be a good idea to call this out?

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so what I've done is swapped the assertions to use assert!(matches!(...)) instead, since I think it's clearer to say assert!(matches!(..., Less | Greater)) than assert_ne!(..., Equal). To cover the non-example case, I've used a little hidden-line trickery to change the ordering of variants and write this as a should_panic example, so in the docs it'll get a nice red highlight saying it'll panic (only kind of a lie).

image

Provides an unstable ordering between `Discriminant<T>` values.
@bushrat011899
bushrat011899 force-pushed the discriminant_unstable_cmp branch from 3dbf96a to 6b272b0 Compare October 13, 2026 02:43

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants