allow unions with mutable references and tuples of allowed types

Author: RalfJung

compiler/rustc_passes/src/stability.rs

@@ -772,17 +772,27 @@ impl<'tcx> Visitor<'tcx> for Checker<'tcx> {
                 let ty = self.tcx.type_of(item.def_id);
                 let ty::Adt(adt_def, substs) = ty.kind() else { bug!() };
 
-                #[allow(rustc::usage_of_qualified_ty)] // `Ty` is the wrong type here, we really want `ty::Ty`.
+                #[allow(rustc::usage_of_qualified_ty)] // `Ty` is `hir::Ty` here, we really want `ty::Ty`.
                 fn allowed_union_field<'tcx>(
                     tcx: TyCtxt<'tcx>,
                     param_env: ty::ParamEnv<'tcx>,
                     ty: ty::Ty<'tcx>,
                 ) -> bool {
-                    ty.ty_adt_def().map_or(false, |adt_def| adt_def.is_manually_drop())
-                        || ty.is_copy_modulo_regions(tcx.at(DUMMY_SP), param_env)
+                    // We don't just accept all !needs_drop fields, due to semver concerns.
+                    match ty.kind() {
+                        ty::Ref(..) => true, // references never drop (even mutable refs, which are non-Copy and hence fail the later check)
+                        ty::Tuple(tys) => {
+                            // allow tuples of allowed types
+                            tys.iter().all(|ty| allowed_union_field(tcx, param_env, ty))
+                        }
+                        _ => {
+                            ty.ty_adt_def().map_or(false, |adt_def| adt_def.is_manually_drop())
+                                || ty.is_copy_modulo_regions(tcx.at(DUMMY_SP), param_env)
+                        }
+                    }
                 }
 
-                // Non-`Copy` fields are unstable, except for `ManuallyDrop`.
+                // `allowed_union_field` determines which fields are allowed on stable.
                 let param_env = self.tcx.param_env(item.def_id);
                 for field in &adt_def.non_enum_variant().fields {
                     let field_ty = field.ty(self.tcx, substs);
@@ -799,10 +809,17 @@ impl<'tcx> Visitor<'tcx> for Checker<'tcx> {
                                 &self.tcx.sess.parse_sess,
                                 sym::untagged_unions,
                                 self.tcx.def_span(field.did),
-                                "unions with non-`Copy` fields other than `ManuallyDrop<T>` are unstable",
+                                "unions with non-`Copy` fields other than `ManuallyDrop<T>`, \
+                                references, and tuples of such types are unstable",
                             )
                             .emit();
                         }
+                    } else {
+                        // We allow this field. Make extra sure it does not drop.
+                        assert!(
+                            !field_ty.needs_drop(self.tcx, param_env),
+                            "we should accept no maybe-dropping union fields"
+                        );
                     }
                 }
             }

src/test/ui/feature-gates/feature-gate-untagged_unions.rs

@@ -1,4 +1,5 @@
 // ignore-tidy-linelength
+use std::mem::ManuallyDrop;
 
 union U1 { // OK
     a: u8,
@@ -9,15 +10,19 @@ union U2<T: Copy> { // OK
 }
 
 union U22<T> { // OK
-    a: std::mem::ManuallyDrop<T>,
+    a: ManuallyDrop<T>,
+}
+
+union U23<T> { // OK
+    a: (ManuallyDrop<T>, i32),
 }
 
 union U3 {
     a: String, //~ ERROR unions cannot contain fields that may need dropping
 }
 
 union U32 { // field that does not drop but is not `Copy`, either -- this is the real feature gate test!
-    a: std::cell::RefCell<i32>, //~ ERROR unions with non-`Copy` fields other than `ManuallyDrop<T>` are unstable
+    a: std::cell::RefCell<i32>, //~ ERROR unions with non-`Copy` fields other than `ManuallyDrop<T>`, references, and tuples of such types are unstable
 }
 
 union U4<T> {
@@ -32,4 +37,17 @@ impl Drop for U5 {
     fn drop(&mut self) {}
 }
 
+union U5Nested { // a nested union that drops is NOT OK
+    nest: U5, //~ ERROR unions cannot contain fields that may need dropping
+}
+
+union U6 { // OK
+    s: &'static i32,
+    m: &'static mut i32,
+}
+
+union U7<T> { // OK
+    f: (&'static mut i32, ManuallyDrop<T>, i32),
+}
+
 fn main() {}

src/test/ui/feature-gates/feature-gate-untagged_unions.stderr

@@ -1,5 +1,5 @@
-error[E0658]: unions with non-`Copy` fields other than `ManuallyDrop<T>` are unstable
-  --> $DIR/feature-gate-untagged_unions.rs:20:5
+error[E0658]: unions with non-`Copy` fields other than `ManuallyDrop<T>`, references, and tuples of such types are unstable
+  --> $DIR/feature-gate-untagged_unions.rs:25:5
    |
 LL |     a: std::cell::RefCell<i32>,
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -8,7 +8,7 @@ LL |     a: std::cell::RefCell<i32>,
    = help: add `#![feature(untagged_unions)]` to the crate attributes to enable
 
 error[E0740]: unions cannot contain fields that may need dropping
-  --> $DIR/feature-gate-untagged_unions.rs:16:5
+  --> $DIR/feature-gate-untagged_unions.rs:21:5
    |
 LL |     a: String,
    |     ^^^^^^^^^
@@ -20,7 +20,7 @@ LL |     a: std::mem::ManuallyDrop<String>,
    |        +++++++++++++++++++++++      +
 
 error[E0740]: unions cannot contain fields that may need dropping
-  --> $DIR/feature-gate-untagged_unions.rs:24:5
+  --> $DIR/feature-gate-untagged_unions.rs:29:5
    |
 LL |     a: T,
    |     ^^^^
@@ -31,7 +31,19 @@ help: when the type does not implement `Copy`, wrap it inside a `ManuallyDrop<_>
 LL |     a: std::mem::ManuallyDrop<T>,
    |        +++++++++++++++++++++++ +
 
-error: aborting due to 3 previous errors
+error[E0740]: unions cannot contain fields that may need dropping
+  --> $DIR/feature-gate-untagged_unions.rs:41:5
+   |
+LL |     nest: U5,
+   |     ^^^^^^^^
+   |
+   = note: a type is guaranteed not to need dropping when it implements `Copy`, or when it is the special `ManuallyDrop<_>` type
+help: when the type does not implement `Copy`, wrap it inside a `ManuallyDrop<_>` and ensure it is manually dropped
+   |
+LL |     nest: std::mem::ManuallyDrop<U5>,
+   |           +++++++++++++++++++++++  +
+
+error: aborting due to 4 previous errors
 
 Some errors have detailed explanations: E0658, E0740.
 For more information about an error, try `rustc --explain E0658`.