From 11f7cb26c27c3eb3af3f7ef4f9cdb85327e6e030 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 7 May 2013 11:41:27 -0400 Subject: [PATCH] When autoborrowing a fn in trans, adjust the type of the datum to be `&fn`. Fixes #6141. --- src/librustc/middle/trans/datum.rs | 8 +-- src/librustc/middle/trans/expr.rs | 65 ++++++++++--------- src/librustc/middle/ty.rs | 8 +-- src/librustc/middle/typeck/check/regionck.rs | 25 +++++-- src/rt/rust_task.cpp | 11 +--- .../run-pass/issue-6141-leaking-owned-fn.rs | 8 +++ 6 files changed, 71 insertions(+), 54 deletions(-) create mode 100644 src/test/run-pass/issue-6141-leaking-owned-fn.rs diff --git a/src/librustc/middle/trans/datum.rs b/src/librustc/middle/trans/datum.rs index c19650e3b6848..3b885ae9b94ab 100644 --- a/src/librustc/middle/trans/datum.rs +++ b/src/librustc/middle/trans/datum.rs @@ -548,11 +548,9 @@ pub impl Datum { } fn to_rptr(&self, bcx: block) -> Datum { - //! - // - // Returns a new datum of region-pointer type containing the - // the same ptr as this datum (after converting to by-ref - // using `to_ref_llval()`). + //! Returns a new datum of region-pointer type containing the + //! the same ptr as this datum (after converting to by-ref + //! using `to_ref_llval()`). // Convert to ref, yielding lltype *T. Then create a Rust // type &'static T (which translates to *T). Construct new diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs index 0e8b2e0474661..2c59d5c6bd635 100644 --- a/src/librustc/middle/trans/expr.rs +++ b/src/librustc/middle/trans/expr.rs @@ -183,30 +183,25 @@ fn drop_and_cancel_clean(bcx: block, dat: Datum) -> block { pub fn trans_to_datum(bcx: block, expr: @ast::expr) -> DatumBlock { debug!("trans_to_datum(expr=%s)", bcx.expr_to_str(expr)); - return match bcx.tcx().adjustments.find(&expr.id) { - None => { - trans_to_datum_unadjusted(bcx, expr) - } - Some(&@AutoAddEnv(*)) => { - let mut bcx = bcx; - let datum = unpack_datum!(bcx, { - trans_to_datum_unadjusted(bcx, expr) - }); - add_env(bcx, expr, datum) - } - Some(&@AutoDerefRef(ref adj)) => { - let mut bcx = bcx; - let mut datum = unpack_datum!(bcx, { - trans_to_datum_unadjusted(bcx, expr) - }); - - debug!("unadjusted datum: %s", datum.to_str(bcx.ccx())); + let mut bcx = bcx; + let mut datum = unpack_datum!(bcx, trans_to_datum_unadjusted(bcx, expr)); + let adjustment = match bcx.tcx().adjustments.find_copy(&expr.id) { + None => { return DatumBlock {bcx: bcx, datum: datum}; } + Some(adj) => { adj } + }; + debug!("unadjusted datum: %s", datum.to_str(bcx.ccx())); + match *adjustment { + AutoAddEnv(*) => { + datum = unpack_datum!(bcx, add_env(bcx, expr, datum)); + } + AutoDerefRef(ref adj) => { if adj.autoderefs > 0 { - let DatumBlock { bcx: new_bcx, datum: new_datum } = - datum.autoderef(bcx, expr.span, expr.id, adj.autoderefs); - datum = new_datum; - bcx = new_bcx; + datum = + unpack_datum!( + bcx, + datum.autoderef(bcx, expr.span, + expr.id, adj.autoderefs)); } datum = match adj.autoref { @@ -224,23 +219,31 @@ pub fn trans_to_datum(bcx: block, expr: @ast::expr) -> DatumBlock { unpack_datum!(bcx, auto_slice_and_ref(bcx, expr, datum)) } Some(AutoBorrowFn(*)) => { - // currently, all closure types are - // represented precisely the same, so no - // runtime adjustment is required: - datum + let adjusted_ty = ty::adjust_ty(bcx.tcx(), expr.span, + datum.ty, Some(adjustment)); + unpack_datum!(bcx, auto_borrow_fn(bcx, adjusted_ty, datum)) } }; - - debug!("after adjustments, datum=%s", datum.to_str(bcx.ccx())); - - return DatumBlock {bcx: bcx, datum: datum}; } - }; + } + debug!("after adjustments, datum=%s", datum.to_str(bcx.ccx())); + return DatumBlock {bcx: bcx, datum: datum}; fn auto_ref(bcx: block, datum: Datum) -> DatumBlock { DatumBlock {bcx: bcx, datum: datum.to_rptr(bcx)} } + fn auto_borrow_fn(bcx: block, + adjusted_ty: ty::t, + datum: Datum) -> DatumBlock { + // Currently, all closure types are represented precisely the + // same, so no runtime adjustment is required, but we still + // must patchup the type. + DatumBlock {bcx: bcx, + datum: Datum {val: datum.val, ty: adjusted_ty, + mode: datum.mode, source: datum.source}} + } + fn auto_slice(bcx: block, expr: @ast::expr, datum: Datum) -> DatumBlock { // This is not the most efficient thing possible; since slices // are two words it'd be better if this were compiled in diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 892635416c2a4..b6e024b011e25 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -2893,20 +2893,20 @@ pub fn expr_ty_adjusted(cx: ctxt, expr: @ast::expr) -> t { */ let unadjusted_ty = expr_ty(cx, expr); - adjust_ty(cx, expr.span, unadjusted_ty, cx.adjustments.find(&expr.id)) + adjust_ty(cx, expr.span, unadjusted_ty, cx.adjustments.find_copy(&expr.id)) } pub fn adjust_ty(cx: ctxt, span: span, unadjusted_ty: ty::t, - adjustment: Option<&@AutoAdjustment>) -> ty::t + adjustment: Option<@AutoAdjustment>) -> ty::t { /*! See `expr_ty_adjusted` */ return match adjustment { None => unadjusted_ty, - Some(&@AutoAddEnv(r, s)) => { + Some(@AutoAddEnv(r, s)) => { match ty::get(unadjusted_ty).sty { ty::ty_bare_fn(ref b) => { ty::mk_closure( @@ -2924,7 +2924,7 @@ pub fn adjust_ty(cx: ctxt, } } - Some(&@AutoDerefRef(ref adj)) => { + Some(@AutoDerefRef(ref adj)) => { let mut adjusted_ty = unadjusted_ty; for uint::range(0, adj.autoderefs) |i| { diff --git a/src/librustc/middle/typeck/check/regionck.rs b/src/librustc/middle/typeck/check/regionck.rs index 6db6b7556aee4..2274259f18c19 100644 --- a/src/librustc/middle/typeck/check/regionck.rs +++ b/src/librustc/middle/typeck/check/regionck.rs @@ -118,6 +118,7 @@ pub impl Rcx { } /// Try to resolve the type for the given node. + #[config(stage0)] fn resolve_expr_type_adjusted(@mut self, expr: @ast::expr) -> ty::t { let ty_unadjusted = self.resolve_node_type(expr.id); if ty::type_is_error(ty_unadjusted) || ty::type_is_bot(ty_unadjusted) { @@ -125,15 +126,29 @@ pub impl Rcx { } else { let tcx = self.fcx.tcx(); let adjustments = self.fcx.inh.adjustments; - match adjustments.find(&expr.id) { + match adjustments.find_copy(&expr.id) { None => ty_unadjusted, - Some(&adjustment) => { - // FIXME(#3850) --- avoid region scoping errors - ty::adjust_ty(tcx, expr.span, ty_unadjusted, Some(&adjustment)) + Some(adjustment) => { + ty::adjust_ty(tcx, expr.span, ty_unadjusted, + Some(adjustment)) } } } } + + /// Try to resolve the type for the given node. + #[config(not(stage0))] + fn resolve_expr_type_adjusted(@mut self, expr: @ast::expr) -> ty::t { + let ty_unadjusted = self.resolve_node_type(expr.id); + if ty::type_is_error(ty_unadjusted) || ty::type_is_bot(ty_unadjusted) { + ty_unadjusted + } else { + let tcx = self.fcx.tcx(); + let adjustments = self.fcx.inh.adjustments; + ty::adjust_ty(tcx, expr.span, ty_unadjusted, + adjustments.find_copy(&expr.id)) + } + } } pub fn regionck_expr(fcx: @mut FnCtxt, e: @ast::expr) { @@ -650,7 +665,7 @@ fn constrain_regions_in_type_of_node( // is going to fail anyway, so just stop here and let typeck // report errors later on in the writeback phase. let ty0 = rcx.resolve_node_type(id); - let adjustment = rcx.fcx.inh.adjustments.find(&id); + let adjustment = rcx.fcx.inh.adjustments.find_copy(&id); let ty = ty::adjust_ty(tcx, span, ty0, adjustment); debug!("constrain_regions_in_type_of_node(\ ty=%s, ty0=%s, id=%d, minimum_lifetime=%?, adjustment=%?)", diff --git a/src/rt/rust_task.cpp b/src/rt/rust_task.cpp index 23e705357685d..266c0652c6e59 100644 --- a/src/rt/rust_task.cpp +++ b/src/rt/rust_task.cpp @@ -76,15 +76,8 @@ rust_task::delete_this() assert(ref_count == 0); // || // (ref_count == 1 && this == sched->root_task)); - if (borrow_list) { - // NOTE should free borrow_list from within rust code! - // If there is a pointer in there, it is a ~[BorrowRecord] pointer, - // which are currently allocated with LIBC malloc/free. But this is - // not really the right way to do this, we should be freeing this - // pointer from Rust code. - free(borrow_list); - borrow_list = NULL; - } + // The borrow list should be freed in the task annihilator + assert(!borrow_list); sched_loop->release_task(this); } diff --git a/src/test/run-pass/issue-6141-leaking-owned-fn.rs b/src/test/run-pass/issue-6141-leaking-owned-fn.rs new file mode 100644 index 0000000000000..fe11bb0a972ad --- /dev/null +++ b/src/test/run-pass/issue-6141-leaking-owned-fn.rs @@ -0,0 +1,8 @@ +fn run(f: &fn()) { + f() +} + +fn main() { + let f: ~fn() = || (); + run(f); +} \ No newline at end of file