Rust: Tracking issue for RFC 2363, "Allow arbitrary enums to have explicit discriminants"

[Centril]

This is a tracking issue for the RFC "Allow arbitrary enums to have explicit discriminants" (rust-lang/rfcs#2363).

Steps:

• [x] Implement the RFC – https://github.com/rust-lang/rust/pull/60732
• [ ] Adjust documentation (see instructions on rustc-guide)
• [ ] Stabilization PR (see instructions on rustc-guide)

Unresolved questions:

1. [ ] Should discriminants of enumerations with fields be specified as variant attributes?
2. [ ] Should this apply only to enumerations with an explicit representation?

[eddyb]

Mentoring-wise: I'd suggest trying out an example and then tracking down where the error comes from. I don't remember, it's either during parsing or in ast_validation.

Most of the compiler should already support this, where doing the uniform thing is easier.
The "niche" optimization might assume variants with any data have consecutive discriminants, but it should be easy to fix, and we can figure it out after an initial PR is open.

[jswrenn]

Has anyone stepped up to implement this? If not, I'd like to take a stab.

The current error arises in parsing. I haven't fiddled with it yet, but I suspect a good first step is to remove the else before this if: https://github.com/rust-lang/rust/blob/33cde4aac2ec1b0a493d0acaa4c4fb45de0c6e94/src/libsyntax/parse/parser.rs#L7781-L7789

[oli-obk]

Has anyone stepped up to implement this?

I don't know of anyone

If not, I'd like to take a stab.

Go for it!

I haven't fiddled with it yet, but I suspect a good first step is to remove the else before this if:

That looks like the right start, yes.

[jswrenn]

I've started work on this here by removing the parsing restriction preventing non-unit variants from having explicit discriminants. As you predicted @eddyb, things seem to basically just work; e.g.:

#![feature(core_intrinsics)]

enum Enum {
  A(u16) = 7,
  B(u32) = 42,
}

fn main() {
  use std::intrinsics::discriminant_value;

  unsafe {
    assert_eq!(discriminant_value(&Enum::A(1)), 7);
    assert_eq!(discriminant_value(&Enum::B(2)), 42);
  }
}

As it stands, adding this feature required removing more lines of code than it involved writing new code! :D

Future work that comes to mind include writing tests and putting this behind a feature flag. Shall I go ahead and file an initial PR?

[oli-obk]

Sweet! Yes please open a PR with the feature gate and throw some tests in for good measure (there may already be tests checking for the "discriminator values can only be used with a field-less enum" error message, those should now be emitting a feature gate error instead)

[Centril]

The #![feature(arbitrary_enum_discriminant)] was implemented on 2019-06-26 in https://github.com/rust-lang/rust/pull/60732 which was written by @jswrenn and reviewed by @pnkfelix and others.

[jswrenn]

Documentation

• [ ] Adjust documentation (see instructions on rustc-guide)

I've submitted a PR to update the reference to reflect this feature (in its current state): rust-lang-nursery/reference#639.

I do not believe that either the Book or _Rust By Example_ need updating:

1. The Book does not mention discriminants at all.
2. _Rust By Example_ mentions C-like enums, but does not go into their memory layout. Since this feature is only relevant to advance users who care about the memory layout of enums, _Rust By Example_ is probably ill-suited for the topic.

Unresolved Questions

1. [ ] Should discriminants of enumerations with fields be specified as variant attributes?

Presently: no. Discriminants of all types of enumerations are specified with = followed by a constant expression. I implemented this approach because doing so was simplest, but I'd also argue this is the _right_ approach since it means the syntax for specifying discriminants is consistent across variant types.

1. [ ] Should this apply only to enumerations with an explicit representation?

Presently: yes, this feature applies only to enumerations using a primitive representation. Since this feature is only relevant when you care about the precise memory layout of enums, this feature is only reasonable to use when the precise layout of an enum is well-specified.

[newpavlov]

What are layout guarantees for such enums? Ideally I would like to be able to write code likes this:

#[repr(u8, C)]
enum Foo {
    A = 0,
    B { a: u16 } = 1,
    C { a: u32, b: u16 } = 2,
}

And I want for this type to have exactly the same memory layout as:

#[repr(C)]
struct foo_t {
    pub discr: u8,
    pub payload: foo_u,
}

union foo_u {
    b: u16,
    c: c_t,
}

#[repr(C)]
struct c_t {
    a: u32,
    b: u16,
}

Meaning if repr(C) is used:

• Discriminant should always be stored in the beginning.
• Discriminant can not be used for niche-optimizations.
• Variant fields should follow repr(C).

Having such guarantees would allow in some cases some neat zero-cost translations of C types to safe Rust ones.

[eddyb]

@newpavlov You get most of that (the placement and size of the discriminant) from just #[repr(u8)], I'm not sure if #[repr(C)] adds more to that.

Keep in mind that since your discriminants are the default ones (starting at 0 and adding 1 for each variant), you don't need this feature!

We defined all these back in https://github.com/rust-lang/rfcs/pull/2195, for Servo <-> Firefox I believe.

[elichai]

What about casting those enums into the primitive?
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=2e69ca93ace1d4983bd9782074e2df6a

And then even #[repr(Rust)] support

[sztomi]

Being a rust newbie, I instinctively tried this and the compiler sent me to this issue. I see this is fairly old, but I'm not familiar with how quickly features make it into a stable version of the compiler. Is it possible to tell how far this feature is from stabilizing?

[eddyb]

I see this is fairly old

It's not even an year old, and within that, not much has happened once this was implemented.
There's https://github.com/rust-lang/rust/issues/60553#issuecomment-511235271 - which looks like stabilization could've been attempted, but nobody tried?

[nikomatsakis]

The next step to stabilizing would be to prepare a stabilization report, which basically summarizes:

• history of the feature -- when was it implemented, how long has it been on nightly, is anybody using it?
• what the feature does (very briefly, we don't have to duplicate the RFC)
• any changes since the RFC
• examples of test cases showing the behavior
• answers to any unresolved questions from the RFC
• any other interesting things that came up

[Centril]

For an example of a stabilization report, see e.g., https://github.com/rust-lang/rust/pull/67712 and please reach out to e.g., myself (Discord or Zulip works) before publishing it.

[jswrenn]

There's still work to be done to document this feature before stabilization—my PR updating the reference is stalled.

The main roadblock I hit was I wasn't aware of RFC2195 when I submitted that PR, and the details of that RFC aren't already well-documented in the reference. Since it's trivial to explain what this feature does in the context of RFC2195, a PR should probably first be submitted to the reference updating the Type Layout chapter to document RFC2195.

There're some hairy terminology issues nobody's been particularly consistent about, too.

[Centril]

@jswrenn Note that we do not require that you update the reference before stabilization. It's perfectly acceptable to write a good report and then have that report be the material for later changes to the book, the reference, the rustc-dev-guide, etc. I would be happy to work with you towards a stabilization report! (and ❤️ for the work you've done so far).

[elpiel]

Is there any progress on this RFC?

[jswrenn]

@elpiel I think I recall seeing some chatter about this interacting with const generics (or resolving the names of constants?) but I'd have to go searching for it.

EDIT: #70453 is what I had in mind.