Skip to content

Commit

Permalink
Fix handling of temp cleanups for the "self" argument
Browse files Browse the repository at this point in the history
The code that tried to revoke the cleanup for the self argument tried
to use "llself" to do so, but the cleanup might actually be registered
with a different ValueRef due to e.g. casting. Currently, this is
worked around by early revocation of the cleanup for self in
trans_self_arg.

To handle this correctly, we have to put the ValueRef for the cleanup
into the MethodData, so trans_call_inner can use it to revoke the
cleanup when it's actually supposed to.
  • Loading branch information
dotdash committed Jun 29, 2013
1 parent 2a40c5d commit ba922a4
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 25 deletions.
8 changes: 5 additions & 3 deletions src/librustc/middle/trans/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ pub struct FnData {
pub struct MethodData {
llfn: ValueRef,
llself: ValueRef,
temp_cleanup: Option<ValueRef>,
self_ty: ty::t,
self_mode: ty::SelfMode,
explicit_self: ast::explicit_self_
Expand Down Expand Up @@ -646,9 +647,10 @@ pub fn trans_call_inner(in_cx: block,
// Now that the arguments have finished evaluating, we need to revoke
// the cleanup for the self argument, if it exists
match callee.data {
Method(d) if d.self_mode == ty::ByCopy ||
d.explicit_self == ast::sty_value => {
revoke_clean(bcx, d.llself);
Method(d) => {
for d.temp_cleanup.iter().advance |&v| {
revoke_clean(bcx, v);
}
}
_ => {}
}
Expand Down
40 changes: 18 additions & 22 deletions src/librustc/middle/trans/meth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,28 +129,20 @@ pub fn trans_method(ccx: @mut CrateContext,

pub fn trans_self_arg(bcx: block,
base: @ast::expr,
temp_cleanups: &mut ~[ValueRef],
mentry: typeck::method_map_entry) -> Result {
let _icx = push_ctxt("impl::trans_self_arg");
let mut temp_cleanups = ~[];

// self is passed as an opaque box in the environment slot
let self_ty = ty::mk_opaque_box(bcx.tcx());
let result = trans_arg_expr(bcx,
self_ty,
mentry.self_mode,
mentry.explicit_self,
base,
&mut temp_cleanups,
None,
DontAutorefArg);

// FIXME(#3446)---this is wrong, actually. The temp_cleanups
// should be revoked only after all arguments have been passed.
for temp_cleanups.iter().advance |c| {
revoke_clean(bcx, *c)
}

return result;
trans_arg_expr(bcx,
self_ty,
mentry.self_mode,
mentry.explicit_self,
base,
temp_cleanups,
None,
DontAutorefArg)
}

pub fn trans_method_callee(bcx: block,
Expand Down Expand Up @@ -203,12 +195,14 @@ pub fn trans_method_callee(bcx: block,
match origin {
typeck::method_static(did) => {
let callee_fn = callee::trans_fn_ref(bcx, did, callee_id);
let Result {bcx, val} = trans_self_arg(bcx, this, mentry);
let mut temp_cleanups = ~[];
let Result {bcx, val} = trans_self_arg(bcx, this, &mut temp_cleanups, mentry);
Callee {
bcx: bcx,
data: Method(MethodData {
llfn: callee_fn.llfn,
llself: val,
temp_cleanup: temp_cleanups.head_opt().map(|&v| *v),
self_ty: node_id_type(bcx, this.id),
self_mode: mentry.self_mode,
explicit_self: mentry.explicit_self
Expand Down Expand Up @@ -254,9 +248,8 @@ pub fn trans_method_callee(bcx: block,
store,
mentry.explicit_self)
}
typeck::method_super(*) => {
fail!("method_super should have been handled \
above")
typeck::method_super(*) => {
fail!("method_super should have been handled above")
}
}
}
Expand Down Expand Up @@ -413,8 +406,9 @@ pub fn trans_monomorphized_callee(bcx: block,
bcx.ccx(), impl_did, mname);

// obtain the `self` value:
let mut temp_cleanups = ~[];
let Result {bcx, val: llself_val} =
trans_self_arg(bcx, base, mentry);
trans_self_arg(bcx, base, &mut temp_cleanups, mentry);

// create a concatenated set of substitutions which includes
// those from the impl and those from the method:
Expand All @@ -441,6 +435,7 @@ pub fn trans_monomorphized_callee(bcx: block,
data: Method(MethodData {
llfn: llfn_val,
llself: llself_val,
temp_cleanup: temp_cleanups.head_opt().map(|&v| *v),
self_ty: node_id_type(bcx, base.id),
self_mode: mentry.self_mode,
explicit_self: mentry.explicit_self
Expand Down Expand Up @@ -636,6 +631,7 @@ pub fn trans_trait_callee_from_llval(bcx: block,
data: Method(MethodData {
llfn: mptr,
llself: llself,
temp_cleanup: None,
self_ty: ty::mk_opaque_box(bcx.tcx()),
self_mode: ty::ByRef,
explicit_self: explicit_self
Expand Down

0 comments on commit ba922a4

Please sign in to comment.