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

Jump to bottom

Matching on an enum should read the entire enum #56797

Closed
matthewjasper opened this issue Dec 13, 2018 · 2 comments
Closed

Matching on an enum should read the entire enum #56797

matthewjasper opened this issue Dec 13, 2018 · 2 comments
Labels
A-borrow-checker Area: The borrow checker A-NLL Area: Non Lexical Lifetimes (NLL) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@matthewjasper
Copy link
Contributor

matthewjasper commented Dec 13, 2018 •
edited

Enum layout optimizations mean that the discriminant of an enum may not be stored in a tag disjoint from the rest of the fields of the enum. Run this example with MIRI to see this is a problem. https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=4ea0e31adbb3fe1893be528277060248

This is fixed by #56790, but this issue is being raised for tracking any backports.

cc @nikomatsakis
@matthewjasper matthewjasper added A-borrow-checker Area: The borrow checker I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness labels Dec 13, 2018
@matthewjasper matthewjasper mentioned this issue Dec 14, 2018
Merged
@nikomatsakis nikomatsakis added the A-NLL Area: Non Lexical Lifetimes (NLL) label Dec 14, 2018
@goffrie
Copy link
Contributor

goffrie commented Dec 14, 2018

err... maybe I'm missing something, but doesn't the linked code sample already get rejected by NLL?

@matthewjasper
Copy link
Contributor Author

Yes, the example has been corrected now.

Centril added a commit to Centril/rust that referenced this issue Dec 16, 2018
@Centril
Rollup merge of rust-lang#56790 - rust-lang:borrowck-niche-discrimina…
d91032a 
…nts, r=nikomatsakis

Make RValue::Discriminant a normal Shallow read

Enum layout optimizations mean that the discriminant of an enum may not be stored in a tag disjoint from the rest of the fields of the enum. Stop borrow checking as though they are.

Run with MIRI to see why this is needed: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=09a3236685a06b6096e2e2e3968b852c.

This issue exists with the lexical borrow checker as well (see rust-lang#45045) so migrate mode should prevent this from being immediately breaking.

r? @nikomatsakis

Fixes rust-lang#56797
@pnkfelix pnkfelix added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. labels Dec 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-borrow-checker Area: The borrow checker A-NLL Area: Non Lexical Lifetimes (NLL) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness NLL-fixed-by-NLL Bugs fixed, but only when NLL is enabled. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nikomatsakis @pnkfelix @goffrie @matthewjasper