Use more precise type_needs_drop for deciding about emitting cleanup code.

Fix #22536
This commit is contained in:
Felix S. Klock II 2015-02-25 23:09:58 +01:00
parent 2d0e6caeec
commit 5e4867eef1
10 changed files with 69 additions and 17 deletions

View File

@ -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) {

View File

@ -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),

View File

@ -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(&param_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

View File

@ -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)

View File

@ -312,7 +312,7 @@ impl KindOps for Lvalue {
ty: Ty<'tcx>)
-> Block<'blk, 'tcx> {
let _icx = push_ctxt("<Lvalue as KindOps>::post_store");
if type_needs_drop(bcx.tcx(), ty) {
if bcx.fcx.type_needs_drop(ty) {
// cancel cleanup of affine values by zeroing out
let () = zero_mem(bcx, val, ty);
bcx
@ -657,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)

View File

@ -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);

View File

@ -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,

View File

@ -378,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);

View File

@ -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"));

View File

@ -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,