Change untagged_unions to not allow union fields with drop

src/doc/rustc/src/lints/listing/warn-by-default.md

@@ -596,30 +596,6 @@ warning: function cannot return without recursing
   |
 ```
 
-## unions-with-drop-fields
-
-This lint detects use of unions that contain fields with possibly non-trivial drop code. Some
-example code that triggers this lint:
-
-```rust
-#![feature(untagged_unions)]
-
-union U {
-    s: String,
-}
-```
-
-This will produce:
-
-```text
-warning: union contains a field with possibly non-trivial drop code, drop code of union fields is ignored when dropping the union
- --> src/main.rs:4:5
-  |
-4 |     s: String,
-  |     ^^^^^^^^^
-  |
-```
-
 ## unknown-lints
 
 This lint detects unrecognized lint attribute. Some

src/librustc/Cargo.toml

@@ -35,5 +35,5 @@ parking_lot = "0.9"
 byteorder = { version = "1.3" }
 chalk-engine = { version = "0.9.0", default-features=false }
 rustc_fs_util = { path = "../librustc_fs_util" }
-smallvec = { version = "0.6.7", features = ["union", "may_dangle"] }
+smallvec = { version = "0.6.8", features = ["union", "may_dangle"] }
 measureme = "0.3"

src/librustc/ty/util.rs

@@ -805,6 +805,8 @@ impl<'tcx> ty::TyS<'tcx> {
     ///
     /// (Note that this implies that if `ty` has a destructor attached,
     /// then `needs_drop` will definitely return `true` for `ty`.)
+    ///
+    /// Note that this method is used to check eligible types in unions.
     #[inline]
     pub fn needs_drop(&'tcx self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool {
         tcx.needs_drop_raw(param_env.and(self)).0

src/librustc_lint/builtin.rs

@@ -979,35 +979,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnstableFeatures {
     }
 }
 
-declare_lint! {
-    UNIONS_WITH_DROP_FIELDS,
-    Warn,
-    "use of unions that contain fields with possibly non-trivial drop code"
-}
-
-declare_lint_pass!(
-    /// Lint for unions that contain fields with possibly non-trivial destructors.
-    UnionsWithDropFields => [UNIONS_WITH_DROP_FIELDS]
-);
-
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnionsWithDropFields {
-    fn check_item(&mut self, ctx: &LateContext<'_, '_>, item: &hir::Item) {
-        if let hir::ItemKind::Union(ref vdata, _) = item.kind {
-            for field in vdata.fields() {
-                let field_ty = ctx.tcx.type_of(
-                    ctx.tcx.hir().local_def_id(field.hir_id));
-                if field_ty.needs_drop(ctx.tcx, ctx.param_env) {
-                    ctx.span_lint(UNIONS_WITH_DROP_FIELDS,
-                                  field.span,
-                                  "union contains a field with possibly non-trivial drop code, \
-                                   drop code of union fields is ignored when dropping the union");
-                    return;
-                }
-            }
-        }
-    }
-}
-
 declare_lint! {
     pub UNREACHABLE_PUB,
     Allow,
@@ -1287,7 +1258,6 @@ declare_lint_pass!(
         NO_MANGLE_GENERIC_ITEMS,
         MUTABLE_TRANSMUTES,
         UNSTABLE_FEATURES,
-        UNIONS_WITH_DROP_FIELDS,
         UNREACHABLE_PUB,
         TYPE_ALIAS_BOUNDS,
         TRIVIAL_BOUNDS

src/librustc_lint/lib.rs

@@ -164,9 +164,6 @@ macro_rules! late_lint_mod_passes {
             // Depends on referenced function signatures in expressions
             MutableTransmutes: MutableTransmutes,
 
-            // Depends on types of fields, checks if they implement Drop
-            UnionsWithDropFields: UnionsWithDropFields,
-
             TypeAliasBounds: TypeAliasBounds,
 
             TrivialConstraints: TrivialConstraints,

src/librustc_typeck/check/mod.rs

@@ -1387,9 +1387,41 @@ fn check_union(tcx: TyCtxt<'_>, id: hir::HirId, span: Span) {
     def.destructor(tcx); // force the destructor to be evaluated
     check_representable(tcx, span, def_id);
     check_transparent(tcx, span, def_id);
+    check_union_fields(tcx, span, def_id);
     check_packed(tcx, span, def_id);
 }
 
+/// When the `#![feature(untagged_unions)]` gate is active,
+/// check that the fields of the `union` does not contain fields that need dropping.
+fn check_union_fields(tcx: TyCtxt<'_>, _: Span, item_def_id: DefId) -> bool {
+    // Without the feature we check that all fields are `Copy` in our stability checking
+    // infrastructure.
+    if !tcx.features().untagged_unions {
+        return true;
+    }
+    let item_type = tcx.type_of(item_def_id);
+    if let ty::Adt(def, substs) = item_type.kind {
+        if def.is_union() {
+            let fields = &def.non_enum_variant().fields;
+            for field in fields {
+                let field_ty = field.ty(tcx, substs);
+                // We are currently checking the type this field came from, so it must be local.
+                let field_span = tcx.hir().span_if_local(field.did).unwrap();
+                let param_env = tcx.param_env(field.did);
+                if field_ty.needs_drop(tcx, param_env) {
+                    struct_span_err!(tcx.sess, field_span, E0740,
+                                     "unions may not contain fields that need dropping")
+                                .span_note(field_span,
+                                           "`std::mem::ManuallyDrop` can be used to wrap the type")
+                                .emit();
+                    return false;
+                }
+            }
+        }
+    }
+    return true;
+}
+
 /// Checks that an opaque type does not contain cycles and does not use `Self` or `T::Foo`
 /// projections that would result in "inheriting lifetimes".
 fn check_opaque<'tcx>(

