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:

• Implement the RFC – Implement arbitrary_enum_discriminant #60732
• Adjust documentation (see instructions on rustc-guide)
• Stabilization PR – Stabilize arbitrary_enum_discriminant #86860

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:

rust/src/libsyntax/parse/parser.rs

Lines 7781 to 7789 in 33cde4a

	} else if self.eat(&token::Eq) {
	disr_expr = Some(AnonConst {
	id: ast::DUMMY_NODE_ID,
	value: self.parse_expr()?,
	});
	if let Some(sp) = disr_expr.as_ref().map(|c| c.value.span) {
	any_disr.push(sp);
	}
	struct_def = VariantData::Unit(ast::DUMMY_NODE_ID);

[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 #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/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.

2. 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 rust-lang/rfcs#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 #60553 (comment) - 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., #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).

[CeleritasCelery]

How would making fieldless-but-non-c-like enums as-castable be a breaking change?

[jswrenn]

How would making fieldless-but-non-c-like enums as-castable be a breaking change?

If a library author defines a type like:

enum AnswerTo {
    TheQuestion{} = 42,
}

it's reasonable for dependents of that library to assume that assert_eq!(AnswerTo::TheQuestion as u8, 42) will hold across non-major versions of that library. The library author should consider changing the discriminant of that variant to be a breaking change.

However, that same library might think they don't have to worry about changing the discriminant of this enum:

#[repr(u8)]
enum AnswerTo {
    TheQuestion{} = 42,
}

...because #89234 makes it not as-castable. The library author can change the discriminant to whatever they'd like, and can safely assume they won't break downstream users.

But, if we revert #89234, this is no longer a safe assumption to make. Downstream users could start as-casting AnswerTo, depending on what rustc version they are using.

[Soveu]

One note:

#[repr(u8)]
enum Enum {
  A = 1,
  B(u32) = 2,
}

In this case, Enum::B could be treated in two ways - as 2u8 or as a fn(u32) -> Enum

[joshtriplett]

There are a set of changes related to enums that we need to make, including discriminants, as casts, AsRepr, and similar. Lang has had a design meeting on that, and needs to have another once someone supplies a complete design that fits these pieces together.

[jswrenn]

Would anyone like to collaborate with me on this design? I'm very familiar with these issues, and have some design sketches on hand, but would love a collaborator help construct a cohesive vision of the future.

[ehuss]

I'm going to close as I believe this is now essentially complete as of 1.66.

There might be some longer-term decisions to make around as casting. There are some strange exceptions:

• Fieldless enums with variants such as Tuple() or Struct{} allow as casting without a #[repr(…)], and have since 1.15.

// Allowed since 1.15, even without a #[repr]
enum Fieldless {
    Tuple(),
    Struct{},
    Unit,
}
assert_eq!(0, Fieldless::Tuple() as isize);
assert_eq!(1, Fieldless::Struct{} as isize);
assert_eq!(2, Fieldless::Unit as isize);

• Similar to above, fieldless enums allow as casting as long as you don't specify an explicit discriminant. Using an explicit discriminant suddenly prevents all as casts.

• Fieldless enums allow explicit discriminants on unit variants, but not struct or tuple (new 1.66). It's not clear to me why it was decided to allow this.

#[repr(u8)]
enum FieldlessWithDiscrimants {
    First = 10,
    Tuple(),
    Second = 20,
    Struct{},
    Unit,
}
assert_eq!(10, FieldlessWithDiscrimants::First as u8);
assert_eq!(11, FieldlessWithDiscrimants::Tuple() as u8);
assert_eq!(20, FieldlessWithDiscrimants::Second as u8);
assert_eq!(21, FieldlessWithDiscrimants::Struct{} as u8);
assert_eq!(22, FieldlessWithDiscrimants::Unit as u8);

I suspect further changes may require an RFC or some design decisions with the lang team.