Made std::intrinsics::transmute() const fn. #53535
Conversation
rust-highfive
added
the
S-waiting-on-review
|
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 |
There was a problem hiding this comment.
You need to add transmute to
rust/src/libsyntax/feature_gate.rs
Line 222 in 6bf6d50
|
|
||
| #![feature(core_intrinsics)] | ||
|
|
||
| use std::intrinsics; |
There was a problem hiding this comment.
please write this test in terms of std::mem::transmute
| #[repr(transparent)] | ||
| struct Foo(u32); | ||
|
|
||
| const TRANSMUTED_U32: u32 = intrinsics::transmute(Foo(3)); |
There was a problem hiding this comment.
This test should not be passing without an additional const_transmute feature gate
|
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 |
|
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 |
|
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 |
| &self.tcx.sess.parse_sess, "const_transmute", | ||
| self.span, GateIssue::Language, | ||
| &format!("The use of std::mem::transmute() \ | ||
| is gated in {}s",self.mode)); |
| | | ||
| = help: add #![feature(const_transmute)] to the crate attributes to enable | ||
|
|
||
| error[E0015]: calls in constants are limited to constant functions, tuple structs and tuple variants |
There was a problem hiding this comment.
hmm... this error duplication isn't too great. Maybe unconditionally do is_const_fn = Some(def_id), so in case we emit the feature gate error, the rest of the code happily treats transmute as const fn (which doesn't matter, because we already emitted an error)
|
|
||
| #![feature(const_transmute)] | ||
|
|
||
| use std::mem; |
There was a problem hiding this comment.
can you also add a test that checks that static FOO: bool = unsafe { mem::transmute(3) }; is an error?
There was a problem hiding this comment.
The test exists now but it fails saying the sizes are wrong.
Likely you meant mem::transmute(3u8)?
There was a problem hiding this comment.
I'm slightly confused, because I built it on my box and it passes, as does the travis build.
|
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 |
| @@ -825,6 +825,17 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> { | |||
| | "cttz_nonzero" | |||
| | "ctlz" | |||
| | "ctlz_nonzero" => is_const_fn = Some(def_id), | |||
| "transmute" => { | |||
| is_const_fn = Some(def_id); | |||
There was a problem hiding this comment.
this needs to happen inside the mode != Fn, otherwise we are promoting transmute (treating anything as const inside Fn makes it immediately promotable)
still needs to be outside the feature gate check in order to have the better diagnostic
|
☔ The latest upstream changes (presumably #53671) made this pull request unmergeable. Please resolve the merge conflicts. |
|
When this lands, please also open an issue (or, better, a PR :D ) against miri to remove the |
|
What is the motivation for this change? Using |
|
The motivation is that there's no reason not to do it. you can already emulate this with unions on nightly. all this PR does is to allow doing suchconversions with transmute. Miri evaluates everything in the target's platform, so the endianess, pointer size and alignment oddities of the target platform is observable, completely ignoring the platform you are compiling on. I think you only run into trouble with the host when you go beyond the host's limits, e.g. |
Ah, thanks, that is reassuring. Ignore my comment. :) |
TheDarkula
force-pushed
the
master
branch
2 times, most recently
from
6d8f885
to
3409be9
Compare
| use std::mem; | ||
|
|
||
| static FOO: bool = unsafe { mem::transmute(3) }; | ||
| //~^ ERROR transmute called with types of different sizes [E0512] |
There was a problem hiding this comment.
Your test is passing because you are testing for the type size error message instead of the UB message
|
@bors r+ |
|
📌 Commit c5cae79 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - status-appveyor, status-travis |
r? @oli-obk
tracking issue: #53605