about summary refs log tree commit diff
diff options
context:
space:
mode:
authorRalf Jung <post@ralfj.de>2022-06-30 08:40:06 -0400
committerRalf Jung <post@ralfj.de>2022-07-13 18:27:28 -0400
commite4593ef0f214b7259e5f5706b2952610ae1fb5ef (patch)
treeaed045717cc3469edd4f7a96c1ab2ba93c15fbef
parent5bf6017b872bec43db353017f7915d3145ce5f5d (diff)
downloadrust-e4593ef0f214b7259e5f5706b2952610ae1fb5ef.tar.gz
rust-e4593ef0f214b7259e5f5706b2952610ae1fb5ef.zip
assigning to a union field can never drop now
-rw-r--r--compiler/rustc_middle/src/mir/query.rs6
-rw-r--r--compiler/rustc_mir_build/src/check_unsafety.rs20
-rw-r--r--compiler/rustc_mir_transform/src/check_unsafety.rs23
-rw-r--r--src/test/ui/union/union-unsafe.mir.stderr10
-rw-r--r--src/test/ui/union/union-unsafe.rs2
-rw-r--r--src/test/ui/union/union-unsafe.thir.stderr10
6 files changed, 14 insertions, 57 deletions
diff --git a/compiler/rustc_middle/src/mir/query.rs b/compiler/rustc_middle/src/mir/query.rs
index 620f0380d53..6a6ed3dc728 100644
--- 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
index 8585199faaf..54d3b7cdda6 100644
--- 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
index 1f73b7da815..ded1f0462cb 100644
--- 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
index 0aefa21c944..544213dbc55 100644
--- 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
index 7e9d9052a67..5e1837a901d 100644
--- 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
index dbfd92f05d9..f959fe5bdb5 100644
--- 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`.