src/librustc_typeck/error_codes.rs

@@ -4863,6 +4863,10 @@ assert_eq!(1, discriminant(&Enum::Struct{a: 7, b: 11}));
 ```
 "##,
 
+E0740: r##"
+A `union` cannot have fields with destructors.
+"##,
+
 E0733: r##"
 Recursion in an `async fn` requires boxing. For example, this will not compile:
 

src/libstd/lib.rs

@@ -275,6 +275,7 @@
 #![feature(link_args)]
 #![feature(linkage)]
 #![feature(log_syntax)]
+#![feature(manually_drop_take)]
 #![feature(maybe_uninit_ref)]
 #![feature(maybe_uninit_slice)]
 #![feature(mem_take)]

src/libstd/panicking.rs

@@ -12,8 +12,7 @@ use core::panic::{BoxMeUp, PanicInfo, Location};
 use crate::any::Any;
 use crate::fmt;
 use crate::intrinsics;
-use crate::mem;
-use crate::ptr;
+use crate::mem::{self, ManuallyDrop};
 use crate::raw;
 use crate::sync::atomic::{AtomicBool, Ordering};
 use crate::sys::stdio::panic_output;
@@ -227,10 +226,9 @@ pub use realstd::rt::update_panic_count;
 
 /// Invoke a closure, capturing the cause of an unwinding panic if one occurs.
 pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>> {
-    #[allow(unions_with_drop_fields)]
     union Data<F, R> {
-        f: F,
-        r: R,
+        f: ManuallyDrop<F>,
+        r: ManuallyDrop<R>,
     }
 
     // We do some sketchy operations with ownership here for the sake of
@@ -261,7 +259,7 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
     let mut any_data = 0;
     let mut any_vtable = 0;
     let mut data = Data {
-        f,
+        f: ManuallyDrop::new(f)
     };
 
     let r = __rust_maybe_catch_panic(do_call::<F, R>,
@@ -271,7 +269,7 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
 
     return if r == 0 {
         debug_assert!(update_panic_count(0) == 0);
-        Ok(data.r)
+        Ok(ManuallyDrop::into_inner(data.r))
     } else {
         update_panic_count(-1);
         debug_assert!(update_panic_count(0) == 0);
@@ -284,8 +282,9 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
     fn do_call<F: FnOnce() -> R, R>(data: *mut u8) {
         unsafe {
             let data = data as *mut Data<F, R>;
-            let f = ptr::read(&mut (*data).f);
-            ptr::write(&mut (*data).r, f());
+            let data = &mut (*data);
+            let f = ManuallyDrop::take(&mut data.f);
+            data.r = ManuallyDrop::new(f());
         }
     }
 }

src/test/ui/associated-type-bounds/union-bounds.rs

@@ -3,13 +3,13 @@
 #![feature(associated_type_bounds)]
 #![feature(untagged_unions)]
 
-#![allow(unions_with_drop_fields, unused_assignments)]
+#![allow(unused_assignments)]
 
-trait Tr1 { type As1; }
-trait Tr2 { type As2; }
-trait Tr3 { type As3; }
-trait Tr4<'a> { type As4; }
-trait Tr5 { type As5; }
+trait Tr1: Copy { type As1: Copy; }
+trait Tr2: Copy { type As2: Copy; }
+trait Tr3: Copy { type As3: Copy; }
+trait Tr4<'a>: Copy { type As4: Copy; }
+trait Tr5: Copy { type As5: Copy; }
 
 impl Tr1 for &str { type As1 = bool; }
 impl Tr2 for bool { type As2 = u8; }
@@ -71,7 +71,8 @@ where
     let _: &'a T = &x.f0;
 }
 
-union UnSelf<T> where Self: Tr1<As1: Tr2> {
+#[derive(Copy, Clone)]
+union UnSelf<T> where Self: Tr1<As1: Tr2>, T: Copy {
     f0: T,
     f1: <Self as Tr1>::As1,
     f2: <<Self as Tr1>::As1 as Tr2>::As2,

src/test/ui/drop/dynamic-drop.rs

@@ -8,6 +8,7 @@
 #![feature(slice_patterns)]
 
 use std::cell::{Cell, RefCell};
+use std::mem::ManuallyDrop;
 use std::ops::Generator;
 use std::panic;
 use std::pin::Pin;
@@ -152,17 +153,16 @@ fn assignment1(a: &Allocator, c0: bool) {
     _v = _w;
 }
 
-#[allow(unions_with_drop_fields)]
 union Boxy<T> {
-    a: T,
-    b: T,
+    a: ManuallyDrop<T>,
+    b: ManuallyDrop<T>,
 }
 
 fn union1(a: &Allocator) {
     unsafe {
-        let mut u = Boxy { a: a.alloc() };
-        u.b = a.alloc();
-        drop(u.a);
+        let mut u = Boxy { a: ManuallyDrop::new(a.alloc()) };
+        *u.b = a.alloc(); // drops first alloc
+        drop(ManuallyDrop::into_inner(u.a));
     }
 }
 

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

@@ -1,7 +1,7 @@
 #![feature(untagged_unions)]
 
-trait Tr1 { type As1; }
-trait Tr2 { type As2; }
+trait Tr1 { type As1: Copy; }
+trait Tr2 { type As2: Copy; }
 
 struct S1;
 #[derive(Copy, Clone)]
@@ -32,7 +32,7 @@ enum _En1<T: Tr1<As1: Tr2>> {
 
 union _Un1<T: Tr1<As1: Tr2>> {
 //~^ ERROR associated type bounds are unstable
-    outest: T,
+    outest: std::mem::ManuallyDrop<T>,
     outer: T::As1,
     inner: <T::As1 as Tr2>::As2,
 }

src/test/ui/rfc-2093-infer-outlives/explicit-union.rs

@@ -1,13 +1,12 @@
 #![feature(rustc_attrs)]
 #![feature(untagged_unions)]
-#![allow(unions_with_drop_fields)]
 
 #[rustc_outlives]
-union Foo<'b, U> { //~ ERROR rustc_outlives
+union Foo<'b, U: Copy> { //~ ERROR rustc_outlives
     bar: Bar<'b, U>
 }
 
-union Bar<'a, T> where T: 'a {
+union Bar<'a, T: Copy> where T: 'a {
     x: &'a (),
     y: T,
 }

src/test/ui/rfc-2093-infer-outlives/explicit-union.stderr

@@ -1,7 +1,7 @@
 error: rustc_outlives
-  --> $DIR/explicit-union.rs:6:1
+  --> $DIR/explicit-union.rs:5:1
    |
-LL | / union Foo<'b, U> {
+LL | / union Foo<'b, U: Copy> {
 LL | |     bar: Bar<'b, U>
 LL | | }
    | |_^

src/test/ui/rfc-2093-infer-outlives/nested-union.rs

@@ -1,14 +1,13 @@
 #![feature(rustc_attrs)]
 #![feature(untagged_unions)]
-#![allow(unions_with_drop_fields)]
 
 #[rustc_outlives]
-union Foo<'a, T> { //~ ERROR rustc_outlives
+union Foo<'a, T: Copy> { //~ ERROR rustc_outlives
     field1: Bar<'a, T>
 }
 
 // Type U needs to outlive lifetime 'b
-union Bar<'b, U> {
+union Bar<'b, U: Copy> {
     field2: &'b U
 }
 

src/test/ui/rfc-2093-infer-outlives/nested-union.stderr

@@ -1,7 +1,7 @@
 error: rustc_outlives
-  --> $DIR/nested-union.rs:6:1
+  --> $DIR/nested-union.rs:5:1
    |
-LL | / union Foo<'a, T> {
+LL | / union Foo<'a, T: Copy> {
 LL | |     field1: Bar<'a, T>
 LL | | }
    | |_^

src/test/ui/self/self-in-typedefs.rs

@@ -3,7 +3,8 @@
 #![feature(untagged_unions)]
 
 #![allow(dead_code)]
-#![allow(unions_with_drop_fields)]
+
+use std::mem::ManuallyDrop;
 
 enum A<'a, T: 'a>
 where
@@ -24,6 +25,14 @@ where
 union C<'a, T: 'a>
 where
     Self: Send, T: PartialEq<Self>
+{
+    foo: &'a Self,
+    bar: ManuallyDrop<T>,
+}
+
+union D<'a, T: 'a>
+where
+    Self: Send, T: PartialEq<Self> + Copy
 {
     foo: &'a Self,
     bar: T,

src/test/ui/union/issue-41073.rs

@@ -0,0 +1,24 @@
+#![feature(untagged_unions)]
+
+union Test {
+    a: A, //~ ERROR unions may not contain fields that need dropping
+    b: B
+}
+
+#[derive(Debug)]
+struct A(i32);
+impl Drop for A {
+    fn drop(&mut self) { println!("A"); }
+}
+
+#[derive(Debug)]
+struct B(f32);
+impl Drop for B {
+    fn drop(&mut self) { println!("B"); }
+}
+
+fn main() {
+    let mut test = Test { a: A(3) };
+    println!("{:?}", unsafe { test.b });
+    unsafe { test.b = B(0.5); }
+}