-
Notifications
You must be signed in to change notification settings - Fork 434
Add a validity testcase for uninhabited variants #4805
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
base: master
Are you sure you want to change the base?
Add a validity testcase for uninhabited variants #4805
Conversation
|
Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two. |
7a2c7c7 to
df5113a
Compare
|
|
||
| let val = 1u32; | ||
| let ptr = (&raw const val).cast::<E>(); | ||
| unsafe { ptr.read() }; //~ ERROR: encountered an uninhabited enum variant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A simple transmute from 1u32 should have the same result, right? That would even take care of the size check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure, I feel like you'd have a better understanding of what would happen here. Should I change it over to a transmute?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that would make this easier to understand.
Yeah, we probably mostly test this via const-eval tests... but can't hurt to also have a Miri test for this. |
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
Found this testcase in a random unmerged branch of mine. Looks like I wrote it as part of my work on rust-lang/rust#138961, and then forgot to submit it when plans wrt the scope of that PR have changed.
I have double checked by grepping for the error message and it looks like we still don't have any similar tests in miri.