Place unions, pointer casts and pointer derefs behind extra feature gates #51990
Conversation
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
Ping from triage @eddyb @nikomatsakis this PR needs your review. |
| if self.mode != Mode::Fn && | ||
| !self.tcx.sess.features_untracked().const_raw_ptr_deref { | ||
| emit_feature_err( | ||
| &self.tcx.sess.parse_sess, "const_raw_ptr_deref", |
There was a problem hiding this comment.
Pretty sure you wanted to gate on const_raw_ptr_to_usize_cast here instead.
|
@nikomatsakis Does this need FCP? This |
|
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 latest upstream changes (presumably #52409) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Ping from triage! @nikomatsakis this PR needs your review |
|
Ping from triage @nikomatsakis / @rust-lang/compiler: This PR requires your input. |
|
There are other operations on raw pointers we have to be careful about as well: Comparison (whether it is |
|
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 |
done |
|
What's the difference between |
|
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 |
|
Okay, let me ask differently: Most of the code does Just unions do something that at least looks completely different: They call |
|
calling |
Okay. And union field accesses in statics and consts are deliberately allowed for backwards compatibility? This is code that asked to be const and hence it's fine if miri shows strange errors? |
yes, this has been stable for a year and has been erroring with unhelpful errors before miri (field not found even though it obviously was there, just not the one that was assigned to) and with potentially strange errors after miri (if the transmute was unconst). |
|
r=me but needs rebase |
|
@bors r=nikomatsakis |
|
📌 Commit 4b731a9 has been approved by |
bors
added
S-waiting-on-bors
Place unions, pointer casts and pointer derefs behind extra feature gates
To ensure we don't stabilize these things together with const fn stabilization (or any other stabilization)
This PR moves union field accesses inside `const fn` behind a feature gate. It was possible without a feature gate before, but since `const fn` was behind a feature gate we can do this change.
While "dereferencing raw pointers" and "casting raw pointers to usize" were hard errors before this PR, one could work around them by abusing unions:
```rust
// deref
union Foo<T> {
x: &'static T,
y: *const T,
}
const FOO: u32 = unsafe { *Foo { y: 42 as *const T }.x };
// as usize cast
union Bar<T> {
x: usize,
y: *const T,
}
const BAR: usize = unsafe { Bar { y: &1u8 }.x };
```
r? @eddyb
cc @nikomatsakis
|
☀️ Test successful - status-appveyor, status-travis |
| this.add(Qualif::NOT_CONST); | ||
| } else { | ||
| let base_ty = proj.base.ty(this.mir, this.tcx).to_ty(this.tcx); | ||
| if let ty::TyRawPtr(_) = base_ty.sty { |
There was a problem hiding this comment.
I think this change is wrong. All derefs are NOT_CONST, because we can never prove anything strong enough about the pointer/reference. this.add(Qualif::NOT_CONST); was not within if let ty::TyRawPtr(_) = base_ty.sty {.
The practical effects are that this compiles on nightly:
#![feature(nll)]
const FOO: Option<&[[u8; 3]]> = Some(&[*b"foo"]);Thankfully, NLL is still unstable, so we didn't regress in any stable-visible way.
There was a problem hiding this comment.
All derefs are NOT_CONST, because we can never prove anything strong enough about the pointer/reference.
How that, safe references should be fine?
There was a problem hiding this comment.
Oh your comment in #54224 indicates this is about promotion. Then never mind, I anyway have no idea what happens there.^^
There was a problem hiding this comment.
The type is safe, but the value isn't guaranteed to be valid / not point to a static.
There was a problem hiding this comment.
I can't parse the end of your sentence, do you mean "isn't guaranteed to point to a static" or "isn't guaranteed to NOT point to a static"?
There was a problem hiding this comment.
"isn't guaranteed to (be valid | not point to a static)"
(as in, pointing to a static is as invalid as a random unsafe pointer, for promotion - since the value may be different at runtime)
There was a problem hiding this comment.
Uh, what? I would have thoughts statics are the least problematic to point at in a promoted (aka another static)...?
There was a problem hiding this comment.
Maybe in a constant, yeah, but in the general case, a static could've changed at runtime.
To ensure we don't stabilize these things together with const fn stabilization (or any other stabilization)
This PR moves union field accesses inside
const fnbehind a feature gate. It was possible without a feature gate before, but sinceconst fnwas behind a feature gate we can do this change.While "dereferencing raw pointers" and "casting raw pointers to usize" were hard errors before this PR, one could work around them by abusing unions:
r? @eddyb
cc @nikomatsakis