diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index e7af0be88a0..1ca243134cc 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -241,7 +241,12 @@ extern "rust-intrinsic" { /// will trigger a compiler error. pub fn return_address() -> *const u8; - /// Returns `true` if a type requires drop glue. + /// Returns `true` if the actual type given as `T` requires drop + /// glue; returns `false` if the actual type provided for `T` + /// implements `Copy`. + /// + /// If the actual type neither requires drop glue nor implements + /// `Copy`, then may return `true` or `false`. pub fn needs_drop() -> bool; /// Returns `true` if a type is managed (will be allocated on the local heap) diff --git a/src/librustc_trans/trans/_match.rs b/src/librustc_trans/trans/_match.rs index 9a121a8830b..48ff4c83320 100644 --- a/src/librustc_trans/trans/_match.rs +++ b/src/librustc_trans/trans/_match.rs @@ -1499,6 +1499,7 @@ pub fn store_local<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, fn create_dummy_locals<'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, pat: &ast::Pat) -> Block<'blk, 'tcx> { + let _icx = push_ctxt("create_dummy_locals"); // create dummy memory for the variables if we have no // value to store into them immediately let tcx = bcx.tcx(); diff --git a/src/librustc_trans/trans/callee.rs b/src/librustc_trans/trans/callee.rs index 59fcd5492eb..4f234fac9a4 100644 --- a/src/librustc_trans/trans/callee.rs +++ b/src/librustc_trans/trans/callee.rs @@ -734,7 +734,7 @@ pub fn trans_call_inner<'a, 'blk, 'tcx, F>(bcx: Block<'blk, 'tcx>, }; if !is_rust_fn || type_of::return_uses_outptr(ccx, ret_ty) || - common::type_needs_drop(bcx.tcx(), ret_ty) { + bcx.fcx.type_needs_drop(ret_ty) { // Push the out-pointer if we use an out-pointer for this // return type, otherwise push "undef". if common::type_is_zero_size(ccx, ret_ty) { diff --git a/src/librustc_trans/trans/cleanup.rs b/src/librustc_trans/trans/cleanup.rs index a3705a67cdc..ad07f3953cc 100644 --- a/src/librustc_trans/trans/cleanup.rs +++ b/src/librustc_trans/trans/cleanup.rs @@ -386,7 +386,7 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> { cleanup_scope: ScopeId, val: ValueRef, ty: Ty<'tcx>) { - if !common::type_needs_drop(self.ccx.tcx(), ty) { return; } + if !self.type_needs_drop(ty) { return; } let drop = box DropValue { is_immediate: false, must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty), @@ -408,7 +408,8 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> { cleanup_scope: ScopeId, val: ValueRef, ty: Ty<'tcx>) { - if !common::type_needs_drop(self.ccx.tcx(), ty) { return; } + if !self.type_needs_drop(ty) { return; } + let drop = box DropValue { is_immediate: false, must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty), @@ -432,7 +433,7 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> { val: ValueRef, ty: Ty<'tcx>) { - if !common::type_needs_drop(self.ccx.tcx(), ty) { return; } + if !self.type_needs_drop(ty) { return; } let drop = box DropValue { is_immediate: true, must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty), @@ -1007,6 +1008,7 @@ impl<'tcx> Cleanup<'tcx> for DropValue<'tcx> { bcx: Block<'blk, 'tcx>, debug_loc: DebugLoc) -> Block<'blk, 'tcx> { + let _icx = base::push_ctxt("::trans"); let bcx = if self.is_immediate { glue::drop_ty_immediate(bcx, self.val, self.ty, debug_loc) } else { diff --git a/src/librustc_trans/trans/common.rs b/src/librustc_trans/trans/common.rs index d8fc6df2685..ec7ed2fe890 100644 --- a/src/librustc_trans/trans/common.rs +++ b/src/librustc_trans/trans/common.rs @@ -213,8 +213,43 @@ pub fn type_needs_unwind_cleanup<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty< } } +/// If `type_needs_drop` returns true, then `ty` is definitely +/// non-copy and *might* have a destructor attached; if it returns +/// false, then `ty` definitely has no destructor (i.e. no drop glue). +/// +/// (Note that this implies that if `ty` has a destructor attached, +/// then `type_needs_drop` will definitely return `true` for `ty`.) pub fn type_needs_drop<'tcx>(cx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool { - ty::type_contents(cx, ty).needs_drop(cx) + type_needs_drop_given_env(cx, ty, &ty::empty_parameter_environment(cx)) +} + +/// Core implementation of type_needs_drop, potentially making use of +/// and/or updating caches held in the `param_env`. +fn type_needs_drop_given_env<'a,'tcx>(cx: &ty::ctxt<'tcx>, + ty: Ty<'tcx>, + param_env: &ty::ParameterEnvironment<'a,'tcx>) -> bool { + // Issue #22536: We first query type_moves_by_default. It sees a + // normalized version of the type, and therefore will definitely + // know whether the type implements Copy (and thus needs no + // cleanup/drop/zeroing) ... + let implements_copy = !ty::type_moves_by_default(¶m_env, DUMMY_SP, ty); + + if implements_copy { return false; } + + // ... (issue #22536 continued) but as an optimization, still use + // prior logic of asking if the `needs_drop` bit is set; we need + // not zero non-Copy types if they have no destructor. + + // FIXME(#22815): Note that calling `ty::type_contents` is a + // conservative heuristic; it may report that `needs_drop` is set + // when actual type does not actually have a destructor associated + // with it. But since `ty` absolutely did not have the `Copy` + // bound attached (see above), it is sound to treat it as having a + // destructor (e.g. zero its memory on move). + + let contents = ty::type_contents(cx, ty); + debug!("type_needs_drop ty={} contents={:?}", ty.repr(cx), contents); + contents.needs_drop(cx) } fn type_is_newtype_immediate<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool { @@ -534,6 +569,12 @@ impl<'a, 'tcx> FunctionContext<'a, 'tcx> { self.param_substs, value) } + + /// This is the same as `common::type_needs_drop`, except that it + /// may use or update caches within this `FunctionContext`. + pub fn type_needs_drop(&self, ty: Ty<'tcx>) -> bool { + type_needs_drop_given_env(self.ccx.tcx(), ty, &self.param_env) + } } // Basic block context. We create a block context for each basic block diff --git a/src/librustc_trans/trans/controlflow.rs b/src/librustc_trans/trans/controlflow.rs index ad96c506c9d..85d0bc0319f 100644 --- a/src/librustc_trans/trans/controlflow.rs +++ b/src/librustc_trans/trans/controlflow.rs @@ -77,7 +77,7 @@ pub fn trans_stmt_semi<'blk, 'tcx>(cx: Block<'blk, 'tcx>, e: &ast::Expr) -> Block<'blk, 'tcx> { let _icx = push_ctxt("trans_stmt_semi"); let ty = expr_ty(cx, e); - if type_needs_drop(cx.tcx(), ty) { + if cx.fcx.type_needs_drop(ty) { expr::trans_to_lvalue(cx, e, "stmt").bcx } else { expr::trans_into(cx, e, expr::Ignore) diff --git a/src/librustc_trans/trans/datum.rs b/src/librustc_trans/trans/datum.rs index 8262dbf55dd..6ca71254868 100644 --- a/src/librustc_trans/trans/datum.rs +++ b/src/librustc_trans/trans/datum.rs @@ -311,7 +311,8 @@ impl KindOps for Lvalue { val: ValueRef, ty: Ty<'tcx>) -> Block<'blk, 'tcx> { - if type_needs_drop(bcx.tcx(), ty) { + let _icx = push_ctxt("::post_store"); + if bcx.fcx.type_needs_drop(ty) { // cancel cleanup of affine values by zeroing out let () = zero_mem(bcx, val, ty); bcx @@ -656,7 +657,7 @@ impl<'tcx, K: KindOps + fmt::Debug> Datum<'tcx, K> { /// scalar-ish (like an int or a pointer) which (1) does not require drop glue and (2) is /// naturally passed around by value, and not by reference. pub fn to_llscalarish<'blk>(self, bcx: Block<'blk, 'tcx>) -> ValueRef { - assert!(!type_needs_drop(bcx.tcx(), self.ty)); + assert!(!bcx.fcx.type_needs_drop(self.ty)); assert!(self.appropriate_rvalue_mode(bcx.ccx()) == ByValue); if self.kind.is_by_ref() { load_ty(bcx, self.val, self.ty) diff --git a/src/librustc_trans/trans/expr.rs b/src/librustc_trans/trans/expr.rs index 5cc1baf66c6..27f9b9506a5 100644 --- a/src/librustc_trans/trans/expr.rs +++ b/src/librustc_trans/trans/expr.rs @@ -974,7 +974,7 @@ fn trans_rvalue_stmt_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let src_datum = unpack_datum!(bcx, trans(bcx, &**src)); let dst_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, &**dst, "assign")); - if type_needs_drop(bcx.tcx(), dst_datum.ty) { + if bcx.fcx.type_needs_drop(dst_datum.ty) { // If there are destructors involved, make sure we // are copying from an rvalue, since that cannot possible // alias an lvalue. We are concerned about code like: @@ -1498,7 +1498,7 @@ pub fn trans_adt<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, assert_eq!(discr, 0); match ty::expr_kind(bcx.tcx(), &*base.expr) { - ty::RvalueDpsExpr | ty::RvalueDatumExpr if !type_needs_drop(bcx.tcx(), ty) => { + ty::RvalueDpsExpr | ty::RvalueDatumExpr if !bcx.fcx.type_needs_drop(ty) => { bcx = trans_into(bcx, &*base.expr, SaveIn(addr)); }, ty::RvalueStmtExpr => bcx.tcx().sess.bug("unexpected expr kind for struct base expr"), @@ -2116,7 +2116,7 @@ fn trans_assign_op<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, // Evaluate LHS (destination), which should be an lvalue let dst_datum = unpack_datum!(bcx, trans_to_lvalue(bcx, dst, "assign_op")); - assert!(!type_needs_drop(bcx.tcx(), dst_datum.ty)); + assert!(!bcx.fcx.type_needs_drop(dst_datum.ty)); let dst_ty = dst_datum.ty; let dst = load_ty(bcx, dst_datum.val, dst_datum.ty); diff --git a/src/librustc_trans/trans/glue.rs b/src/librustc_trans/trans/glue.rs index c14683aeade..9491c8377a6 100644 --- a/src/librustc_trans/trans/glue.rs +++ b/src/librustc_trans/trans/glue.rs @@ -99,6 +99,16 @@ pub fn get_drop_glue_type<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, if !type_is_sized(tcx, t) { return t } + + // FIXME (#22815): note that type_needs_drop conservatively + // approximates in some cases and may say a type expression + // requires drop glue when it actually does not. + // + // (In this case it is not clear whether any harm is done, i.e. + // erroneously returning `t` in some cases where we could have + // returned `tcx.types.i8` does not appear unsound. The impact on + // code quality is unknown at this time.) + if !type_needs_drop(tcx, t) { return tcx.types.i8; } @@ -125,7 +135,7 @@ pub fn drop_ty<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, // NB: v is an *alias* of type t here, not a direct value. debug!("drop_ty(t={})", t.repr(bcx.tcx())); let _icx = push_ctxt("drop_ty"); - if type_needs_drop(bcx.tcx(), t) { + if bcx.fcx.type_needs_drop(t) { let ccx = bcx.ccx(); let glue = get_drop_glue(ccx, t); let glue_type = get_drop_glue_type(ccx, t); @@ -480,7 +490,7 @@ fn make_drop_glue<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, v0: ValueRef, t: Ty<'tcx>) }, _ => { assert!(type_is_sized(bcx.tcx(), t)); - if type_needs_drop(bcx.tcx(), t) && ty::type_is_structural(t) { + if bcx.fcx.type_needs_drop(t) && ty::type_is_structural(t) { iter_structural_ty(bcx, v0, t, diff --git a/src/librustc_trans/trans/intrinsic.rs b/src/librustc_trans/trans/intrinsic.rs index 993c9eae45b..54644c92869 100644 --- a/src/librustc_trans/trans/intrinsic.rs +++ b/src/librustc_trans/trans/intrinsic.rs @@ -156,6 +156,8 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, let ccx = fcx.ccx; let tcx = bcx.tcx(); + let _icx = push_ctxt("trans_intrinsic_call"); + let ret_ty = match callee_ty.sty { ty::ty_bare_fn(_, ref f) => { ty::erase_late_bound_regions(bcx.tcx(), &f.sig.output()) @@ -376,7 +378,8 @@ pub fn trans_intrinsic_call<'a, 'blk, 'tcx>(mut bcx: Block<'blk, 'tcx>, } (_, "needs_drop") => { let tp_ty = *substs.types.get(FnSpace, 0); - C_bool(ccx, type_needs_drop(ccx.tcx(), tp_ty)) + + C_bool(ccx, bcx.fcx.type_needs_drop(tp_ty)) } (_, "owns_managed") => { let tp_ty = *substs.types.get(FnSpace, 0); diff --git a/src/librustc_trans/trans/meth.rs b/src/librustc_trans/trans/meth.rs index 3411f12886d..4423cd27744 100644 --- a/src/librustc_trans/trans/meth.rs +++ b/src/librustc_trans/trans/meth.rs @@ -454,7 +454,7 @@ fn trans_trait_callee<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let self_datum = unpack_datum!( bcx, expr::trans(bcx, self_expr)); - let llval = if type_needs_drop(bcx.tcx(), self_datum.ty) { + let llval = if bcx.fcx.type_needs_drop(self_datum.ty) { let self_datum = unpack_datum!( bcx, self_datum.to_rvalue_datum(bcx, "trait_callee")); diff --git a/src/librustc_trans/trans/tvec.rs b/src/librustc_trans/trans/tvec.rs index d3acd23e641..a5c3923336a 100644 --- a/src/librustc_trans/trans/tvec.rs +++ b/src/librustc_trans/trans/tvec.rs @@ -53,11 +53,10 @@ pub fn make_drop_glue_unboxed<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, let not_null = IsNotNull(bcx, vptr); with_cond(bcx, not_null, |bcx| { let ccx = bcx.ccx(); - let tcx = bcx.tcx(); let _icx = push_ctxt("tvec::make_drop_glue_unboxed"); let dataptr = get_dataptr(bcx, vptr); - let bcx = if type_needs_drop(tcx, unit_ty) { + let bcx = if bcx.fcx.type_needs_drop(unit_ty) { let len = get_len(bcx, vptr); iter_vec_raw(bcx, dataptr, diff --git a/src/test/run-pass/issue-22536-copy-mustnt-zero.rs b/src/test/run-pass/issue-22536-copy-mustnt-zero.rs new file mode 100644 index 00000000000..b3492180a58 --- /dev/null +++ b/src/test/run-pass/issue-22536-copy-mustnt-zero.rs @@ -0,0 +1,34 @@ +// Copyright 2015 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. + +// Regression test for Issue #22536: If a type implements Copy, then +// moving it must not zero the original memory. + +trait Resources { + type Buffer: Copy; + fn foo(&self) {} +} + +struct BufferHandle { + raw: ::Buffer, +} +impl Copy for BufferHandle {} + +enum Res {} +impl Resources for Res { + type Buffer = u32; +} +impl Copy for Res { } + +fn main() { + let b: BufferHandle = BufferHandle { raw: 1 }; + let c = b; + assert_eq!(c.raw, b.raw) +}