consider assignments of union field of ManuallyDrop type safe

Author: RalfJung

compiler/rustc_middle/src/mir/query.rs

@@ -46,7 +46,7 @@ pub enum UnsafetyViolationDetails {
     UseOfMutableStatic,
     UseOfExternStatic,
     DerefOfRawPointer,
-    AssignToNonCopyUnionField,
+    AssignToDroppingUnionField,
     AccessToUnionField,
     MutationOfLayoutConstrainedField,
     BorrowOfLayoutConstrainedField,
@@ -94,8 +94,8 @@ 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",
             ),
-            AssignToNonCopyUnionField => (
-                "assignment to non-`Copy` union field",
+            AssignToDroppingUnionField => (
+                "assignment to union field that needs dropping",
                 "the previous content of the field will be dropped, which causes undefined \
                  behavior if the field was not properly initialized",
             ),

compiler/rustc_mir/src/transform/check_unsafety.rs

@@ -235,37 +235,40 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
                     UnsafetyViolationKind::GeneralAndConstFn,
                     UnsafetyViolationDetails::DerefOfRawPointer,
                 ),
-                ty::Adt(adt, _) => {
-                    if adt.is_union() {
-                        if context == PlaceContext::MutatingUse(MutatingUseContext::Store)
-                            || context == PlaceContext::MutatingUse(MutatingUseContext::Drop)
-                            || context == PlaceContext::MutatingUse(MutatingUseContext::AsmOutput)
-                        {
-                            let elem_ty = match elem {
-                                ProjectionElem::Field(_, ty) => ty,
-                                _ => span_bug!(
-                                    self.source_info.span,
-                                    "non-field projection {:?} from union?",
-                                    place
-                                ),
-                            };
-                            if !elem_ty.is_copy_modulo_regions(
+                ty::Adt(adt, _) if adt.is_union() => {
+                    if context == PlaceContext::MutatingUse(MutatingUseContext::Store)
+                        || context == PlaceContext::MutatingUse(MutatingUseContext::Drop)
+                        || context == PlaceContext::MutatingUse(MutatingUseContext::AsmOutput)
+                    {
+                        let elem_ty = match elem {
+                            ProjectionElem::Field(_, ty) => ty,
+                            _ => span_bug!(
+                                self.source_info.span,
+                                "non-field projection {:?} from union?",
+                                place
+                            ),
+                        };
+                        let manually_drop = elem_ty
+                            .ty_adt_def()
+                            .map_or(false, |adt_def| adt_def.is_manually_drop());
+                        let nodrop = manually_drop
+                            || elem_ty.is_copy_modulo_regions(
                                 self.tcx.at(self.source_info.span),
                                 self.param_env,
-                            ) {
-                                self.require_unsafe(
-                                    UnsafetyViolationKind::GeneralAndConstFn,
-                                    UnsafetyViolationDetails::AssignToNonCopyUnionField,
-                                )
-                            } else {
-                                // write to non-move union, safe
-                            }
-                        } else {
+                            );
+                        if !nodrop {
                             self.require_unsafe(
                                 UnsafetyViolationKind::GeneralAndConstFn,
-                                UnsafetyViolationDetails::AccessToUnionField,
+                                UnsafetyViolationDetails::AssignToDroppingUnionField,
                             )
+                        } else {
+                            // write to non-drop union field, safe
                         }
+                    } else {
+                        self.require_unsafe(
+                            UnsafetyViolationKind::GeneralAndConstFn,
+                            UnsafetyViolationDetails::AccessToUnionField,
+                        )
                     }
                 }
                 _ => {}

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

@@ -18,7 +18,7 @@ union U4<T: Copy> {
 
 fn generic_noncopy<T: Default>() {
     let mut u3 = U3 { a: ManuallyDrop::new(T::default()) };
-    u3.a = ManuallyDrop::new(T::default()); //~ ERROR assignment to non-`Copy` union field is unsafe
+    u3.a = ManuallyDrop::new(T::default()); // OK (assignment does not drop)
     *u3.a = T::default(); //~ ERROR access to union field is unsafe
 }
 
@@ -41,14 +41,14 @@ fn main() {
     // let U1 { .. } = u1; // OK
 
     let mut u2 = U2 { a: ManuallyDrop::new(String::from("old")) }; // OK
-    u2.a = ManuallyDrop::new(String::from("new")); //~ ERROR assignment to non-`Copy` union
+    u2.a = ManuallyDrop::new(String::from("new")); // OK (assignment does not drop)
     *u2.a = String::from("new"); //~ ERROR access to union field is unsafe
 
     let mut u3 = U3 { a: ManuallyDrop::new(0) }; // OK
     u3.a = ManuallyDrop::new(1); // OK
     *u3.a = 1; //~ ERROR access to union field is unsafe
 
     let mut u3 = U3 { a: ManuallyDrop::new(String::from("old")) }; // OK
-    u3.a = ManuallyDrop::new(String::from("new")); //~ ERROR assignment to non-`Copy` union
+    u3.a = ManuallyDrop::new(String::from("new")); // OK (assignment does not drop)
     *u3.a = String::from("new"); //~ ERROR access to union field is unsafe
 }

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

@@ -1,11 +1,3 @@
-error[E0133]: assignment to non-`Copy` union field is unsafe and requires unsafe function or block
-  --> $DIR/union-unsafe.rs:21:5
-   |
-LL |     u3.a = ManuallyDrop::new(T::default());
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to non-`Copy` union field
-   |
-   = 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:22:6
    |
@@ -46,14 +38,6 @@ LL |     if let U1 { a: 12 } = u1 {}
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
-error[E0133]: assignment to non-`Copy` union field is unsafe and requires unsafe function or block
-  --> $DIR/union-unsafe.rs:44:5
-   |
-LL |     u2.a = ManuallyDrop::new(String::from("new"));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to non-`Copy` union field
-   |
-   = 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:45:6
    |
@@ -70,14 +54,6 @@ LL |     *u3.a = 1;
    |
    = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior
 
-error[E0133]: assignment to non-`Copy` union field is unsafe and requires unsafe function or block
-  --> $DIR/union-unsafe.rs:52:5
-   |
-LL |     u3.a = ManuallyDrop::new(String::from("new"));
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to non-`Copy` union field
-   |
-   = 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:53:6
    |
@@ -86,6 +62,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 11 previous errors
+error: aborting due to 8 previous errors
 
 For more information about this error, try `rustc --explain E0133`.