Unreachable variants of enum still impact layout

I've noticed some behavior in Rust's struct and enum layouts that I think could be improved (w.r.t. zero sized and unconstructable types).

I am currently using static type arguments to enable/disable variants of a type used to provide matching (instances of the matching behavior not included). I had hoped that we'd only pay (in size) for the enabled variants, but it seems that the enums are the size of the largest variant, not just the largest construct-able variant, leading to much larger sizes than hoped.

Please consider the following playground code which demonstrates this:

#[allow(unused_imports)]
use core::marker::PhantomData;
use std::fmt::Debug;
use std::mem::size_of;

// assert_eq! fails fast so we can't see the full picture. Use this instead.
macro_rules! check {
    ($left: expr, $right: expr) => {
        let left = $left;
        let right = $right;
        if left == right {
            eprintln!("  PASS: {} == {}", stringify!($left), stringify!($right));
        } else {
            if format!("{:?}", right) == stringify!($right) {
              eprintln!("  FAIL: {} => {:?} \texpected {}", stringify!($left), left, stringify!($right));
            } else {
              eprintln!("  FAIL: {} => {:?} \texpected {} => {:?}", stringify!($left), left, stringify!($right), right);
            }
        }
    };
}

// Marker trait just used to avoid accidents.
trait IsAllowed {}

// Implements a Void/Never/False type
// (note instances are derived for ease of use but Never has no values).
#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
enum Never {}
impl IsAllowed for Never {}

// Implements an Always/True type.
// (note instances are trivial and size is 0).
#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
struct Always;
impl IsAllowed for Always {}

// Here we make some enum variants impossible to construct via type arguments.
// This essentially disables some variants at compile time.
enum Matchable<'a, AnyAllowed: IsAllowed=Never, FuncAllowed: IsAllowed=Never, T=i32> {
    Id(T),
    Any(AnyAllowed),
    #[allow(dead_code)]
    // Func(FuncAllowed, PhantomData<Box<dyn 'a + Debug>>), // THIS EXHIBITS CORRECT SIZING.
    Func(FuncAllowed, &'a dyn Fn(&T) -> bool), // THIS DOES NOT.
    // The two should be the same size iff FuncAllowed is Never.
}
use Matchable::*;

fn main() {
  eprintln!("Ensure the values are constructable:");
  check!(format!("{:?}", Id(3) as Matchable), "Id(3)");
  check!(format!("{:?}", ANY), "ANY");
  
  eprintln!("Sizes for reference:");
  check!(size_of::<()>(), 0);
  check!(size_of::<i32>(), 4);
  check!(size_of::<()>(), 0);
  
  eprintln!("Fail due to impossible enum variants still impacting layout:");
  check!(size_of::<Matchable<Never, Never, ()>>(), 0);
  check!(size_of::<Matchable<Always, Never, ()>>(), 1);
  check!(size_of::<Matchable<Never, Never, i32>>(), 4);
  check!(size_of::<Matchable<Always, Never, i32>>(), 8);
 
  eprintln!("Fail when using the PhantomData solution as the function ptr is not carried:");
  
  check!(size_of::<Matchable<Never, Always, ()>>(), 16);
  check!(size_of::<Matchable<Always, Always, ()>>(), 24);
  check!(size_of::<Matchable<Never, Always, i32>>(), 24);
  check!(size_of::<Matchable<Always, Always, i32>>(), 24);
}

// Helpers:
const ANY: Matchable<'static, Always, Never> = Any(Always);

impl <'a, T: Debug, AnyAllowed: IsAllowed, FuncAllowed: IsAllowed> Debug for Matchable<'a, AnyAllowed, FuncAllowed, T> {
    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
        match self {
            Id(v) => write!(f, "Id({:?})", v),
            Any(_allowed) => write!(f, "ANY"),
            Func(_allowed, func) => write!(f, "<func {:?}>", std::ptr::addr_of!(func)),
        }
    }
}

(Playground)

Reposting from: Unreachable variants of enum still impact layout - help - The Rust Programming Language Forum As I did want help but it was suggested this was more about language design. Sorry for the noise.

1 Like

I believe I read a post you actually could reach the &'a dyn Fn(&T) -> bool even if FuncAllowed is Never with partial initialization. But I forgot how though. Anyway, I believe this is actually intended.

Edit: See Uninhabited types are ZSTs despite partial initialization being allowed in safe code. · Issue #49298 · rust-lang/rust · GitHub

I'd love to see that, it would justify this behaviour, though I'd question the semantics that got us there. Hope someone has a link fingers crossed

enum Never {}

enum Enum {
    Invalid(i32, Never),
}

