Skip to content

Update discriminant-elision optimization on Option-like enums #149

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

Merged
merged 12 commits into from
Jul 18, 2019

Conversation

gnzlbg
Copy link
Contributor

@gnzlbg gnzlbg commented Jun 22, 2019

This PR updates the discriminant-elision optimization on option-like enums with the semantics specified in rust-lang/rust#60300

The main change is requiring that option-like enums do not have any explicit #[repr(...)] attributes.

@RalfJung
Copy link
Member

RalfJung commented Jun 23, 2019

This still does not specify, though, that transmuting between e.g. u32 and Option<NonZeroU32> is safe, right? For that we have to say that the Some case of that option is represented exactly like the inner type, and the None case is 0. I guess this works for types that have a "single-value niche", which is the case for the NonZero*/NonNull types. For &T, their niche currently is just 0, but in the future might grow to also include some unaligned values (e.g., &u32 could have a niche consisting of the values 0..=3).

EDIT: Oh I see, that RFC pretty much fixes the None value to be 0. But where do our documents state that?

gnzlbg and others added 3 commits June 25, 2019 13:52
@gnzlbg
Copy link
Contributor Author

gnzlbg commented Jun 25, 2019

The last commit hyperlinks everything.

Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@RalfJung RalfJung merged commit 1c44b54 into rust-lang:master Jul 18, 2019
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

Successfully merging this pull request may close these issues.

2 participants