diff --git a/src/librustc/middle/ty/mod.rs b/src/librustc/middle/ty/mod.rs index 5daa9bcd0d1ae..47e7eee469660 100644 --- a/src/librustc/middle/ty/mod.rs +++ b/src/librustc/middle/ty/mod.rs @@ -46,7 +46,7 @@ use std::vec::IntoIter; use std::collections::{HashMap, HashSet}; use syntax::ast::{self, CrateNum, Name, NodeId}; use syntax::attr::{self, AttrMetaMethods}; -use syntax::codemap::Span; +use syntax::codemap::{DUMMY_SP, Span}; use syntax::parse::token::{InternedString, special_idents}; use rustc_front::hir; @@ -2394,6 +2394,39 @@ impl<'tcx> ctxt<'tcx> { || self.sess.cstore.item_super_predicates(self, did)) } + /// 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_given_env<'a>(&self, + 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.moves_by_default(param_env, DUMMY_SP); + + 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(self); + debug!("type_needs_drop ty={:?} contents={:?}", ty, contents); + contents.needs_drop(self) + } + /// Get the attributes of a definition. pub fn get_attrs(&self, did: DefId) -> Cow<'tcx, [ast::Attribute]> { if let Some(id) = self.map.as_local_node_id(did) { diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 4d136d265e5f4..b5e029ed94bd1 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -248,7 +248,7 @@ impl<'a,'tcx> Builder<'a,'tcx> { kind: DropKind, lvalue: &Lvalue<'tcx>, lvalue_ty: Ty<'tcx>) { - if self.hir.needs_drop(lvalue_ty, span) { + if self.hir.needs_drop(lvalue_ty) { match self.scopes.iter_mut().rev().find(|s| s.extent == extent) { Some(scope) => { scope.drops.push((kind, span, lvalue.clone())); diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs index f2bc5fec2ff5d..07d32240d4330 100644 --- a/src/librustc_mir/hair/cx/mod.rs +++ b/src/librustc_mir/hair/cx/mod.rs @@ -90,17 +90,8 @@ impl<'a,'tcx:'a> Cx<'a, 'tcx> { .collect() } - pub fn needs_drop(&mut self, ty: Ty<'tcx>, span: Span) -> bool { - if self.infcx.type_moves_by_default(ty, span) { - // FIXME(#21859) we should do an add'l check here to determine if - // any dtor will execute, but the relevant fn - // (`type_needs_drop`) is currently factored into - // `librustc_trans`, so we can't easily do so. - true - } else { - // if type implements Copy, cannot require drop - false - } + pub fn needs_drop(&mut self, ty: Ty<'tcx>) -> bool { + self.tcx.type_needs_drop_given_env(ty, &self.infcx.parameter_environment) } pub fn span_bug(&mut self, span: Span, message: &str) -> ! { diff --git a/src/librustc_trans/trans/common.rs b/src/librustc_trans/trans/common.rs index fa500ab93552c..964e981aec093 100644 --- a/src/librustc_trans/trans/common.rs +++ b/src/librustc_trans/trans/common.rs @@ -73,45 +73,6 @@ pub fn type_is_fat_ptr<'tcx>(cx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool { } } -/// 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 { - type_needs_drop_given_env(cx, ty, &cx.empty_parameter_environment()) -} - -/// 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.moves_by_default(param_env, DUMMY_SP); - - 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); - debug!("type_needs_drop ty={:?} contents={:?}", ty, contents); - contents.needs_drop(cx) -} - fn type_is_newtype_immediate<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool { match ty.sty { ty::TyStruct(def, substs) => { @@ -518,7 +479,7 @@ impl<'a, 'tcx> FunctionContext<'a, 'tcx> { /// 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) + self.ccx.tcx().type_needs_drop_given_env(ty, &self.param_env) } pub fn eh_personality(&self) -> ValueRef { diff --git a/src/librustc_trans/trans/glue.rs b/src/librustc_trans/trans/glue.rs index 3fc9bc0d66d3d..a1165ffe171d0 100644 --- a/src/librustc_trans/trans/glue.rs +++ b/src/librustc_trans/trans/glue.rs @@ -88,6 +88,10 @@ pub fn trans_exchange_free_ty<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, } } +fn type_needs_drop<'tcx>(tcx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool { + tcx.type_needs_drop_given_env(ty, &tcx.empty_parameter_environment()) +} + pub fn get_drop_glue_type<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>) -> Ty<'tcx> { let tcx = ccx.tcx(); @@ -106,11 +110,11 @@ pub fn get_drop_glue_type<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // returned `tcx.types.i8` does not appear unsound. The impact on // code quality is unknown at this time.) - if !type_needs_drop(tcx, t) { + if !type_needs_drop(&tcx, t) { return tcx.types.i8; } match t.sty { - ty::TyBox(typ) if !type_needs_drop(tcx, typ) + ty::TyBox(typ) if !type_needs_drop(&tcx, typ) && type_is_sized(tcx, typ) => { let llty = sizing_type_of(ccx, typ); // `Box` does not allocate.