fn main() {
    Enum::Invalid(123, panic!());
}

From Uninhabited types are ZSTs despite partial initialization being allowed in safe code. · Issue #49298 · rust-lang/rust · GitHub

2 Likes

I don't think this motivates the sizing of the variant.

The field values can be constructed but don't need to be stored in a variant as the (inevitable) panic will occur before they can ever be accessed.

But they may write to the stack; and they may override things there.

So, I don't think that example, as laid out there, is the motivator exactly: Enum::Invalid is a function with an uninhabited argument, and therefore uncallable in practice; the "stack" / local storage used for the 123 local thus goes into function-call storage.

The real issue would be with Enum::Invalid { 0: 123, 1: panic!() }, since that one is not a function call but a literal instantiation; so, imho, it seems like allowing an opt-in annotation on enums to forbid this last syntax, thus enabling this size optimization could be warranted.


@Cypher1 I guess it's workaround time; with feature(generic_associated_types), you can define a HKT wrapper:

trait Wrapper {
    type Wrap<T>;
    fn extract<T>(it: Self::Wrap<T>) -> T;
}

/// `<T> T`
enum Identity {}
impl Wrapper for Identity {
    type Wrap<T> = T;
    fn extract<T>(it: Self::Wrap<T>) -> T {
        it
    }
}

/// `<T> Never`
enum Never {}
impl Wrapper for Never {
    type Wrap<T> = Self;
    fn extract<T>(it: Self::Wrap<T>) -> T {
        match it {}
    }
}

and then use:

