Implement PartialOrd and Ord for Discriminant by EFanZh · Pull Request #106418 · rust-lang/rust (original) (raw)

Thanks for the ping here, Mark!

I remain strongly opposed to anything that makes it impossible for a library author to leave open the door to reorder their enum variants later.

If they want to opt-in to exposing the order, such as if these impls were impl<T> cmp::Ord for Discriminant<T> where T: ExposedVariantOrder with a #[derive(ExposedVariantOrder)] or something, then sure, having this would be fine.

But, for example, ControlFlow is intentionally not exposing the order of its variants right now

// ControlFlow should not implement PartialOrd or Ord, per RFC 3058:
// https://rust-lang.github.io/rfcs/3058-try-trait-v2.html#traits-for-controlflow

and I'd like to keep it that way, but that's impossible if this PR lands. This is not just a PR for two impls, this is a PR for "no public enum can ever be reordered later".

(As another example here, there's been work in the past to see whether we could reorder the variants in Result to make ? more efficient, which would have to include manually writing the Ord impl to match, but would be possible so long as this PR doesn't land, but if this does land, that becomes impossible.)

Even derive(PartialOrd) I could at least change from deriving it to implementing it manually in order to continue to provide consistent behaviour after reordering, but there's no way to fixup a Discriminant ordering later.

(Another direction that I'd like to see and would help the motivation here is better ways of accessing discriminants, like the derive in rust-lang/rfcs#3046 )