Layout and behavior of enums with uninhabited fields in some variants

[RalfJung]

This is about enums like the following:

enum E1 { A, B(!) }
enum E2 { A, B(!), C(i32, !) }

The first interesting point is that E1 actually will not get any space assigned for storing a discriminant, it has size 0. The algorithm that assigns discriminants entirely skips B. In Miri we had to make SetDiscriminant throw UB early if the requested variant to set is uninhabited, since otherwise we get ICEs later.

However, E2 has size 8, even though there is only a single valid variant and that variant has size 0. I think we currently always provide storage for all fields of all variants, even if they are uninhabited. This is useful because in theory it lets us compile E2::C(f(), panic!()) into something that does in-place initialization:

let val: E2;
val.C.0 = f();
panic!();
SetDiscriminant(val, C); // this would be UB but we don't get here

(AFAIK we currently don't actually do that, we introduce temporaries instead.)

For structs we have decided that we will always have storage for all fields, and this is unavoidable since safe code can partially initialize a struct. However, safe code cannot partially initialize an enum, so we could soundly decide that E2 has size 0. We have to decide between smaller enums and in-place initialization for arbitrary enums. (We can of course have small enums and then have an analysis that uses in-place initialization where possible -- but in generic MIR, it might not be possible to tell whether the variant we are about to initialize is inhabited.)

[Nadrieril]

Is it guaranteed (and documented somewhere) that ReadDiscriminant cannot return the discriminant of an uninhabited variant (on pain of UB)? Is that true even of repr(whatever) enums?

[RalfJung]

That is the current behavior as implemented in Miri, but I don't think we guarantee it. ReadDiscriminant is anyway more of an implementation detail, it doesn't show up directly in the surface syntax.

[Nadrieril]

Do we not specify what data a pattern-match accesses?

My real question is: is there or will there ever be a UB-free execution that can reach a match arm that matches on an uninhabited variant. Such an execution could look like the following (I'm assuming this example is fine for e.g. T=u8, do tell me if I'm wrong):

#![feature(never_type)]

#[repr(u8)]
enum Enum<T> {
    A = 0,
    B = 1,
    C(u8, T) = 2,
}

fn main() {
    let mut x: Enum<!> = Enum::A;
    unsafe { (&raw mut x).cast::<u8>().write(2u8) };
    match x {
        Enum::A => println!("got A!"),
        Enum::B => println!("got B!"),
        Enum::C(..) => println!("got C!"), // is this ever reachable?
    }
}

[RalfJung]

Do we not specify what data a pattern-match accesses?

We specify basically nothing here. It's all details of how exactly match gets lowered to MIR / MiniRust, and that isn't really specified in any way.

Personally I think it's reasonable to say that GetDiscriminant will never return the discriminant of an uninhabited enum. Though this is yet another case of special-casing uninhabited types in the semantics (similar to #413 in that sense), which not everyone is happy with. And for repr(C) enums it is particularly tricky since we make so many layout guarantees about them.

[GKFX]

There has been a good bit of discussion of how this affects code which manually constructs an enum at rust-lang/rust#120141. (Also an IRLO thread.) Essentially, there would be a need for a fallible variant of offset_of, since if the enum is too small to contain the fields of one of its variants there is no valid offset to give for those fields. This is not insurmountably difficult to handle but it is likely to end up generally awkward for users.

Personally, I think that a type smaller than the fields it contains is just too surprising/strange of an optimization, and I doubt that manual construction of an enum is the only thing it would affect.

Are there other ways to achieve the same performance benefits without changing the enum's size? E.g., since the Rust calling convention is private to the compiler, having functions that take or return Result<T, Uninhabited> actually (in the ABI) take/return T. More generically, this would be - if there is exactly one inhabited variant, pass that directly.

[zachs18]

enum E2 { A, B(!), C(i32, !) }

However, E2 has size 8, even though there is only a single valid variant and that variant has size 0. I think we currently always provide storage for all fields of all variants, even if they are uninhabited. This is useful because in theory it lets us compile E2::C(f(), panic!()) into something that does in-place initialization:

It seems like it should be possible to make E2 be 4 bytes while still having space for all fields by having C not hold space for a tag, since there is no sound program that could ever read or write the tag, and the tag is not exposed as a "real" field. (This optimization would not apply to repr(C/Int) enums, of course, only repr(Rust)).

I was looking at compiler/rustc_abi/src/layout.rs when working on rust-lang/rust#135802 and realized that that seemed doable, so here's a branch with that change applied, and everything seems to work fine in ui tests: rust-lang/rust@master...zachs18:rust:enum-uninhabited-variant-no-tag I haven't tested Miri with this change yet

[RalfJung]

It seems like it should be possible to make E2 be 4 bytes while still having space for all fields by having C not hold space for a tag, since there is no sound program that could ever read or write the tag, and the tag is not exposed as a "real" field. (This optimization would not apply to repr(C/Int) enums, of course, only repr(Rust)).

Yeah that makes sense. We already do something like that in some cases -- AFAIK when computing a niched enum layout, variants with uninhabited fields are ignored when determining discriminants.