Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Union of non-null types should not be non-null #101096

Closed
andrepd opened this issue Aug 27, 2022 · 4 comments
Closed

Union of non-null types should not be non-null #101096

andrepd opened this issue Aug 27, 2022 · 4 comments

Comments

@andrepd
Copy link

andrepd commented Aug 27, 2022

Example

use std::mem::size_of;
use std::num::NonZeroIsize;

type Foo = &'static i32;

type Bar = NonZeroIsize;

union FooBar {
    foo: Foo, 
    bar: Bar,
}

fn main() {
    println!("{} {}", size_of::<Foo>(), size_of::<Option<Foo>>());
    println!("{} {}", size_of::<Bar>(), size_of::<Option<Bar>>());
    println!("{} {}", size_of::<FooBar>(), size_of::<Option<FooBar>>());
}

Expected behaviour

Just as Option<Foo> and Option<Bar> have the same size as Foo and Bar respectively, I expected that Option<FooBar> also has the same size as FooBar. However this does not happen. Adding #[rustc_nonnull_optimization_guaranteed] does not seem to make a difference.

@deltragon
Copy link
Contributor

AFAIK, this is undecided currently - see rust-lang/unsafe-code-guidelines#73 for more background/discussion on how to handle unions.

@RalfJung
Copy link
Member

This is deliberate, unions are currently by design not having any niches. unions are used to do whacky low-level things with the underlying bytes, and e.g. we do not assume that the list of variants is actually exhaustive -- one might want to add new variants in a future-compatible way later (this is not an uncommon use of unions in C).

@andrepd
Copy link
Author

andrepd commented Aug 29, 2022

AFAIK, this is undecided currently - see rust-lang/unsafe-code-guidelines#73 for more background/discussion on how to handle unions.

Thanks for the link! I read through pretty much the entire thread. Looks like there's much more to it that I thought.

@Noratrieb
Copy link
Member

Noratrieb commented Mar 14, 2023

I don't think keeping this issue open will have too much value. The validity of unions are tracked on the opsem repo already. Feel free to reopen if you disagree.

@Noratrieb Noratrieb closed this as not planned Won't fix, can't repro, duplicate, stale Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants