assigning to a union field can never drop now

RalfJung · RalfJung · commit 1cf6f5c01d83 · 2022-07-11T09:26:20.000-04:00
diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs
@@ -35,7 +35,6 @@ pub enum UnsafetyViolationDetails {
     UseOfMutableStatic,
     UseOfExternStatic,
     DerefOfRawPointer,
-    AssignToDroppingUnionField,
     AccessToUnionField,
     MutationOfLayoutConstrainedField,
     BorrowOfLayoutConstrainedField,
@@ -78,11 +77,6 @@ impl UnsafetyViolationDetails {
                 "raw pointers may be null, dangling or unaligned; they can violate aliasing rules \
                  and cause data races: all of these are undefined behavior",
             ),
-            AssignToDroppingUnionField => (
-                "assignment to union field that might need dropping",
-                "the previous content of the field will be dropped, which causes undefined \
-                 behavior if the field was not properly initialized",
-            ),
             AccessToUnionField => (
                 "access to union field",
                 "the field may not be properly initialized: using uninitialized data will cause \
diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs
@@ -431,16 +431,9 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                 let lhs = &self.thir[lhs];
                 if let ty::Adt(adt_def, _) = lhs.ty.kind() && adt_def.is_union() {
                     if let Some((assigned_ty, assignment_span)) = self.assignment_info {
-                        // To avoid semver hazard, we only consider `Copy` and `ManuallyDrop` non-dropping.
-                        if !(assigned_ty
-                            .ty_adt_def()
-                            .map_or(false, |adt| adt.is_manually_drop())
-                            || assigned_ty
-                                .is_copy_modulo_regions(self.tcx.at(expr.span), self.param_env))
-                        {
-                            self.requires_unsafe(assignment_span, AssignToDroppingUnionField);
-                        } else {
-                            // write to non-drop union field, safe
+                        if assigned_ty.needs_drop(self.tcx, self.tcx.param_env(adt_def.did())) {
+                            // This would be unsafe, but should be outright impossible since we reject such unions.
+                            self.tcx.sess.delay_span_bug(assignment_span, "union fields that need dropping should be impossible");
                         }
                     } else {
                         self.requires_unsafe(expr.span, AccessToUnionField);
@@ -537,7 +530,6 @@ enum UnsafeOpKind {
     UseOfMutableStatic,
     UseOfExternStatic,
     DerefOfRawPointer,
-    AssignToDroppingUnionField,
     AccessToUnionField,
     MutationOfLayoutConstrainedField,
     BorrowOfLayoutConstrainedField,
@@ -555,7 +547,6 @@ impl UnsafeOpKind {
             UseOfMutableStatic => "use of mutable static",
             UseOfExternStatic => "use of extern static",
             DerefOfRawPointer => "dereference of raw pointer",
-            AssignToDroppingUnionField => "assignment to union field that might need dropping",
             AccessToUnionField => "access to union field",
             MutationOfLayoutConstrainedField => "mutation of layout constrained field",
             BorrowOfLayoutConstrainedField => {
@@ -600,11 +591,6 @@ impl UnsafeOpKind {
                 "raw pointers may be null, dangling or unaligned; they can violate aliasing rules \
                  and cause data races: all of these are undefined behavior",
             ),
-            AssignToDroppingUnionField => (
-                Cow::Borrowed(self.simple_description()),
-                "the previous content of the field will be dropped, which causes undefined \
-                 behavior if the field was not properly initialized",
-            ),
             AccessToUnionField => (
                 Cow::Borrowed(self.simple_description()),
                 "the field may not be properly initialized: using uninitialized data will cause \
diff --git a/compiler/rustc_mir_transform/src/check_unsafety.rs b/compiler/rustc_mir_transform/src/check_unsafety.rs
@@ -219,22 +219,15 @@ impl<'tcx> Visitor<'tcx> for UnsafetyChecker<'_, 'tcx> {
                     // We have to check the actual type of the assignment, as that determines if the
                     // old value is being dropped.
                     let assigned_ty = place.ty(&self.body.local_decls, self.tcx).ty;
-                    // To avoid semver hazard, we only consider `Copy` and `ManuallyDrop` non-dropping.
-                    let manually_drop = assigned_ty
-                        .ty_adt_def()
-                        .map_or(false, |adt_def| adt_def.is_manually_drop());
-                    let nodrop = manually_drop
-                        || assigned_ty.is_copy_modulo_regions(
-                            self.tcx.at(self.source_info.span),
-                            self.param_env,
+                    if assigned_ty.needs_drop(
+                        self.tcx,
+                        self.tcx.param_env(base_ty.ty_adt_def().unwrap().did()),
+                    ) {
+                        // This would be unsafe, but should be outright impossible since we reject such unions.
+                        self.tcx.sess.delay_span_bug(
+                            self.source_info.span,
+                            "union fields that need dropping should be impossible",
                         );
-                    if !nodrop {
-                        self.require_unsafe(
-                            UnsafetyViolationKind::General,
-                            UnsafetyViolationDetails::AssignToDroppingUnionField,
-                        );
-                    } else {
-                        // write to non-drop union field, safe
                     }
                 } else {
                     self.require_unsafe(
diff --git a/src/test/ui/union/union-unsafe.mir.stderr b/src/test/ui/union/union-unsafe.mir.stderr
@@ -6,14 +6,6 @@ LL |     *(u.p) = 13;
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
-error[E0133]: assignment to union field that might need dropping is unsafe and requires unsafe function or block
-  --> $DIR/union-unsafe.rs:38:5
-   |
-LL |     u.a = (ManuallyDrop::new(RefCell::new(0)), 1);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to union field that might need dropping
-   |
-   = note: the previous content of the field will be dropped, which causes undefined behavior if the field was not properly initialized
-
 error[E0133]: access to union field is unsafe and requires unsafe function or block
   --> $DIR/union-unsafe.rs:46:6
    |
@@ -78,6 +70,6 @@ LL |     *u3.a = String::from("new");
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
-error: aborting due to 10 previous errors
+error: aborting due to 9 previous errors
 
 For more information about this error, try `rustc --explain E0133`.
diff --git a/src/test/ui/union/union-unsafe.rs b/src/test/ui/union/union-unsafe.rs
@@ -35,7 +35,7 @@ fn deref_union_field(mut u: URef) {
 
 fn assign_noncopy_union_field(mut u: URefCell) {
     // FIXME(thir-unsafeck)
-    u.a = (ManuallyDrop::new(RefCell::new(0)), 1); //~ ERROR assignment to union field
+    u.a = (ManuallyDrop::new(RefCell::new(0)), 1); // OK (assignment does not drop)
     u.a.0 = ManuallyDrop::new(RefCell::new(0)); // OK (assignment does not drop)
     u.a.1 = 1; // OK
 }
diff --git a/src/test/ui/union/union-unsafe.thir.stderr b/src/test/ui/union/union-unsafe.thir.stderr
@@ -6,14 +6,6 @@ LL |     *(u.p) = 13;
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
-error[E0133]: assignment to union field that might need dropping is unsafe and requires unsafe function or block
-  --> $DIR/union-unsafe.rs:38:5
-   |
-LL |     u.a = (ManuallyDrop::new(RefCell::new(0)), 1);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to union field that might need dropping
-   |
-   = note: the previous content of the field will be dropped, which causes undefined behavior if the field was not properly initialized
-
 error[E0133]: access to union field is unsafe and requires unsafe function or block
   --> $DIR/union-unsafe.rs:46:6
    |
@@ -78,6 +70,6 @@ LL |     *u3.a = String::from("new");
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
-error: aborting due to 10 previous errors
+error: aborting due to 9 previous errors
 
 For more information about this error, try `rustc --explain E0133`.

Original file line number	Diff line number	Diff line change
`@@ -6,14 +6,6 @@ LL \| *(u.p) = 13;`
`6`	`6`	`\|`
`7`	`7`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`8`	`8`
`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`		`-`
`17`	`9`	`error[E0133]: access to union field is unsafe and requires unsafe function or block`
`18`	`10`	`--> $DIR/union-unsafe.rs:46:6`
`19`	`11`	`\|`
`@@ -78,6 +70,6 @@ LL \| *u3.a = String::from("new");`
`78`	`70`	`\|`
`79`	`71`	`= note: the field may not be properly initialized: using uninitialized data will cause undefined behavior`
`80`	`72`
`81`		`-error: aborting due to 10 previous errors`
	`73`	`+error: aborting due to 9 previous errors`
`82`	`74`
`83`	`75`	For more information about this error, try `rustc --explain E0133`.
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ fn deref_union_field(mut u: URef) {`
`35`	`35`
`36`	`36`	`fn assign_noncopy_union_field(mut u: URefCell) {`
`37`	`37`	`// 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)`
`39`	`39`	`u.a.0 = ManuallyDrop::new(RefCell::new(0)); // OK (assignment does not drop)`
`40`	`40`	`u.a.1 = 1; // OK`
`41`	`41`	`}`