Change untagged_unions to not allow union fields with drop

src/libcore/slice/rotate.rs

@@ -8,7 +8,7 @@ use ptr;
 union RawArray<T> {
     /// Ensure this is appropriately aligned for T, and is big
     /// enough for two elements even if T is enormous.
-    typed: [T; 2],
+    typed: [MaybeUninit<T>; 2],
     /// For normally-sized types, especially things like u8, having more
     /// than 2 in the buffer is necessary for usefulness, so pad it out
     /// enough to be helpful, but not so big as to risk overflow.
@@ -73,7 +73,8 @@ pub unsafe fn ptr_rotate<T>(mut left: usize, mid: *mut T, mut right: usize) {
     }
 
     let mut rawarray = MaybeUninit::<RawArray<T>>::uninitialized();
-    let buf = &mut (*rawarray.as_mut_ptr()).typed as *mut [T; 2] as *mut T;
+    let buf = &mut (*rawarray.as_mut_ptr()).typed as *mut [MaybeUninit<T>; 2]
+              as *mut [T; 2] as *mut T;
 
     let dim = mid.sub(left).add(right);
     if left <= right {

src/librustc/ty/util.rs

@@ -740,6 +740,8 @@ impl<'a, '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<'a, 'tcx, 'tcx>,

src/librustc_typeck/check/mod.rs

@@ -1302,9 +1302,41 @@ fn check_union<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     def.destructor(tcx); // force the destructor to be evaluated
     check_representable(tcx, span, def_id);
 
+    check_union_fields(tcx, span, def_id);
     check_packed(tcx, span, def_id);
 }
 
+fn check_union_fields<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+                                _sp: Span,
+                                item_def_id: DefId)
+                                -> bool {
+    // Without the feature we check Copy types only later
+    if !tcx.features().untagged_unions {
+        return true;
+    }
+    let t = tcx.type_of(item_def_id);
+    if let ty::Adt(def, substs) = t.sty {
+        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, E0730,
+                                     "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;
+}
+
 fn check_opaque<'a, 'tcx>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     def_id: DefId,

src/librustc_typeck/diagnostics.rs

@@ -4831,6 +4831,10 @@ fn make_recursive_type() -> impl Sized {
 ```
 "##,
 
+E0730: r##"
+A `union` can not have fields with destructors.
+"##,
+
 }
 
 register_diagnostics! {

src/libstd/panicking.rs

@@ -16,7 +16,7 @@ use cell::RefCell;
 use core::panic::{PanicInfo, Location};
 use fmt;
 use intrinsics;
-use mem;
+use mem::{self, ManuallyDrop};
 use ptr;
 use raw;
 use sys::stdio::panic_output;
@@ -238,8 +238,8 @@ pub use realstd::rt::update_panic_count;
 pub unsafe fn 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
@@ -270,7 +270,7 @@ pub unsafe fn 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>,
@@ -280,7 +280,7 @@ pub unsafe fn 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);
@@ -293,8 +293,8 @@ pub unsafe fn 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 f = ptr::read(&mut *(*data).f);
+            ptr::write(&mut *(*data).r, f());
         }
     }
 }

src/test/run-pass/drop/dynamic-drop.rs

@@ -10,6 +10,7 @@
 #![feature(slice_patterns)]
 
 use std::cell::{Cell, RefCell};
+use std::mem::ManuallyDrop;
 use std::ops::Generator;
 use std::panic;
 use std::usize;
@@ -139,17 +140,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/run-pass/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/run-pass/union/union-derive.rs

@@ -24,11 +24,11 @@ impl PartialEq for U { fn eq(&self, rhs: &Self) -> bool { true } }
     Copy,
     Eq
 )]
-union W<T> {
+union W<T: Copy> {
     a: T,
 }
 
-impl<T> PartialEq for W<T> { fn eq(&self, rhs: &Self) -> bool { true } }
+impl<T: Copy> PartialEq for W<T> { fn eq(&self, rhs: &Self) -> bool { true } }
 
 fn main() {
     let u = U { b: 0 };

src/test/run-pass/union/union-drop-assign.rs

@@ -6,10 +6,12 @@
 
 #![feature(untagged_unions)]
 
+use std::mem::ManuallyDrop;
+
 struct S;
 
 union U {
-    a: S
+    a: ManuallyDrop<S>
 }
 
 impl Drop for S {
@@ -28,11 +30,11 @@ static mut CHECK: u8 = 0;
 
 fn main() {
     unsafe {
-        let mut u = U { a: S };
+        let mut u = U { a: ManuallyDrop::new(S) };
         assert_eq!(CHECK, 0);
-        u = U { a: S };
+        u = U { a: ManuallyDrop::new(S) };
         assert_eq!(CHECK, 1); // union itself is assigned, union is dropped, field is not dropped
-        u.a = S;
+        *u.a = S;
         assert_eq!(CHECK, 11); // union field is assigned, field is dropped
     }
 }

src/test/run-pass/union/union-drop.rs

@@ -21,12 +21,6 @@ union Y {
     a: S,
 }
 
-impl Drop for S {
-    fn drop(&mut self) {
-        unsafe { CHECK += 10; }
-    }
-}
-
 impl Drop for U {
     fn drop(&mut self) {
         unsafe { CHECK += 1; }
@@ -51,10 +45,10 @@ fn main() {
         {
             let w = W { a: S };
         }
-        assert_eq!(CHECK, 2); // 2, not 11, dtor of S is not called
+        assert_eq!(CHECK, 2); // 2, dtor of W is called
         {
             let y = Y { a: S };
         }
-        assert_eq!(CHECK, 2); // 2, not 12, dtor of S is not called
+        assert_eq!(CHECK, 2); // 2, dtor of Y is called
     }
 }

src/test/run-pass/union/union-generic.rs

@@ -1,30 +1,24 @@
 // run-pass
 #![allow(dead_code)]
-#![allow(unions_with_drop_fields)]
 
 #![feature(untagged_unions)]
 
-union MaybeItem<T: Iterator> {
+union MaybeItem<T: Iterator> where T::Item: Copy {
     elem: T::Item,
     none: (),
 }
 
-union U<A, B> {
+union U<A, B> where A: Copy, B: Copy {
     a: A,
     b: B,
 }
 
-unsafe fn union_transmute<A, B>(a: A) -> B {
+unsafe fn union_transmute<A, B>(a: A) -> B where A: Copy, B: Copy {
     U { a: a }.b
 }
 
 fn main() {
     unsafe {
-        let u = U::<String, Vec<u8>> { a: String::from("abcd") };
-
-        assert_eq!(u.b.len(), 4);
-        assert_eq!(u.b[0], b'a');
-
         let b = union_transmute::<(u8, u8), u16>((1, 1));
         assert_eq!(b, (1 << 8) + 1);
 

src/test/run-pass/union/union-manuallydrop.rs

@@ -0,0 +1,42 @@
+#![feature(untagged_unions)]
+#![allow(dead_code)]
+// run-pass
+
+use std::mem::needs_drop;
+use std::mem::ManuallyDrop;
+
+struct NeedDrop;
+
+impl Drop for NeedDrop {
+    fn drop(&mut self) {}
+}
+
+union UnionOk1<T> {
+    empty: (),
+    value: ManuallyDrop<T>,
+}
+
+union UnionOk2 {
+    value: ManuallyDrop<NeedDrop>,
+}
+
+#[allow(dead_code)]
+union UnionOk3<T: Copy> {
+    empty: (),
+    value: T,
+}
+
+trait Foo { }
+
+trait ImpliesCopy : Copy { }
+
+#[allow(dead_code)]
+union UnionOk4<T: ImpliesCopy> {
+    value: T,
+}
+
+fn main() {
+    // NeedDrop should not make needs_drop true
+    assert!(!needs_drop::<UnionOk1<NeedDrop>>());
+    assert!(!needs_drop::<UnionOk3<&Foo>>());
+}

src/test/run-pass/union/union-nodrop.rs

@@ -1,12 +1,11 @@
 // run-pass
 
-#![feature(core_intrinsics)]
 #![feature(untagged_unions)]
 
-#![allow(unions_with_drop_fields)]
 #![allow(dead_code)]
 
-use std::intrinsics::needs_drop;
+use std::mem::needs_drop;
+use std::mem::ManuallyDrop;
 
 struct NeedDrop;
 
@@ -16,10 +15,10 @@ impl Drop for NeedDrop {
 
 // Constant expressios allow `NoDrop` to go out of scope,
 // unlike a value of the interior type implementing `Drop`.
-static X: () = (NoDrop { inner: NeedDrop }, ()).1;
+static X: () = (NoDrop { inner: ManuallyDrop::new(NeedDrop) }, ()).1;
 
 // A union that scrubs the drop glue from its inner type
-union NoDrop<T> {inner: T}
+union NoDrop<T> { inner: ManuallyDrop<T> }
 
 // Copy currently can't be implemented on drop-containing unions,
 // this may change later
@@ -40,7 +39,7 @@ struct Baz {
     y: Box<u8>,
 }
 
-union ActuallyDrop<T> {inner: T}
+union ActuallyDrop<T> { inner: ManuallyDrop<T> }
 
 impl<T> Drop for ActuallyDrop<T> {
     fn drop(&mut self) {}