Repository navigation
Add Discriminant::unstable_cmp - #164231
Closed
bushrat011899 wants to merge 1 commit into
Closed
bushrat011899 wants to merge 1 commit into
bushrat011899 wants to merge 1 commit into
Conversation
rustbot
added
S-waiting-on-review
Collaborator
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
cuviper
reviewed
Oct 12, 2026
This comment has been minimized.
This comment has been minimized.
bushrat011899
force-pushed
the
discriminant_unstable_cmp
branch
from
October 12, 2026 23:33
efbd650 to
3dbf96a
Compare
bushrat011899
force-pushed
the
discriminant_unstable_cmp
branch
2 times, most recently
from
October 14, 2026 21:28
6b272b0 to
242c4c1
Compare
This comment has been minimized.
This comment has been minimized.
Provides an unstable ordering between `Discriminant<T>` values.
bushrat011899
force-pushed
the
discriminant_unstable_cmp
branch
from
October 14, 2026 22:07
242c4c1 to
2044e73
Compare
Mark-Simulacrum
added
S-waiting-on-ACP
Contributor
Author
|
This was referenced Nov 3, 2026
Contributor
Author
rustbot
added
S-waiting-on-review
Contributor
Author
|
Based on approval for the alternate implementation here, I'm going to close this PR. But it's easy enough to reopen if we want to revisit this and undo the other PR. Thanks to everyone who helped review this! |
rustbot
removed
the
S-waiting-on-review
JonathanBrouwer
added a commit
to JonathanBrouwer/rust
that referenced
this pull request
Nov 4, 2026
…clarfonthey Add `Discriminant::ord` ACP: rust-lang/libs-team#925 Tracking Issue: rust-lang#165438 Alternative To: rust-lang#164231 This is an implementation for Option 2 from rust-lang#165438. This provides a new method `Discriminant::ord` which returns a new type, `DiscriminantOrd`, suitable for key comparison. This follows a philosophy similar to that of `Path::display` for providing functional through standard traits gated behind a wrapper. I've elected for the return type of `Discriminant::ord` to be named as was requested during the T-Libs meeting proposing this option, and I've chosen to implement all traits currently implemented by `Discriminant`, this could be reduced. In comparison to rust-lang#164231, this has the advantage of allowing the storage of the discriminant in a structure and the deriving a `Partial/Ord` implementation. # Notes * No AI tooling of any kind was used during the creation of this PR. * Chose the name `ord` as that appeared to be the generally preferred option, but alternatives may include `unstable_ord`, etc.
JonathanBrouwer
added a commit
to JonathanBrouwer/rust
that referenced
this pull request
Nov 4, 2026
…clarfonthey Add `Discriminant::ord` ACP: rust-lang/libs-team#925 Tracking Issue: rust-lang#165438 Alternative To: rust-lang#164231 This is an implementation for Option 2 from rust-lang#165438. This provides a new method `Discriminant::ord` which returns a new type, `DiscriminantOrd`, suitable for key comparison. This follows a philosophy similar to that of `Path::display` for providing functional through standard traits gated behind a wrapper. I've elected for the return type of `Discriminant::ord` to be named as was requested during the T-Libs meeting proposing this option, and I've chosen to implement all traits currently implemented by `Discriminant`, this could be reduced. In comparison to rust-lang#164231, this has the advantage of allowing the storage of the discriminant in a structure and the deriving a `Partial/Ord` implementation. # Notes * No AI tooling of any kind was used during the creation of this PR. * Chose the name `ord` as that appeared to be the generally preferred option, but alternatives may include `unstable_ord`, etc.
JonathanBrouwer
added a commit
to JonathanBrouwer/rust
that referenced
this pull request
Nov 4, 2026
…clarfonthey Add `Discriminant::ord` ACP: rust-lang/libs-team#925 Tracking Issue: rust-lang#165438 Alternative To: rust-lang#164231 This is an implementation for Option 2 from rust-lang#165438. This provides a new method `Discriminant::ord` which returns a new type, `DiscriminantOrd`, suitable for key comparison. This follows a philosophy similar to that of `Path::display` for providing functional through standard traits gated behind a wrapper. I've elected for the return type of `Discriminant::ord` to be named as was requested during the T-Libs meeting proposing this option, and I've chosen to implement all traits currently implemented by `Discriminant`, this could be reduced. In comparison to rust-lang#164231, this has the advantage of allowing the storage of the discriminant in a structure and the deriving a `Partial/Ord` implementation. # Notes * No AI tooling of any kind was used during the creation of this PR. * Chose the name `ord` as that appeared to be the generally preferred option, but alternatives may include `unstable_ord`, etc.
rust-bors Bot
pushed a commit
that referenced
this pull request
Nov 5, 2026
Rollup merge of #165443 - bushrat011899:discriminant_ord, r=clarfonthey Add `Discriminant::ord` ACP: rust-lang/libs-team#925 Tracking Issue: #165438 Alternative To: #164231 This is an implementation for Option 2 from #165438. This provides a new method `Discriminant::ord` which returns a new type, `DiscriminantOrd`, suitable for key comparison. This follows a philosophy similar to that of `Path::display` for providing functional through standard traits gated behind a wrapper. I've elected for the return type of `Discriminant::ord` to be named as was requested during the T-Libs meeting proposing this option, and I've chosen to implement all traits currently implemented by `Discriminant`, this could be reduced. In comparison to #164231, this has the advantage of allowing the storage of the discriminant in a structure and the deriving a `Partial/Ord` implementation. # Notes * No AI tooling of any kind was used during the creation of this PR. * Chose the name `ord` as that appeared to be the generally preferred option, but alternatives may include `unstable_ord`, etc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
View all comments
ACP: rust-lang/libs-team#925
Tracking Issue: #165438
Supersedes: #106418
Zulip Discussion: #t-libs > Impl Partial/Ord for Discriminant
Based on this concern on the implementation of
Partial/OrdforDiscriminant, I'm proposing we instead offer anunstable_cmpfunction. This mirrors thefN::total_cmpfunction used to provideOrd-like functionality tof16/32/64/128without actually guaranteeing the type itself isOrd.Notes
unstable_cmphas been chosen to explicitly document at each usage that the ordering ofDiscriminantvalues is not stable and may change.const, but that requires either makingDiscriminantKind::Discriminant: const Ordor adding a fairly verbosewhere <T as DiscriminantKind>::Discriminant: [const] Ordtounstable_cmp. As such I'm deferring its inclusion. It would remain unstable untilconst_cmpis stabilized anyway.DiscriminantKind::Discriminanthave been simplified by removingPartialEqandClone, as they are implicitly included byEqandCopyrespectively. This also ensures that addingOrddoes not causerustfmtto wrap the line. See this comment for details.