enum Matchable<…, FuncAllowed : Wrapper = Never, …> {
    …
    Func(FuncAllowed::Wrap<&'a dyn Fn(&T) -> bool>),
}

and when extracting the payload:

Func(func) => {
    let func = FuncAllowed::extract(func);
    …
}
2 Likes

Yes, the only way to get the behavior you want is to actually select between ! and T, because (T, !) is required to be the size of T.

This is more obvious with a bare tuple; initialization works something like the following:

let mut x: MaybeUninit<(i32, Allowed)> = MaybeUninit::uninit();
let p: *mut (i32, Allowed) = x.as_mut_ptr().cast();
addr_of!((*p).0).write(123);
addr_of!((*p).1).write(allowed);
let x: (i32, Allowed) = transmute(x);

With an enum variant, piecewise initialization currently isn't something that can be written by hand, because it's not possible to manually write the discriminant, or talk about the type of an enum known to be a specific variant (e.g. in order to get field offsets).

It could potentially be possible to get this size optimization for tuple variants only the way @dhm mentions, but that involves prohibiting braced initialization[1] as well as an unknown set of future functionality for working with enums unsafely directly and not via pattern matching.

Ultimately it's just much more straightforward for the language to say "are all variant fields uninhabited? then the variant does not exist" than "are any variant fields uninhabited? then the variant does not exist and by the way everyone with a generic parameter has to worry about maybe the variant not existing."

(To the point that if all fields being uninhabited deleting the variant wasn't just so dang useful I might potentially argue against it today, as unsafe code might want to write the discriminant before calling a function to emplace a generic type into a variant... Niche opts already make partial initialization of enums tricky, but enum variant types might make it possible, and that will have to watch out for deleted variants.)


Btw, GATs aren't needed for this; you can just use trait Wrapper<T> { type Wrapped; } instead of trait Wrapper { type Wrapped<T>; } since you only project a single type through each type parameter and don't need/want to hide it from the API.


  1. Or maybe it's possible to keep braced initialization by carefully requiring it to reserve max(size_of::<EnumWithInhabitedOpt>(), size_of::<EnumWithoutInhabitedOpt>) and ensuring that niche optimizations don't lay things out differently? That's additional complexity that I'm not sure is sound... ↩︎

3 Likes

Of course, writing generic code is messier as you're pushing the types into the generic list when not using GATs, but it works:

#[allow(unused_imports)]
use core::marker::PhantomData;
use std::fmt::Debug;
use std::mem::size_of;

// assert_eq! fails fast so we can't see the full picture. Use this instead.
macro_rules! check {
    ($left: expr, $right: expr) => {
        let left = $left;
        let right = $right;
        if left == right {
            eprintln!("  PASS: {} == {}", stringify!($left), stringify!($right));
        } else {
            if format!("{:?}", right) == stringify!($right) {
                eprintln!(
                    "  FAIL: {} => {:?} \texpected {}",
                    stringify!($left),
                    left,
                    stringify!($right)
                );
            } else {
                eprintln!(
                    "  FAIL: {} => {:?} \texpected {} => {:?}",
                    stringify!($left),
                    left,
                    stringify!($right),
                    right
                );
            }
        }
    };
}

// Marker trait just used to avoid accidents.
trait IsAllowed<T> {
    type Check;
}

// Implements a Void/Never/False type
// (note instances are derived for ease of use but Never has no values).
#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
enum Never {}
impl<T> IsAllowed<T> for Never {
    type Check = Never;
}

// Implements an Always/True type.
// (note instances are trivial and size is 0).
#[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
struct Always;
impl<T> IsAllowed<T> for Always {
    type Check = T;
}

// Here we make some enum variants impossible to construct via type arguments.
// This essentially disables some variants at compile time.
enum Matchable<
    'a,
    AnyAllowed: IsAllowed<()> = Never,
    FuncAllowed: IsAllowed<&'a dyn Fn(&T) -> bool> = Never,
    T = i32,
> {
    Id(T),
    Any(AnyAllowed::Check),
    #[allow(dead_code)]
    // Func(FuncAllowed, PhantomData<Box<dyn 'a + Debug>>), // THIS EXHIBITS CORRECT SIZING.
    Func(FuncAllowed::Check), // THIS DOES NOT.
                              // The two should be the same size iff FuncAllowed is Never.
}
use Matchable::*;

fn main() {
    eprintln!("Ensure the values are constructable:");
    check!(format!("{:?}", Id(3) as Matchable), "Id(3)");
    check!(format!("{:?}", ANY), "ANY");

    eprintln!("Sizes for reference:");
    check!(size_of::<()>(), 0);
    check!(size_of::<i32>(), 4);
    check!(size_of::<()>(), 0);

    eprintln!("Fail due to impossible enum variants still impacting layout:");
    check!(size_of::<Matchable<Never, Never, ()>>(), 0);
    check!(size_of::<Matchable<Always, Never, ()>>(), 1);
    check!(size_of::<Matchable<Never, Never, i32>>(), 4);
    check!(size_of::<Matchable<Always, Never, i32>>(), 8);

    eprintln!("Fail when using the PhantomData solution as the function ptr is not carried:");

    check!(size_of::<Matchable<Never, Always, ()>>(), 16);
    check!(size_of::<Matchable<Always, Always, ()>>(), 24);
    check!(size_of::<Matchable<Never, Always, i32>>(), 24);
    check!(size_of::<Matchable<Always, Always, i32>>(), 24);
}

// Helpers:
const ANY: Matchable<'static, Always, Never> = Any(());

impl<'a, T: Debug, AnyAllowed: IsAllowed<()>, FuncAllowed: IsAllowed<&'a dyn Fn(&T) -> bool>> Debug
    for Matchable<'a, AnyAllowed, FuncAllowed, T>
{
    fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
        match self {
            Id(v) => write!(f, "Id({:?})", v),
            Any(_) => write!(f, "ANY"),
            Func(func) => write!(f, "<func {:?}>", std::ptr::addr_of!(func)),
        }
    }
}
Ensure the values are constructable:
  PASS: format!("{:?}", Id(3) as Matchable) == "Id(3)"
  PASS: format!("{:?}", ANY) == "ANY"
Sizes for reference:
  PASS: size_of::<()>() == 0
  PASS: size_of::<i32>() == 4
  PASS: size_of::<()>() == 0
Fail due to impossible enum variants still impacting layout:
  PASS: size_of::<Matchable<Never, Never, ()>>() == 0
  PASS: size_of::<Matchable<Always, Never, ()>>() == 1
  PASS: size_of::<Matchable<Never, Never, i32>>() == 4
  PASS: size_of::<Matchable<Always, Never, i32>>() == 8
Fail when using the PhantomData solution as the function ptr is not carried:
  PASS: size_of::<Matchable<Never, Always, ()>>() == 16
  PASS: size_of::<Matchable<Always, Always, ()>>() == 24
  PASS: size_of::<Matchable<Never, Always, i32>>() == 24
  PASS: size_of::<Matchable<Always, Always, i32>>() == 24

(I just changed the trait impl)

But ... the problem is that actually accessing your func variant as-this-is doesn't work without an extract fn like @dhm provided, as you're getting back an abstract unusable Allowed::Check (but that can be trivially added as fn extract(Self::Check) -> T). Your debug fmt is taking the address of the binding, and not printing the actual function address (for that you want {:p}).

[playground]

1 Like

Thank you all! The extraction approach is a great work around and totally gives me what I need to make use of the benefits I was hoping to get for free (and I pray the optimizer will specialize and inline enough that I see that pay off well :slight_smile: ). I do think the original code is a tad simpler, but the change isn't actually too different, so perhaps there's little to gain here.

Really appreciate the discussion so far.

CAD97, I have questions. With the example you gave, it seems clear that the IR should do the same steps for the variant, as well as setting the tag. It does seem a shame that the variant's are not constructed and then moved as I imagine small variants could easily be constructed and used without points being involved (maybe this is done in optimization). This would also (I think) enable the optimization I've suggested, as the variant's members would have to be fully constructed before the variant was constructed. In this way, the stack would need to reserve space for the construct-able members of the variant but the size of the enum would not be impacted.

To make this more clear, consider what rustc says about the following

#[derive(Debug)]
enum Never{}

struct ANNA {
    a: i32,
    n: Never,
    a2: i32,
}

fn main() {
    dbg!(std::mem::size_of::<ANNA>());
    ANNA {
        a: dbg!(8),
        a2: dbg!(12),
        n: todo!(),
    };
}    

To which, Rustc says:

