Implement drop translation and add lint for unions with drop fields

Fix some typeck bugs blocking drop tests

Author: petrochenkov

src/librustc_lint/builtin.rs

@@ -1164,3 +1164,36 @@ impl LateLintPass for UnstableFeatures {
         }
     }
 }
+
+/// Lint for unions that contain fields with possibly non-trivial destructors.
+pub struct UnionsWithDropFields;
+
+declare_lint! {
+    UNIONS_WITH_DROP_FIELDS,
+    Warn,
+    "use of unions that contain fields with possibly non-trivial drop code"
+}
+
+impl LintPass for UnionsWithDropFields {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(UNIONS_WITH_DROP_FIELDS)
+    }
+}
+
+impl LateLintPass for UnionsWithDropFields {
+    fn check_item(&mut self, ctx: &LateContext, item: &hir::Item) {
+        if let hir::ItemUnion(ref vdata, _) = item.node {
+            let param_env = &ty::ParameterEnvironment::for_item(ctx.tcx, item.id);
+            for field in vdata.fields() {
+                let field_ty = ctx.tcx.node_id_to_type(field.id);
+                if ctx.tcx.type_needs_drop_given_env(field_ty, 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;
+                }
+            }
+        }
+    }
+}

src/librustc_lint/lib.rs

@@ -128,6 +128,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                  InvalidNoMangleItems,
                  PluginAsLibrary,
                  MutableTransmutes,
+                 UnionsWithDropFields,
                  );
 
     add_builtin_with_new!(sess,

src/librustc_trans/glue.rs

@@ -267,7 +267,8 @@ pub fn implement_drop_glue<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
 
 fn trans_struct_drop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                                  t: Ty<'tcx>,
-                                 v0: ValueRef)
+                                 v0: ValueRef,
+                                 shallow_drop: bool)
                                  -> Block<'blk, 'tcx>
 {
     debug!("trans_struct_drop t: {}", t);
@@ -286,7 +287,9 @@ fn trans_struct_drop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 
     // Issue #23611: schedule cleanup of contents, re-inspecting the
     // discriminant (if any) in case of variant swap in drop code.
-    bcx.fcx.schedule_drop_adt_contents(contents_scope, v0, t);
+    if !shallow_drop {
+        bcx.fcx.schedule_drop_adt_contents(contents_scope, v0, t);
+    }
 
     let (sized_args, unsized_args);
     let args: &[ValueRef] = if type_is_sized(tcx, t) {
@@ -470,9 +473,6 @@ fn make_drop_glue<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, v0: ValueRef, g: DropGlueK
                 trans_exchange_free_ty(bcx, llbox, content_ty, DebugLoc::None)
             }
         }
-        ty::TyUnion(..) => {
-            unimplemented_unions!();
-        }
         ty::TyTrait(..) => {
             // No support in vtable for distinguishing destroying with
             // versus without calling Drop::drop. Assert caller is
@@ -491,6 +491,13 @@ fn make_drop_glue<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, v0: ValueRef, g: DropGlueK
                 if def.dtor_kind().is_present() && !skip_dtor => {
             trans_struct_drop(bcx, t, v0)
         }
+        ty::TyUnion(def, _) => {
+            if def.dtor_kind().is_present() && !skip_dtor {
+                trans_struct_drop(bcx, t, v0, true)
+            } else {
+                bcx
+            }
+        }
         _ => {
             if bcx.fcx.type_needs_drop(t) {
                 drop_structural_ty(bcx, v0, t)

src/librustc_typeck/check/mod.rs

@@ -3235,11 +3235,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                 Some((type_did, self.tcx.expect_variant_def(def)))
             }
             Def::TyAlias(did) => {
-                if let Some(&ty::TyStruct(adt, _)) = self.tcx.opt_lookup_item_type(did)
-                                                             .map(|scheme| &scheme.ty.sty) {
-                    Some((did, adt.struct_variant()))
-                } else {
-                    None
+                match self.tcx.opt_lookup_item_type(did).map(|scheme| &scheme.ty.sty) {
+                    Some(&ty::TyStruct(adt, _)) |
+                    Some(&ty::TyUnion(adt, _)) => Some((did, adt.struct_variant())),
+                    _ => None,
                 }
             }
             _ => None

src/librustc_typeck/collect.rs

@@ -1450,7 +1450,8 @@ fn generics_of_def_id<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>,
 
                     ItemTy(_, ref generics) |
                     ItemEnum(_, ref generics) |
-                    ItemStruct(_, ref generics) => {
+                    ItemStruct(_, ref generics) |
+                    ItemUnion(_, ref generics) => {
                         allow_defaults = true;
                         generics
                     }

src/test/compile-fail/union-with-drop-fields-lint.rs

@@ -0,0 +1,40 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(untagged_unions)]
+#![allow(dead_code)]
+#![deny(unions_with_drop_fields)]
+
+union U {
+    a: u8, // OK
+}
+
+union W {
+    a: String, //~ ERROR union contains a field with possibly non-trivial drop code
+    b: String, // OK, only one field is reported
+}
+
+struct S(String);
+
+// `S` doesn't implement `Drop` trait, but still has non-trivial destructor
+union Y {
+    a: S, //~ ERROR union contains a field with possibly non-trivial drop code
+}
+
+// We don't know if `T` is trivially-destructable or not until trans
+union J<T> {
+    a: T, //~ ERROR union contains a field with possibly non-trivial drop code
+}
+
+union H<T: Copy> {
+    a: T, // OK, `T` is `Copy`, no destructor
+}
+
+fn main() {}

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

@@ -12,15 +12,54 @@
 
 #![feature(untagged_unions)]
 
+struct S;
+
 union U {
     a: u8
 }
 
+union W {
+    a: S,
+}
+
+union Y {
+    a: S,
+}
+
+impl Drop for S {
+    fn drop(&mut self) {
+        unsafe { CHECK += 10; }
+    }
+}
+
 impl Drop for U {
-    fn drop(&mut self) {}
+    fn drop(&mut self) {
+        unsafe { CHECK += 1; }
+    }
 }
 
+impl Drop for W {
+    fn drop(&mut self) {
+        unsafe { CHECK += 1; }
+    }
+}
+
+static mut CHECK: u8 = 0;
+
 fn main() {
-    // 'unions are not fully implemented', src/librustc_trans/glue.rs:567
-    // let u = U { a: 1 };
+    unsafe {
+        assert_eq!(CHECK, 0);
+        {
+            let u = U { a: 1 };
+        }
+        assert_eq!(CHECK, 1); // 1, dtor of U is called
+        {
+            let w = W { a: S };
+        }
+        assert_eq!(CHECK, 2); // 2, not 11, dtor of S is not called
+        {
+            let y = Y { a: S };
+        }
+        assert_eq!(CHECK, 2); // 2, not 12, dtor of S is not called
+    }
 }