-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
make various intrinsics work in const contexts #50579
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Would you be so kind and also open a PR against miri removing all those that are added here? I think the only controversial one might be transmute, but you can implement a bad version of it with unions, so.... This is related to the const fn discussion, but since this PR is not actually stabilizing anything, just making already unstable intrinsics usable in consts, I see no reason not to do this. On the contrary, this allows us to test out things like transmute and discuss the behaviour we expect from it in consts |
Minor point: this doesn't constify bit_reverse because that's still unstable in the stdlib and I'm not sure what the bootstrap implications are for that. |
r? @oli-obk |
|
||
fn random() -> u32 { 0 } | ||
|
||
const fn sub(x: &u32) -> usize { | ||
unsafe { transmute(x) } //~ ERROR E0015 |
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.
so... I have a bad feeling about these intrinsics. The fact that this change was necessary makes me wonder whether the intrinsics have become insta-stable (because e.g. std::mem::transmute
is the intrinsic directly, without a wrapper function).
Please add tests that make sure that each intrinsic's stable version does not become const fn on stable Rust.
It's probably best to add some tests that check whether the intrinsics work with feature gates, too.
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.
There should probably be a #[const_unstable_...
attribute on the version of transmute
that's marked as stable, and somehow have that checked in qualify_consts
.
☔ The latest upstream changes (presumably #50249) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping from triage @gankro! It's been a while since we heard from you, will you have time to work on this again soon? |
Thank you for this PR @gankro! Unfortunately we haven't heard from you on this in a while, so I'm closing the PR to keep things tidy. Don't worry though, if you'll have time again in the future please reopen this PR, we'll be happy to review it again! |
These intrinsics are fairly inoffensive, and marking them as usable in const contexts doesn't make them any more usable to stable users while making them available to the stdlib at our own discretion. Doing this sooner rather than later minimizes the amount of
cfg(stage0)
shenanigans that will be required.Mostly this just avoids enum, dst, and pointer intrinsics.
The implementations are copied from miri verbatim.