From 0cca08a21a33b60f9a27a4bab119b5ec8261be56 Mon Sep 17 00:00:00 2001 From: James Miller Date: Sun, 23 Jun 2013 14:41:45 +1200 Subject: [PATCH 1/3] Add support for #[no_drop_flag] attribute --- src/librustc/middle/trans/adt.rs | 2 +- src/librustc/middle/trans/glue.rs | 57 ++++++++++++++++++++++++++----- src/librustc/middle/ty.rs | 15 ++++++-- 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index 624c6607859..9b7c7037f42 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -135,7 +135,7 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr { ty::lookup_field_type(cx.tcx, def_id, field.id, substs) }; let packed = ty::lookup_packed(cx.tcx, def_id); - let dtor = ty::ty_dtor(cx.tcx, def_id).is_present(); + let dtor = ty::ty_dtor(cx.tcx, def_id).has_drop_flag(); let ftys = if dtor { ftys + [ty::mk_bool()] } else { ftys }; return Univariant(mk_struct(cx, ftys, packed), dtor) diff --git a/src/librustc/middle/trans/glue.rs b/src/librustc/middle/trans/glue.rs index d8ba524b2bd..056e5b8cdf7 100644 --- a/src/librustc/middle/trans/glue.rs +++ b/src/librustc/middle/trans/glue.rs @@ -406,13 +406,8 @@ pub fn make_free_glue(bcx: block, v: ValueRef, t: ty::t) { build_return(bcx); } -pub fn trans_struct_drop(bcx: block, - t: ty::t, - v0: ValueRef, - dtor_did: ast::def_id, - class_did: ast::def_id, - substs: &ty::substs) - -> block { +pub fn trans_struct_drop_flag(bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast::def_id, + class_did: ast::def_id, substs: &ty::substs) -> block { let repr = adt::represent_type(bcx.ccx(), t); let drop_flag = adt::trans_drop_flag_ptr(bcx, repr, v0); do with_cond(bcx, IsNotNull(bcx, Load(bcx, drop_flag))) |cx| { @@ -454,6 +449,49 @@ pub fn trans_struct_drop(bcx: block, } } +pub fn trans_struct_drop(mut bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast::def_id, + class_did: ast::def_id, substs: &ty::substs) -> block { + let repr = adt::represent_type(bcx.ccx(), t); + + // Find and call the actual destructor + let dtor_addr = get_res_dtor(bcx.ccx(), dtor_did, + class_did, /*bad*/copy substs.tps); + + // The second argument is the "self" argument for drop + let params = unsafe { + let ty = Type::from_ref(llvm::LLVMTypeOf(dtor_addr)); + ty.element_type().func_params() + }; + + // Class dtors have no explicit args, so the params should + // just consist of the environment (self) + assert_eq!(params.len(), 1); + + // Take a reference to the class (because it's using the Drop trait), + // do so now. + let llval = alloca(bcx, val_ty(v0)); + Store(bcx, v0, llval); + + let self_arg = PointerCast(bcx, llval, params[0]); + let args = ~[self_arg]; + + Call(bcx, dtor_addr, args); + + // Drop the fields + let field_tys = ty::struct_fields(bcx.tcx(), class_did, substs); + for field_tys.iter().enumerate().advance |(i, fld)| { + let llfld_a = adt::trans_field_ptr(bcx, repr, v0, 0, i); + bcx = drop_ty(bcx, llfld_a, fld.mt.ty); + } + + // Zero out the struct + unsafe { + let ty = Type::from_ref(llvm::LLVMTypeOf(v0)); + memzero(bcx, v0, ty); + } + + bcx +} pub fn make_drop_glue(bcx: block, v0: ValueRef, t: ty::t) { // NB: v0 is an *alias* of type t here, not a direct value. @@ -474,7 +512,10 @@ pub fn make_drop_glue(bcx: block, v0: ValueRef, t: ty::t) { ty::ty_struct(did, ref substs) => { let tcx = bcx.tcx(); match ty::ty_dtor(tcx, did) { - ty::TraitDtor(dtor) => { + ty::TraitDtor(dtor, true) => { + trans_struct_drop_flag(bcx, t, v0, dtor, did, substs) + } + ty::TraitDtor(dtor, false) => { trans_struct_drop(bcx, t, v0, dtor, did, substs) } ty::NoDtor => { diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 90cd8a8665e..424307502f2 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -3855,7 +3855,7 @@ pub fn item_path_str(cx: ctxt, id: ast::def_id) -> ~str { pub enum DtorKind { NoDtor, - TraitDtor(def_id) + TraitDtor(def_id, bool) } impl DtorKind { @@ -3869,13 +3869,24 @@ impl DtorKind { pub fn is_present(&const self) -> bool { !self.is_not_present() } + + pub fn has_drop_flag(&self) -> bool { + match self { + &NoDtor => false, + &TraitDtor(_, flag) => flag + } + } } /* If struct_id names a struct with a dtor, return Some(the dtor's id). Otherwise return none. */ pub fn ty_dtor(cx: ctxt, struct_id: def_id) -> DtorKind { match cx.destructor_for_type.find(&struct_id) { - Some(&method_def_id) => TraitDtor(method_def_id), + Some(&method_def_id) => { + let flag = has_attr(cx, struct_id, "no_drop_flag"); + + TraitDtor(method_def_id, flag) + } None => NoDtor, } } From d9f6dd263c16a21108c27dbf15a3d59a43a5b490 Mon Sep 17 00:00:00 2001 From: James Miller Date: Sun, 23 Jun 2013 16:13:29 +1200 Subject: [PATCH 2/3] Set #[no_drop_flag] on Rc and AtomicOption. Add Test --- src/libextra/rc.rs | 20 ++++++++++------- src/librustc/middle/ty.rs | 2 +- src/libstd/unstable/atomics.rs | 1 + src/test/run-pass/attr-no-drop-flag-size.rs | 25 +++++++++++++++++++++ 4 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 src/test/run-pass/attr-no-drop-flag-size.rs diff --git a/src/libextra/rc.rs b/src/libextra/rc.rs index b90b0983dc2..555cceb5b44 100644 --- a/src/libextra/rc.rs +++ b/src/libextra/rc.rs @@ -70,10 +70,12 @@ impl Rc { impl Drop for Rc { fn finalize(&self) { unsafe { - (*self.ptr).count -= 1; - if (*self.ptr).count == 0 { - ptr::replace_ptr(self.ptr, intrinsics::uninit()); - free(self.ptr as *c_void) + if self.ptr.is_not_null() { + (*self.ptr).count -= 1; + if (*self.ptr).count == 0 { + ptr::replace_ptr(self.ptr, intrinsics::uninit()); + free(self.ptr as *c_void) + } } } } @@ -220,10 +222,12 @@ impl RcMut { impl Drop for RcMut { fn finalize(&self) { unsafe { - (*self.ptr).count -= 1; - if (*self.ptr).count == 0 { - ptr::replace_ptr(self.ptr, uninit()); - free(self.ptr as *c_void) + if self.ptr.is_not_null() { + (*self.ptr).count -= 1; + if (*self.ptr).count == 0 { + ptr::replace_ptr(self.ptr, uninit()); + free(self.ptr as *c_void) + } } } } diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 424307502f2..d5a2ed7dbd0 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -3883,7 +3883,7 @@ impl DtorKind { pub fn ty_dtor(cx: ctxt, struct_id: def_id) -> DtorKind { match cx.destructor_for_type.find(&struct_id) { Some(&method_def_id) => { - let flag = has_attr(cx, struct_id, "no_drop_flag"); + let flag = !has_attr(cx, struct_id, "no_drop_flag"); TraitDtor(method_def_id, flag) } diff --git a/src/libstd/unstable/atomics.rs b/src/libstd/unstable/atomics.rs index 6e7a7e2b129..7a3a5f51d35 100644 --- a/src/libstd/unstable/atomics.rs +++ b/src/libstd/unstable/atomics.rs @@ -62,6 +62,7 @@ pub struct AtomicPtr { /** * An owned atomic pointer. Ensures that only a single reference to the data is held at any time. */ +#[no_drop_flag] pub struct AtomicOption { priv p: *mut c_void } diff --git a/src/test/run-pass/attr-no-drop-flag-size.rs b/src/test/run-pass/attr-no-drop-flag-size.rs new file mode 100644 index 00000000000..e6f05970cce --- /dev/null +++ b/src/test/run-pass/attr-no-drop-flag-size.rs @@ -0,0 +1,25 @@ +// Copyright 2012 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::sys::size_of; + +#[no_drop_flag] +struct Test { + a: T +} + +#[unsafe_destructor] +impl Drop for Test { + fn finalize(&self) { } +} + +fn main() { + assert_eq!(size_of::(), size_of::>()); +} From 721164d5ec0c8b617bd72df36830fe1861e6362b Mon Sep 17 00:00:00 2001 From: James Miller Date: Tue, 25 Jun 2013 16:39:56 +1200 Subject: [PATCH 3/3] Zero the struct in the take glue, not the drop glue --- src/librustc/middle/trans/glue.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/librustc/middle/trans/glue.rs b/src/librustc/middle/trans/glue.rs index 056e5b8cdf7..8ed3112b520 100644 --- a/src/librustc/middle/trans/glue.rs +++ b/src/librustc/middle/trans/glue.rs @@ -484,12 +484,6 @@ pub fn trans_struct_drop(mut bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast:: bcx = drop_ty(bcx, llfld_a, fld.mt.ty); } - // Zero out the struct - unsafe { - let ty = Type::from_ref(llvm::LLVMTypeOf(v0)); - memzero(bcx, v0, ty); - } - bcx } @@ -635,6 +629,23 @@ pub fn make_take_glue(bcx: block, v: ValueRef, t: ty::t) { ty::ty_opaque_closure_ptr(ck) => { closure::make_opaque_cbox_take_glue(bcx, ck, v) } + ty::ty_struct(did, ref substs) => { + let tcx = bcx.tcx(); + let bcx = iter_structural_ty(bcx, v, t, take_ty); + + match ty::ty_dtor(tcx, did) { + ty::TraitDtor(dtor, false) => { + // Zero out the struct + unsafe { + let ty = Type::from_ref(llvm::LLVMTypeOf(v)); + memzero(bcx, v, ty); + } + + } + _ => { } + } + bcx + } _ if ty::type_is_structural(t) => { iter_structural_ty(bcx, v, t, take_ty) }