Skip to content

Commit d161e63

Browse files
committed
auto merge of rust-lang#7317 : Aatch/rust/no-drop-flag, r=thestinger
This adds a `#[no_drop_flag]` attribute. This attribute tells the compiler to omit the drop flag from the struct, if it has a destructor. When the destructor is run, instead of setting the drop flag, it instead zeroes-out the struct. This means the destructor can run multiple times and therefore it is up to the developer to use it safely. The primary usage case for this is smart-pointer types like `Rc<T>` as the extra flag caused the struct to be 1 word larger because of alignment. This closes rust-lang#7271 and rust-lang#7138
2 parents 7aee5da + 721164d commit d161e63

File tree

6 files changed

+112
-19
lines changed

6 files changed

+112
-19
lines changed

src/libextra/rc.rs

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,12 @@ impl<T> Rc<T> {
7070
impl<T> Drop for Rc<T> {
7171
fn finalize(&self) {
7272
unsafe {
73-
(*self.ptr).count -= 1;
74-
if (*self.ptr).count == 0 {
75-
ptr::replace_ptr(self.ptr, intrinsics::uninit());
76-
free(self.ptr as *c_void)
73+
if self.ptr.is_not_null() {
74+
(*self.ptr).count -= 1;
75+
if (*self.ptr).count == 0 {
76+
ptr::replace_ptr(self.ptr, intrinsics::uninit());
77+
free(self.ptr as *c_void)
78+
}
7779
}
7880
}
7981
}
@@ -220,10 +222,12 @@ impl<T> RcMut<T> {
220222
impl<T> Drop for RcMut<T> {
221223
fn finalize(&self) {
222224
unsafe {
223-
(*self.ptr).count -= 1;
224-
if (*self.ptr).count == 0 {
225-
ptr::replace_ptr(self.ptr, uninit());
226-
free(self.ptr as *c_void)
225+
if self.ptr.is_not_null() {
226+
(*self.ptr).count -= 1;
227+
if (*self.ptr).count == 0 {
228+
ptr::replace_ptr(self.ptr, uninit());
229+
free(self.ptr as *c_void)
230+
}
227231
}
228232
}
229233
}

src/librustc/middle/trans/adt.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ fn represent_type_uncached(cx: &mut CrateContext, t: ty::t) -> Repr {
135135
ty::lookup_field_type(cx.tcx, def_id, field.id, substs)
136136
};
137137
let packed = ty::lookup_packed(cx.tcx, def_id);
138-
let dtor = ty::ty_dtor(cx.tcx, def_id).is_present();
138+
let dtor = ty::ty_dtor(cx.tcx, def_id).has_drop_flag();
139139
let ftys =
140140
if dtor { ftys + [ty::mk_bool()] } else { ftys };
141141
return Univariant(mk_struct(cx, ftys, packed), dtor)

src/librustc/middle/trans/glue.rs

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -404,13 +404,8 @@ pub fn make_free_glue(bcx: block, v: ValueRef, t: ty::t) {
404404
build_return(bcx);
405405
}
406406

407-
pub fn trans_struct_drop(bcx: block,
408-
t: ty::t,
409-
v0: ValueRef,
410-
dtor_did: ast::def_id,
411-
class_did: ast::def_id,
412-
substs: &ty::substs)
413-
-> block {
407+
pub fn trans_struct_drop_flag(bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast::def_id,
408+
class_did: ast::def_id, substs: &ty::substs) -> block {
414409
let repr = adt::represent_type(bcx.ccx(), t);
415410
let drop_flag = adt::trans_drop_flag_ptr(bcx, repr, v0);
416411
do with_cond(bcx, IsNotNull(bcx, Load(bcx, drop_flag))) |cx| {
@@ -452,6 +447,43 @@ pub fn trans_struct_drop(bcx: block,
452447
}
453448
}
454449

450+
pub fn trans_struct_drop(mut bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast::def_id,
451+
class_did: ast::def_id, substs: &ty::substs) -> block {
452+
let repr = adt::represent_type(bcx.ccx(), t);
453+
454+
// Find and call the actual destructor
455+
let dtor_addr = get_res_dtor(bcx.ccx(), dtor_did,
456+
class_did, /*bad*/copy substs.tps);
457+
458+
// The second argument is the "self" argument for drop
459+
let params = unsafe {
460+
let ty = Type::from_ref(llvm::LLVMTypeOf(dtor_addr));
461+
ty.element_type().func_params()
462+
};
463+
464+
// Class dtors have no explicit args, so the params should
465+
// just consist of the environment (self)
466+
assert_eq!(params.len(), 1);
467+
468+
// Take a reference to the class (because it's using the Drop trait),
469+
// do so now.
470+
let llval = alloca(bcx, val_ty(v0));
471+
Store(bcx, v0, llval);
472+
473+
let self_arg = PointerCast(bcx, llval, params[0]);
474+
let args = ~[self_arg];
475+
476+
Call(bcx, dtor_addr, args);
477+
478+
// Drop the fields
479+
let field_tys = ty::struct_fields(bcx.tcx(), class_did, substs);
480+
for field_tys.iter().enumerate().advance |(i, fld)| {
481+
let llfld_a = adt::trans_field_ptr(bcx, repr, v0, 0, i);
482+
bcx = drop_ty(bcx, llfld_a, fld.mt.ty);
483+
}
484+
485+
bcx
486+
}
455487

456488
pub fn make_drop_glue(bcx: block, v0: ValueRef, t: ty::t) {
457489
// NB: v0 is an *alias* of type t here, not a direct value.
@@ -472,7 +504,10 @@ pub fn make_drop_glue(bcx: block, v0: ValueRef, t: ty::t) {
472504
ty::ty_struct(did, ref substs) => {
473505
let tcx = bcx.tcx();
474506
match ty::ty_dtor(tcx, did) {
475-
ty::TraitDtor(dtor) => {
507+
ty::TraitDtor(dtor, true) => {
508+
trans_struct_drop_flag(bcx, t, v0, dtor, did, substs)
509+
}
510+
ty::TraitDtor(dtor, false) => {
476511
trans_struct_drop(bcx, t, v0, dtor, did, substs)
477512
}
478513
ty::NoDtor => {
@@ -592,6 +627,23 @@ pub fn make_take_glue(bcx: block, v: ValueRef, t: ty::t) {
592627
ty::ty_opaque_closure_ptr(ck) => {
593628
closure::make_opaque_cbox_take_glue(bcx, ck, v)
594629
}
630+
ty::ty_struct(did, ref substs) => {
631+
let tcx = bcx.tcx();
632+
let bcx = iter_structural_ty(bcx, v, t, take_ty);
633+
634+
match ty::ty_dtor(tcx, did) {
635+
ty::TraitDtor(dtor, false) => {
636+
// Zero out the struct
637+
unsafe {
638+
let ty = Type::from_ref(llvm::LLVMTypeOf(v));
639+
memzero(bcx, v, ty);
640+
}
641+
642+
}
643+
_ => { }
644+
}
645+
bcx
646+
}
595647
_ if ty::type_is_structural(t) => {
596648
iter_structural_ty(bcx, v, t, take_ty)
597649
}

src/librustc/middle/ty.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3852,7 +3852,7 @@ pub fn item_path_str(cx: ctxt, id: ast::def_id) -> ~str {
38523852

38533853
pub enum DtorKind {
38543854
NoDtor,
3855-
TraitDtor(def_id)
3855+
TraitDtor(def_id, bool)
38563856
}
38573857

38583858
impl DtorKind {
@@ -3866,13 +3866,24 @@ impl DtorKind {
38663866
pub fn is_present(&const self) -> bool {
38673867
!self.is_not_present()
38683868
}
3869+
3870+
pub fn has_drop_flag(&self) -> bool {
3871+
match self {
3872+
&NoDtor => false,
3873+
&TraitDtor(_, flag) => flag
3874+
}
3875+
}
38693876
}
38703877

38713878
/* If struct_id names a struct with a dtor, return Some(the dtor's id).
38723879
Otherwise return none. */
38733880
pub fn ty_dtor(cx: ctxt, struct_id: def_id) -> DtorKind {
38743881
match cx.destructor_for_type.find(&struct_id) {
3875-
Some(&method_def_id) => TraitDtor(method_def_id),
3882+
Some(&method_def_id) => {
3883+
let flag = !has_attr(cx, struct_id, "no_drop_flag");
3884+
3885+
TraitDtor(method_def_id, flag)
3886+
}
38763887
None => NoDtor,
38773888
}
38783889
}

src/libstd/unstable/atomics.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ pub struct AtomicPtr<T> {
6262
/**
6363
* An owned atomic pointer. Ensures that only a single reference to the data is held at any time.
6464
*/
65+
#[no_drop_flag]
6566
pub struct AtomicOption<T> {
6667
priv p: *mut c_void
6768
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use std::sys::size_of;
12+
13+
#[no_drop_flag]
14+
struct Test<T> {
15+
a: T
16+
}
17+
18+
#[unsafe_destructor]
19+
impl<T> Drop for Test<T> {
20+
fn finalize(&self) { }
21+
}
22+
23+
fn main() {
24+
assert_eq!(size_of::<int>(), size_of::<Test<int>>());
25+
}

0 commit comments

Comments
 (0)