librustc: Forbid destructors from being attached to any structs that might contain non-Owned fields. r=nmatsakis

Author: pcwalton

src/libcore/condition.rs

@@ -84,6 +84,7 @@ struct Guard<T, U> {
     cond: &'self Condition/&self<T, U>
 }
 
+#[unsafe_destructor]
 impl<T, U> Drop for Guard/&self<T, U> {
     fn finalize(&self) {
         unsafe {

src/libcore/io.rs

@@ -1230,15 +1230,17 @@ pub mod fsync {
         arg: Arg<t>,
     }
 
+    #[unsafe_destructor]
     impl<T:Copy> Drop for Res<T> {
         fn finalize(&self) {
-          match self.arg.opt_level {
-            None => (),
-            Some(level) => {
-              // fail hard if not succesful
-              fail_unless!(((self.arg.fsync_fn)(self.arg.val, level) != -1));
+            match self.arg.opt_level {
+                None => (),
+                Some(level) => {
+                  // fail hard if not succesful
+                  fail_unless!(((self.arg.fsync_fn)(self.arg.val, level)
+                    != -1));
+                }
             }
-          }
         }
     }
 

src/libcore/option.rs

@@ -514,6 +514,7 @@ fn test_unwrap_resource() {
        i: @mut int,
     }
 
+    #[unsafe_destructor]
     impl ::ops::Drop for R {
        fn finalize(&self) { *(self.i) += 1; }
     }

src/libcore/pipes.rs

@@ -350,6 +350,7 @@ struct BufferResource<T> {
 
 }
 
+#[unsafe_destructor]
 impl<T> ::ops::Drop for BufferResource<T> {
     fn finalize(&self) {
         unsafe {
@@ -445,16 +446,17 @@ pub fn try_recv<T:Owned,Tbuffer:Owned>(p: RecvPacketBuffered<T, Tbuffer>)
     let p_ = p.unwrap();
     let p = unsafe { &*p_ };
 
+    #[unsafe_destructor]
     struct DropState {
         p: &'self PacketHeader,
 
         drop {
-            if task::failing() {
-                self.p.state = Terminated;
-                let old_task = swap_task(&mut self.p.blocked_task,
-                                         ptr::null());
-                if !old_task.is_null() {
-                    unsafe {
+            unsafe {
+                if task::failing() {
+                    self.p.state = Terminated;
+                    let old_task = swap_task(&mut self.p.blocked_task,
+                                             ptr::null());
+                    if !old_task.is_null() {
                         rustrt::rust_task_deref(old_task);
                     }
                 }
@@ -773,6 +775,7 @@ pub struct SendPacketBuffered<T, Tbuffer> {
     mut buffer: Option<BufferResource<Tbuffer>>,
 }
 
+#[unsafe_destructor]
 impl<T:Owned,Tbuffer:Owned> ::ops::Drop for SendPacketBuffered<T,Tbuffer> {
     fn finalize(&self) {
         //if self.p != none {
@@ -842,6 +845,7 @@ pub struct RecvPacketBuffered<T, Tbuffer> {
     mut buffer: Option<BufferResource<Tbuffer>>,
 }
 
+#[unsafe_destructor]
 impl<T:Owned,Tbuffer:Owned> ::ops::Drop for RecvPacketBuffered<T,Tbuffer> {
     fn finalize(&self) {
         //if self.p != none {

src/libcore/unstable.rs

@@ -118,6 +118,7 @@ struct ArcDestruct<T> {
     mut data: *libc::c_void,
 }
 
+#[unsafe_destructor]
 impl<T> Drop for ArcDestruct<T>{
     fn finalize(&self) {
         unsafe {

src/libcore/unstable/finally.rs

@@ -45,6 +45,7 @@ struct Finallyalizer {
     dtor: &'self fn()
 }
 
+#[unsafe_destructor]
 impl Drop for Finallyalizer/&self {
     fn finalize(&self) {
         (self.dtor)();

src/librustc/middle/kind.rs

@@ -22,6 +22,7 @@ use core::str;
 use core::vec;
 use std::oldmap::HashMap;
 use syntax::ast::*;
+use syntax::attr::attrs_contains_name;
 use syntax::codemap::{span, spanned};
 use syntax::print::pprust::expr_to_str;
 use syntax::{visit, ast_util};
@@ -55,6 +56,8 @@ use syntax::{visit, ast_util};
 // primitives in the stdlib are explicitly annotated to only take sendable
 // types.
 
+use core::hashmap::linear::LinearSet;
+
 pub const try_adding: &'static str = "Try adding a move";
 
 pub type rval_map = HashMap<node_id, ()>;
@@ -63,7 +66,7 @@ pub struct Context {
     tcx: ty::ctxt,
     method_map: typeck::method_map,
     last_use_map: liveness::last_use_map,
-    current_item: node_id
+    current_item: node_id,
 }
 
 pub fn check_crate(tcx: ty::ctxt,
@@ -74,16 +77,15 @@ pub fn check_crate(tcx: ty::ctxt,
         tcx: tcx,
         method_map: method_map,
         last_use_map: last_use_map,
-        current_item: -1
+        current_item: -1,
     };
     let visit = visit::mk_vt(@visit::Visitor {
         visit_arm: check_arm,
         visit_expr: check_expr,
         visit_fn: check_fn,
         visit_ty: check_ty,
-        visit_item: |i, cx, v| {
-            visit::visit_item(i, Context { current_item: i.id,.. cx }, v);
-        },
+        visit_item: check_item,
+        visit_block: check_block,
         .. *visit::default_visitor()
     });
     visit::visit_crate(*crate, ctx, visit);
@@ -92,6 +94,93 @@ pub fn check_crate(tcx: ty::ctxt,
 
 type check_fn = @fn(Context, @freevar_entry);
 
+fn check_struct_safe_for_destructor(cx: Context,
+                                    span: span,
+                                    struct_did: def_id) {
+    let struct_tpt = ty::lookup_item_type(cx.tcx, struct_did);
+    if struct_tpt.bounds.len() == 0 {
+        let struct_ty = ty::mk_struct(cx.tcx, struct_did, ty::substs {
+            self_r: None,
+            self_ty: None,
+            tps: ~[]
+        });
+        if !ty::type_is_owned(cx.tcx, struct_ty) {
+            cx.tcx.sess.span_err(span,
+                                 ~"cannot implement a destructor on a struct \
+                                   that is not Owned");
+            cx.tcx.sess.span_note(span,
+                                  ~"use \"#[unsafe_destructor]\" on the \
+                                    implementation to force the compiler to \
+                                    allow this");
+        }
+    } else {
+        cx.tcx.sess.span_err(span,
+                             ~"cannot implement a destructor on a struct \
+                               with type parameters");
+        cx.tcx.sess.span_note(span,
+                              ~"use \"#[unsafe_destructor]\" on the \
+                                implementation to force the compiler to \
+                                allow this");
+    }
+}
+
+fn check_block(block: &blk, cx: Context, visitor: visit::vt<Context>) {
+    visit::visit_block(block, cx, visitor);
+}
+
+fn check_item(item: @item, cx: Context, visitor: visit::vt<Context>) {
+    // If this is a destructor, check kinds.
+    if !attrs_contains_name(item.attrs, "unsafe_destructor") {
+        match item.node {
+            item_impl(_, Some(trait_ref), self_type, _) => {
+                match cx.tcx.def_map.find(&trait_ref.ref_id) {
+                    None => cx.tcx.sess.bug(~"trait ref not in def map!"),
+                    Some(trait_def) => {
+                        let trait_def_id = ast_util::def_id_of_def(trait_def);
+                        if cx.tcx.lang_items.drop_trait() == trait_def_id {
+                            // Yes, it's a destructor.
+                            match self_type.node {
+                                ty_path(_, path_node_id) => {
+                                    let struct_def = cx.tcx.def_map.get(
+                                        &path_node_id);
+                                    let struct_did =
+                                        ast_util::def_id_of_def(struct_def);
+                                    check_struct_safe_for_destructor(
+                                        cx,
+                                        self_type.span,
+                                        struct_did);
+                                }
+                                _ => {
+                                    cx.tcx.sess.span_bug(self_type.span,
+                                                         ~"the self type for \
+                                                           the Drop trait \
+                                                           impl is not a \
+                                                           path");
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+            item_struct(struct_def, _) => {
+                match struct_def.dtor {
+                    None => {}
+                    Some(ref dtor) => {
+                        let struct_did = def_id { crate: 0, node: item.id };
+                        check_struct_safe_for_destructor(cx,
+                                                         dtor.span,
+                                                         struct_did);
+                    }
+                }
+            }
+            _ => {}
+        }
+    }
+
+    let cx = Context { current_item: item.id, ..cx };
+    visit::visit_item(item, cx, visitor);
+}
+
 // Yields the appropriate function to check the kind of closed over
 // variables. `id` is the node_id for some expression that creates the
 // closure.

src/librustc/middle/trans/base.rs

@@ -88,11 +88,14 @@ pub struct icx_popper {
     ccx: @CrateContext,
 }
 
+#[unsafe_destructor]
 impl Drop for icx_popper {
     fn finalize(&self) {
-      if self.ccx.sess.count_llvm_insns() {
-          self.ccx.stats.llvm_insn_ctxt.pop();
-      }
+        unsafe {
+            if self.ccx.sess.count_llvm_insns() {
+                self.ccx.stats.llvm_insn_ctxt.pop();
+            }
+        }
     }
 }
 

src/libstd/arena.rs

@@ -88,6 +88,7 @@ pub struct Arena {
     priv mut chunks: @List<Chunk>,
 }
 
+#[unsafe_destructor]
 impl Drop for Arena {
     fn finalize(&self) {
         unsafe {

src/libstd/c_vec.rs

@@ -54,11 +54,14 @@ struct DtorRes {
   dtor: Option<@fn()>,
 }
 
+#[unsafe_destructor]
 impl Drop for DtorRes {
     fn finalize(&self) {
-        match self.dtor {
-          option::None => (),
-          option::Some(f) => f()
+        unsafe {
+            match self.dtor {
+                option::None => (),
+                option::Some(f) => f()
+            }
         }
     }
 }

src/libstd/future.rs

@@ -35,6 +35,7 @@ pub struct Future<A> {
 
 // FIXME(#2829) -- futures should not be copyable, because they close
 // over ~fn's that have pipes and so forth within!
+#[unsafe_destructor]
 impl<A> Drop for Future<A> {
     fn finalize(&self) {}
 }

src/libstd/net_tcp.rs

@@ -55,6 +55,7 @@ pub struct TcpSocket {
   socket_data: @TcpSocketData,
 }
 
+#[unsafe_destructor]
 impl Drop for TcpSocket {
     fn finalize(&self) {
         unsafe {

src/libstd/sort.rs

@@ -1190,6 +1190,7 @@ mod big_tests {
         key: &'self fn(@uint),
     }
 
+    #[unsafe_destructor]
     impl Drop for LVal/&self {
         fn finalize(&self) {
             let x = unsafe { task::local_data::local_data_get(self.key) };

src/libstd/sync.rs

@@ -167,9 +167,12 @@ type SemRelease = SemReleaseGeneric<'self, ()>;
 type SemAndSignalRelease = SemReleaseGeneric<'self, ~[Waitqueue]>;
 struct SemReleaseGeneric<Q> { sem: &'self Sem<Q> }
 
+#[unsafe_destructor]
 impl<Q:Owned> Drop for SemReleaseGeneric/&self<Q> {
     fn finalize(&self) {
-        self.sem.release();
+        unsafe {
+            self.sem.release();
+        }
     }
 }
 
@@ -189,6 +192,7 @@ fn SemAndSignalRelease(sem: &'r Sem<~[Waitqueue]>)
 /// A mechanism for atomic-unlock-and-deschedule blocking and signalling.
 pub struct Condvar { priv sem: &'self Sem<~[Waitqueue]> }
 
+#[unsafe_destructor]
 impl Drop for Condvar/&self { fn finalize(&self) {} }
 
 pub impl Condvar/&self {
@@ -261,6 +265,7 @@ pub impl Condvar/&self {
             sem: &'self Sem<~[Waitqueue]>,
         }
 
+        #[unsafe_destructor]
         impl Drop for SemAndSignalReacquire/&self {
             fn finalize(&self) {
                 unsafe {
@@ -613,6 +618,7 @@ struct RWlockReleaseRead {
     lock: &'self RWlock,
 }
 
+#[unsafe_destructor]
 impl Drop for RWlockReleaseRead/&self {
     fn finalize(&self) {
         unsafe {
@@ -643,10 +649,12 @@ fn RWlockReleaseRead(lock: &'r RWlock) -> RWlockReleaseRead/&r {
 
 // FIXME(#3588) should go inside of downgrade()
 #[doc(hidden)]
+#[unsafe_destructor]
 struct RWlockReleaseDowngrade {
     lock: &'self RWlock,
 }
 
+#[unsafe_destructor]
 impl Drop for RWlockReleaseDowngrade/&self {
     fn finalize(&self) {
         unsafe {
@@ -685,10 +693,12 @@ fn RWlockReleaseDowngrade(lock: &'r RWlock) -> RWlockReleaseDowngrade/&r {
 
 /// The "write permission" token used for rwlock.write_downgrade().
 pub struct RWlockWriteMode { priv lock: &'self RWlock }
+#[unsafe_destructor]
 impl Drop for RWlockWriteMode/&self { fn finalize(&self) {} }
 
 /// The "read permission" token used for rwlock.write_downgrade().
 pub struct RWlockReadMode  { priv lock: &'self RWlock }
+#[unsafe_destructor]
 impl Drop for RWlockReadMode/&self { fn finalize(&self) {} }
 
 pub impl RWlockWriteMode/&self {

src/libstd/task_pool.rs

@@ -28,6 +28,7 @@ pub struct TaskPool<T> {
 
 }
 
+#[unsafe_destructor]
 impl<T> Drop for TaskPool<T> {
     fn finalize(&self) {
         for self.channels.each |channel| {

src/libsyntax/parse/parser.rs

@@ -268,6 +268,7 @@ pub struct Parser {
 
 }
 
+#[unsafe_destructor]
 impl Drop for Parser {
     /* do not copy the parser; its state is tied to outside state */
     fn finalize(&self) {}

src/test/auxiliary/issue-2526.rs

@@ -19,6 +19,7 @@ struct arc_destruct<T> {
   _data: int,
 }
 
+#[unsafe_destructor]
 impl<T:Const> Drop for arc_destruct<T> {
     fn finalize(&self) {}
 }