 Compiling playground v0.0.1 (/playground)
warning: unreachable expression
  --> src/main.rs:12:5
   |
12 | /     ANNA {
13 | |         a: dbg!(8),
14 | |         a2: dbg!(12),
15 | |         n: todo!(),
   | |            ------- any code following this expression is unreachable
16 | |     };
   | |_____^ unreachable expression
   |
   = note: `#[warn(unreachable_code)]` on by default

(as well as warning that the fields are never read).

As the expression of the Struct itself is not reachable, this proves that rustc knows that the struct cannot be constructed. We do not need to reserve space for the struct, just the members that will be constructed before the panic.

Assuming we see enum variants as being roughly equivalent to structs, I think this works for both.

shrug At this point I have a way forward that doesn't pay this cost, so I'm not too worried about it, but I do think it's an opportunity to enable clearer code that benefits from making use of type safety.

Thanks again to everyone.

The difference is that rustc has a special rule that if all fields are uninhabited, the variant is uninhabitable as well. The exact details are, of course, more complicated, but that's the gist of it.

This would imply that whether you ever construct a value would impact the size/inhabitedness of the value. This is… a questionable optimization for a lower level language on its own, and potentially unsound in the face of separate compilation and/or other ways to create an instance of a type (such as manually initializing it in place behind a manually allocd pointer).

To be clear, the warnings rustc is emitting here have nothing to do with the ANNA type. Instead, they have to do with the fact that rustc knows that todo!() diverges and any code after it is unreachable. If you instead use fn make() -> Never { todo!() }, you won't get any unreachable warnings.

The struct case is specifically such that the following code does not cause UB:

struct Anna<T> {
    a: i32,
    nn: i64,
    a2: T,
}

unsafe fn emplace_anna<T>(
    a: impl FnOnce(*mut i32),
    nn: impl FnOnce(*mut i64),
    a2: impl FnOnce(*mut T),
) -> Box<Anna<T>> {
    let mut b: Box<MaybeUninit<Anna<T>>> = Box::new();
    let p: *mut Anna<T> = b.as_mut_ptr();
    // you can use a Drop guard here to avoid leaking
    // on an unwind but that's needless complexity
    // for the purpose of showing this example
    a(addr_of_mut!((*p).a));
    nn(addr_of_mut!((*p).nn));
    a2(addr_of_mut!((*p).a2));
    Box::from_raw(Box::into_raw(b) as _)
}

fn make_anna<T>(f: impl FnOnce() -> T) -> Anna<T> {
    unsafe {
        emplace_anna(
            |a: *mut i32| a.write(0),
            |nn: *mut i64| nn.write(0),
            |a: *mut T| a.write(f()),
        )
    }
}

make_anna::<Never>(|| todo!());

Rust wants to make sure that lower level manual piecewise initialization of places like this is reasonable. We could potentially require checking some mem::is_inhabited::<T: ?Sized> before writing to an uninitialized place... but this is complexity that would make existing code unsound for questionable gain.

#[repr(Rust)] enums are actually more interesting, because it's currently not possible to manually piecewise initialize them like this. However, it's much more consistent and predictable to keep the same inhabited rules as with structs than to change it, and limit it to the simple "if all fields are uninhabited, delete the variant" which does have obvious material benefits.

This could change in the future. The niching algorithm is deliberately unspecified, and could change to delete variants with any uninhabited field in the future. However, this would be a careful change, and would unfortunately make enum E { Variant { fields } } niche differently than enum E { Variant(struct Variant { fields }) } (since the struct is inhabited, per the above).

All of this is underspecified and could potentially change in the future. But any changes that can silently make previously okay code UB are done very hesitantly and only when the benefit is clear.

3 Likes

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.