Skip to content

Commit e4593ef

Browse files
RalfJungRalfJung
committed
assigning to a union field can never drop now
1 parent 5bf6017 commit e4593ef

File tree

6 files changed

+14
-57
lines changed

6 files changed

+14
-57
lines changed

‎compiler/rustc_middle/src/mir/query.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ pub enum UnsafetyViolationDetails {
3535
UseOfMutableStatic,
3636
UseOfExternStatic,
3737
DerefOfRawPointer,
38-
AssignToDroppingUnionField,
3938
AccessToUnionField,
4039
MutationOfLayoutConstrainedField,
4140
BorrowOfLayoutConstrainedField,
@@ -78,11 +77,6 @@ impl UnsafetyViolationDetails {
7877
"raw pointers may be null, dangling or unaligned; they can violate aliasing rules \
7978
and cause data races: all of these are undefined behavior",
8079
),
81-
AssignToDroppingUnionField => (
82-
"assignment to union field that might need dropping",
83-
"the previous content of the field will be dropped, which causes undefined \
84-
behavior if the field was not properly initialized",
85-
),
8680
AccessToUnionField => (
8781
"access to union field",
8882
"the field may not be properly initialized: using uninitialized data will cause \

‎compiler/rustc_mir_build/src/check_unsafety.rs

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -431,16 +431,9 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
431431
let lhs = &self.thir[lhs];
432432
if let ty::Adt(adt_def, _) = lhs.ty.kind() && adt_def.is_union() {
433433
if let Some((assigned_ty, assignment_span)) = self.assignment_info {
434-
// To avoid semver hazard, we only consider `Copy` and `ManuallyDrop` non-dropping.
435-
if !(assigned_ty
436-
.ty_adt_def()
437-
.map_or(false, |adt| adt.is_manually_drop())
438-
|| assigned_ty
439-
.is_copy_modulo_regions(self.tcx.at(expr.span), self.param_env))
440-
{
441-
self.requires_unsafe(assignment_span, AssignToDroppingUnionField);
442-
} else {
443-
// write to non-drop union field, safe
434+
if assigned_ty.needs_drop(self.tcx, self.tcx.param_env(adt_def.did())) {
435+
// This would be unsafe, but should be outright impossible since we reject such unions.
436+
self.tcx.sess.delay_span_bug(assignment_span, "union fields that need dropping should be impossible");
444437
}
445438
} else {
446439
self.requires_unsafe(expr.span, AccessToUnionField);
@@ -537,7 +530,6 @@ enum UnsafeOpKind {
537530
UseOfMutableStatic,
538531
UseOfExternStatic,
539532
DerefOfRawPointer,
540-
AssignToDroppingUnionField,
541533
AccessToUnionField,
542534
MutationOfLayoutConstrainedField,
543535
BorrowOfLayoutConstrainedField,
@@ -555,7 +547,6 @@ impl UnsafeOpKind {
555547
UseOfMutableStatic => "use of mutable static",
556548
UseOfExternStatic => "use of extern static",
557549
DerefOfRawPointer => "dereference of raw pointer",
558-
AssignToDroppingUnionField => "assignment to union field that might need dropping",
559550
AccessToUnionField => "access to union field",
560551
MutationOfLayoutConstrainedField => "mutation of layout constrained field",
561552
BorrowOfLayoutConstrainedField => {
@@ -600,11 +591,6 @@ impl UnsafeOpKind {
600591
"raw pointers may be null, dangling or unaligned; they can violate aliasing rules \
601592
and cause data races: all of these are undefined behavior",
602593
),
603-
AssignToDroppingUnionField => (
604-
Cow::Borrowed(self.simple_description()),
605-
"the previous content of the field will be dropped, which causes undefined \
606-
behavior if the field was not properly initialized",
607-
),
608594
AccessToUnionField => (
609595
Cow::Borrowed(self.simple_description()),
610596
"the field may not be properly initialized: using uninitialized data will cause \

‎compiler/rustc_mir_transform/src/check_unsafety.rs

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -219,22 +219,15 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> {
219219
// We have to check the actual type of the assignment, as that determines if the
220220
// old value is being dropped.
221221
let assigned_ty = place.ty(&self.body.local_decls, self.tcx).ty;
222-
// To avoid semver hazard, we only consider `Copy` and `ManuallyDrop` non-dropping.
223-
let manually_drop = assigned_ty
224-
.ty_adt_def()
225-
.map_or(false, |adt_def| adt_def.is_manually_drop());
226-
let nodrop = manually_drop
227-
|| assigned_ty.is_copy_modulo_regions(
228-
self.tcx.at(self.source_info.span),
229-
self.param_env,
222+
if assigned_ty.needs_drop(
223+
self.tcx,
224+
self.tcx.param_env(base_ty.ty_adt_def().unwrap().did()),
225+
) {
226+
// This would be unsafe, but should be outright impossible since we reject such unions.
227+
self.tcx.sess.delay_span_bug(
228+
self.source_info.span,
229+
"union fields that need dropping should be impossible",
230230
);
231-
if !nodrop {
232-
self.require_unsafe(
233-
UnsafetyViolationKind::General,
234-
UnsafetyViolationDetails::AssignToDroppingUnionField,
235-
);
236-
} else {
237-
// write to non-drop union field, safe
238231
}
239232
} else {
240233
self.require_unsafe(

‎src/test/ui/union/union-unsafe.mir.stderr

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,6 @@ LL | *(u.p) = 13;
66
|
77
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
88

9-
error[E0133]: assignment to union field that might need dropping is unsafe and requires unsafe function or block
10-
--> $DIR/union-unsafe.rs:38:5
11-
|
12-
LL | u.a = (ManuallyDrop::new(RefCell::new(0)), 1);
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to union field that might need dropping
14-
|
15-
= note: the previous content of the field will be dropped, which causes undefined behavior if the field was not properly initialized
16-
179
error[E0133]: access to union field is unsafe and requires unsafe function or block
1810
--> $DIR/union-unsafe.rs:46:6
1911
|
@@ -78,6 +70,6 @@ LL | *u3.a = String::from("new");
7870
|
7971
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
8072

81-
error: aborting due to 10 previous errors
73+
error: aborting due to 9 previous errors
8274

8375
For more information about this error, try `rustc --explain E0133`.

‎src/test/ui/union/union-unsafe.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ fn deref_union_field(mut u: URef) {
3535

3636
fn assign_noncopy_union_field(mut u: URefCell) {
3737
// FIXME(thir-unsafeck)
38-
u.a = (ManuallyDrop::new(RefCell::new(0)), 1); //~ ERROR assignment to union field
38+
u.a = (ManuallyDrop::new(RefCell::new(0)), 1); // OK (assignment does not drop)
3939
u.a.0 = ManuallyDrop::new(RefCell::new(0)); // OK (assignment does not drop)
4040
u.a.1 = 1; // OK
4141
}

‎src/test/ui/union/union-unsafe.thir.stderr

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,6 @@ LL | *(u.p) = 13;
66
|
77
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
88

9-
error[E0133]: assignment to union field that might need dropping is unsafe and requires unsafe function or block
10-
--> $DIR/union-unsafe.rs:38:5
11-
|
12-
LL | u.a = (ManuallyDrop::new(RefCell::new(0)), 1);
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to union field that might need dropping
14-
|
15-
= note: the previous content of the field will be dropped, which causes undefined behavior if the field was not properly initialized
16-
179
error[E0133]: access to union field is unsafe and requires unsafe function or block
1810
--> $DIR/union-unsafe.rs:46:6
1911
|
@@ -78,6 +70,6 @@ LL | *u3.a = String::from("new");
7870
|
7971
= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
8072

81-
error: aborting due to 10 previous errors
73+
error: aborting due to 9 previous errors
8274

8375
For more information about this error, try `rustc --explain E0133`.

0 commit comments

Comments
 (0)