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

Validity of enum variants with #[repr(...)] #[non_exhaustive] #240

Closed
TyPR124 opened this issue Jul 5, 2020 · 6 comments
Closed

Validity of enum variants with #[repr(...)] #[non_exhaustive] #240

TyPR124 opened this issue Jul 5, 2020 · 6 comments
Labels
A-layout Topic: Related to data structure layout (`#[repr]`) A-validity Topic: Related to validity invariants C-support Category: Supporting a user to solve a concrete problem

Comments

@TyPR124
Copy link

TyPR124 commented Jul 5, 2020

Take, for example

#[repr(u8)]
#[non_exhaustive]
enum Enum {
    Zero,
    One,
}

fn is_this_safe() -> Enum {
    unsafe { std::mem::transmute(2u8) }
}

I believe https://rust-lang.github.io/unsafe-code-guidelines/layout/enums.html explains well enough that this enum will be u8-sized, and will have variants Zero=0 and One=1. However, does adding #[non_exhaustive] make other bit-representations of this type valid? Similar questions for other kinds of (non-fieldless) enums.

Also, does #[non_exhaustive] have any affect on the rules around "option-like enums" and "niche" values?

@RalfJung
Copy link
Member

RalfJung commented Jul 5, 2020

However, does adding #[non_exhaustive] make other bit-representations of this type valid? Similar questions for other kinds of (non-fieldless) enums.

Also, does #[non_exhaustive] have any affect on the rules around "option-like enums" and "niche" values?

No. #[non_exhaustive] only affects type checking, but it has no effect at all on niches, validity, or UB.

That is the current status, anyway.

@RalfJung RalfJung added C-support Category: Supporting a user to solve a concrete problem A-layout Topic: Related to data structure layout (`#[repr]`) A-validity Topic: Related to validity invariants labels Jul 5, 2020
@TyPR124
Copy link
Author

TyPR124 commented Jul 5, 2020

Thanks, I figured it was unlikely to change validity (or layout), but it could be good to make this explicit (and relax it later if it ever changes). Someone could (with misguided logic) think any value is valid since any match will always include a catch-all arm.

I could add a note to the enum layout page, or add a (likely short) page on enum validity if you want.

@hanna-kruppe
Copy link

Someone could (with misguided logic) think any value is valid since any match will always include a catch-all arm.

Note that matches in the crate (or module? not sure) defining the non-exhaustive type don't need the catch-all arm. Perhaps this is the "misguided logic" you refer to but I want to make sure we're all on the same page.

@TyPR124
Copy link
Author

TyPR124 commented Jul 5, 2020

Note that matches in the crate (or module? not sure) defining the non-exhaustive type don't need the catch-all arm

I was not aware of this. I'm pretty sure this is the first I've ever heard that non_exhaustive behaves differently in different contexts. I did start with the language well before non_exhaustive was stabilized so it if wasn't mentioned specifically in release notes then I probably never would have known this until I found out the hard way somehow.

Still, I feel like if I had the thought that it could maybe affect validity, I have to assume someone else will think the same at some point.

I used "misguided" mostly in the sense that you can come to a logical conclusion that doesn't match reality. Although in this specific case having a fuller knowledge of non_exhaustive would have invalidated that logic anyway.

@RalfJung
Copy link
Member

RalfJung commented Jul 6, 2020

I could add a note to the enum layout page

It probably makes sense to explicitly say this when discussing niches etc, yes. If you could prepare a PR, that would be great. :)

or add a (likely short) page on enum validity if you want.

Such a page doesn't exist yet; #69 is where we discuss what should go onto that page. Can you leave a comment there?

@RalfJung
Copy link
Member

RalfJung commented Aug 8, 2020

I think this question has been answered / recorded in the relevant issue #69.

@RalfJung RalfJung closed this as completed Aug 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Topic: Related to data structure layout (`#[repr]`) A-validity Topic: Related to validity invariants C-support Category: Supporting a user to solve a concrete problem
Projects
None yet
Development

No branches or pull requests

3 participants