Use type based qualification for unions #90373
Conversation
Union field access is currently qualified based on the qualification of
a value previously assigned to the union. At the same time, every union
access transmutes the content of the union, which might result in a
different qualification.
For example, consider constants A and B as defined below, under the
current rules neither contains interior mutability, since a value used
in the initial assignment did not contain `UnsafeCell` constructor.
```rust
#![feature(untagged_unions)]
union U { i: u32, c: std::cell::Cell<u32> }
const A: U = U { i: 0 };
const B: std::cell::Cell<u32> = unsafe { U { i: 0 }.c };
```
To avoid the issue, the changes here propose to consider the content of
a union as opaque and use type based qualification for union types.
|
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
apiraino
added
the
T-compiler
compiler/rustc_const_eval/src/transform/check_consts/resolver.rs
Outdated
Show resolved
Hide resolved
| @@ -258,6 +258,9 @@ where | |||
| if Q::in_adt_inherently(cx, def, substs) { | |||
| return true; | |||
| } | |||
| if def.is_union() && Q::in_any_value_of_ty(cx, rvalue.ty(cx.body, cx.tcx)) { | |||
There was a problem hiding this comment.
This is correct, but conservative for things like drop, which unions must directly implement, it's never invoked on their fields. Will this get us in trouble wit ManuallyDrop in the future?
There was a problem hiding this comment.
Is the concern here that we will look inside ManuallyDrop and qualify value as needing drop based on the inner content? That seems like a possible issue, but more for already existing implementation, not for the new one?
There was a problem hiding this comment.
hmm good point. Ok. I'm convinced about this PR XD
|
r? @oli-obk |
tmiasko
force-pushed
the
union-qualification
branch
from
efd6c5d
to
3f778f3
Compare
|
@bors r+ |
|
📌 Commit 3f778f3 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (9ed5b94): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
A validity companion to changes from rust-lang#90373.
Extend check for UnsafeCell in consts to cover unions A validity companion to changes from rust-lang#90373. `@rust-lang/wg-const-eval`
Union field access is currently qualified based on the qualification of
a value previously assigned to the union. At the same time, every union
access transmutes the content of the union, which might result in a
different qualification.
For example, consider constants A and B as defined below, under the
current rules neither contains interior mutability, since a value used
in the initial assignment did not contain
UnsafeCellconstructor.To avoid the issue, the changes here propose to consider the content of
a union as opaque and use type based qualification for union types.
Fixes #90268.
@rust-lang/wg-const-eval