From 0adfd810f8aa16ce9e63f6feae182db0ba4833ef Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Tue, 18 Apr 2017 00:22:55 +0300 Subject: [PATCH] rustc: combine type_needs_drop_given_env and may_drop into needs_drop. --- src/librustc/ty/mod.rs | 37 +--------- src/librustc/ty/util.rs | 78 +++++++++++++++----- src/librustc_borrowck/borrowck/mir/mod.rs | 2 +- src/librustc_lint/builtin.rs | 2 +- src/librustc_mir/hair/cx/mod.rs | 2 +- src/librustc_mir/transform/inline.rs | 2 +- src/librustc_mir/transform/qualify_consts.rs | 2 +- src/librustc_mir/util/elaborate_drops.rs | 3 +- src/librustc_passes/consts.rs | 2 +- src/librustc_trans/context.rs | 2 +- 10 files changed, 68 insertions(+), 64 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index b6e47568ff4db..5c0889976c21a 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -425,8 +425,8 @@ bitflags! { const MOVES_BY_DEFAULT = 1 << 19, const FREEZENESS_CACHED = 1 << 20, const IS_FREEZE = 1 << 21, - const MAY_DROP_CACHED = 1 << 22, - const MAY_DROP = 1 << 23, + const NEEDS_DROP_CACHED = 1 << 22, + const NEEDS_DROP = 1 << 23, } } @@ -2381,39 +2381,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { Some(self.item_mir(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(self, - ty: Ty<'gcx>, - param_env: &ty::ParameterEnvironment<'gcx>) -> 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 tcx = self.global_tcx(); - let implements_copy = !ty.moves_by_default(tcx, param_env, DUMMY_SP); - - if implements_copy { return false; } - - // ... (issue #22536 continued) but as an optimization, still use - // prior logic of asking for the structural `may_drop`. - - // FIXME(#22815): Note that calling `ty::may_drop` is a - // conservative heuristic; it may report `true` ("may drop") - // 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. - - let may_drop = ty.may_drop(tcx); - debug!("type_needs_drop ty={:?} may_drop={:?}", ty, may_drop); - may_drop - } - /// Get the attributes of a definition. pub fn get_attrs(self, did: DefId) -> Cow<'gcx, [ast::Attribute]> { if let Some(id) = self.hir.as_local_node_id(did) { diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index a3d2518909d74..49d79f6545e2d 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -699,31 +699,52 @@ impl<'a, 'tcx> ty::TyS<'tcx> { result } + /// If `ty.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 `needs_drop` will definitely return `true` for `ty`.) #[inline] - pub fn may_drop(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { - if self.flags.get().intersects(TypeFlags::MAY_DROP_CACHED) { - return self.flags.get().intersects(TypeFlags::MAY_DROP); + pub fn needs_drop(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>, + param_env: &ty::ParameterEnvironment<'tcx>) -> bool { + if self.flags.get().intersects(TypeFlags::NEEDS_DROP_CACHED) { + return self.flags.get().intersects(TypeFlags::NEEDS_DROP); } - self.may_drop_inner(tcx, &mut FxHashSet()) + self.needs_drop_uncached(tcx, param_env, &mut FxHashSet()) } - fn may_drop_inner(&'tcx self, - tcx: TyCtxt<'a, 'tcx, 'tcx>, - visited: &mut FxHashSet>) - -> bool { - if self.flags.get().intersects(TypeFlags::MAY_DROP_CACHED) { - return self.flags.get().intersects(TypeFlags::MAY_DROP); + fn needs_drop_inner(&'tcx self, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + param_env: &ty::ParameterEnvironment<'tcx>, + stack: &mut FxHashSet>) + -> bool { + if self.flags.get().intersects(TypeFlags::NEEDS_DROP_CACHED) { + return self.flags.get().intersects(TypeFlags::NEEDS_DROP); } // This should be reported as an error by `check_representable`. // // Consider the type as not needing drop in the meanwhile to avoid // further errors. - if visited.replace(self).is_some() { + if let Some(_) = stack.replace(self) { return false; } + let needs_drop = self.needs_drop_uncached(tcx, param_env, stack); + + // "Pop" the cycle detection "stack". + stack.remove(self); + + needs_drop + } + + fn needs_drop_uncached(&'tcx self, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + param_env: &ty::ParameterEnvironment<'tcx>, + stack: &mut FxHashSet>) + -> bool { assert!(!self.needs_infer()); let result = match self.sty { @@ -733,6 +754,21 @@ impl<'a, 'tcx> ty::TyS<'tcx> { ty::TyFnDef(..) | ty::TyFnPtr(_) | ty::TyChar | ty::TyRawPtr(_) | ty::TyRef(..) | ty::TyStr => false, + // 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) ... + _ if !self.moves_by_default(tcx, param_env, DUMMY_SP) => false, + + // ... (issue #22536 continued) but as an optimization, still use + // prior logic of asking for the structural "may drop". + + // FIXME(#22815): Note that this is a conservative heuristic; + // it may report that the type "may drop" when actual type does + // not actually have a destructor associated with it. But since + // the type absolutely did not have the `Copy` bound attached + // (see above), it is sound to treat it as having a destructor. + // User destructors are the only way to have concrete drop types. ty::TyAdt(def, _) if def.has_dtor(tcx) => true, @@ -743,16 +779,16 @@ impl<'a, 'tcx> ty::TyS<'tcx> { // Structural recursion. ty::TyArray(ty, _) | ty::TySlice(ty) => { - ty.may_drop_inner(tcx, visited) + ty.needs_drop_inner(tcx, param_env, stack) } ty::TyClosure(def_id, ref substs) => { substs.upvar_tys(def_id, tcx) - .any(|ty| ty.may_drop_inner(tcx, visited)) + .any(|ty| ty.needs_drop_inner(tcx, param_env, stack)) } ty::TyTuple(ref tys, _) => { - tys.iter().any(|ty| ty.may_drop_inner(tcx, visited)) + tys.iter().any(|ty| ty.needs_drop_inner(tcx, param_env, stack)) } // unions don't have destructors regardless of the child types @@ -761,17 +797,19 @@ impl<'a, 'tcx> ty::TyS<'tcx> { ty::TyAdt(def, substs) => { def.variants.iter().any(|v| { v.fields.iter().any(|f| { - f.ty(tcx, substs).may_drop_inner(tcx, visited) + f.ty(tcx, substs).needs_drop_inner(tcx, param_env, stack) }) }) } }; - self.flags.set(self.flags.get() | if result { - TypeFlags::MAY_DROP_CACHED | TypeFlags::MAY_DROP - } else { - TypeFlags::MAY_DROP_CACHED - }); + if !self.has_param_types() && !self.has_self_ty() { + self.flags.set(self.flags.get() | if result { + TypeFlags::NEEDS_DROP_CACHED | TypeFlags::NEEDS_DROP + } else { + TypeFlags::NEEDS_DROP_CACHED + }); + } result } diff --git a/src/librustc_borrowck/borrowck/mir/mod.rs b/src/librustc_borrowck/borrowck/mir/mod.rs index dc01cbe5e7605..de5613dbfaa38 100644 --- a/src/librustc_borrowck/borrowck/mir/mod.rs +++ b/src/librustc_borrowck/borrowck/mir/mod.rs @@ -322,7 +322,7 @@ fn on_all_drop_children_bits<'a, 'tcx, F>( let ty = lvalue.ty(mir, tcx).to_ty(tcx); debug!("on_all_drop_children_bits({:?}, {:?} : {:?})", path, lvalue, ty); - if tcx.type_needs_drop_given_env(ty, &ctxt.param_env) { + if ty.needs_drop(tcx, &ctxt.param_env) { each_child(child); } else { debug!("on_all_drop_children_bits - skipping") diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 0ee9d4a42c7f8..1c69f3cff172a 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1152,7 +1152,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnionsWithDropFields { let param_env = &ty::ParameterEnvironment::for_item(ctx.tcx, item.id); for field in vdata.fields() { let field_ty = ctx.tcx.item_type(ctx.tcx.hir.local_def_id(field.id)); - if ctx.tcx.type_needs_drop_given_env(field_ty, param_env) { + if field_ty.needs_drop(ctx.tcx, param_env) { ctx.span_lint(UNIONS_WITH_DROP_FIELDS, field.span, "union contains a field with possibly non-trivial drop code, \ diff --git a/src/librustc_mir/hair/cx/mod.rs b/src/librustc_mir/hair/cx/mod.rs index 5f9fb8e1b120f..db9da2a280b94 100644 --- a/src/librustc_mir/hair/cx/mod.rs +++ b/src/librustc_mir/hair/cx/mod.rs @@ -168,7 +168,7 @@ impl<'a, 'gcx, 'tcx> Cx<'a, 'gcx, 'tcx> { type with inference types/regions", ty); }); - self.tcx.type_needs_drop_given_env(ty, &self.infcx.parameter_environment) + ty.needs_drop(self.tcx.global_tcx(), &self.infcx.parameter_environment) } pub fn tcx(&self) -> TyCtxt<'a, 'gcx, 'tcx> { diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index ac2bdaad24f76..892d67ac23725 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -357,7 +357,7 @@ impl<'a, 'tcx> Inliner<'a, 'tcx> { // a regular goto. let ty = location.ty(&callee_mir, tcx).subst(tcx, callsite.substs); let ty = ty.to_ty(tcx); - if tcx.type_needs_drop_given_env(ty, ¶m_env) { + if ty.needs_drop(tcx, ¶m_env) { cost += CALL_PENALTY; if let Some(unwind) = unwind { work_list.push(unwind); diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index cc7d25628432e..526c1488ab480 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -83,7 +83,7 @@ impl<'a, 'tcx> Qualif { if ty.is_freeze(tcx, param_env, DUMMY_SP) { *self = *self - Qualif::MUTABLE_INTERIOR; } - if !tcx.type_needs_drop_given_env(ty, param_env) { + if !ty.needs_drop(tcx, param_env) { *self = *self - Qualif::NEEDS_DROP; } } diff --git a/src/librustc_mir/util/elaborate_drops.rs b/src/librustc_mir/util/elaborate_drops.rs index 04a1fc891cf1e..07025fcfdb944 100644 --- a/src/librustc_mir/util/elaborate_drops.rs +++ b/src/librustc_mir/util/elaborate_drops.rs @@ -277,8 +277,7 @@ impl<'l, 'b, 'tcx, D> DropCtxt<'l, 'b, 'tcx, D> let mut fields = fields; fields.retain(|&(ref lvalue, _)| { - self.tcx().type_needs_drop_given_env( - self.lvalue_ty(lvalue), self.elaborator.param_env()) + self.lvalue_ty(lvalue).needs_drop(self.tcx(), self.elaborator.param_env()) }); debug!("drop_ladder - fields needing drop: {:?}", fields); diff --git a/src/librustc_passes/consts.rs b/src/librustc_passes/consts.rs index 535c6a7ab9e12..fdb6752213378 100644 --- a/src/librustc_passes/consts.rs +++ b/src/librustc_passes/consts.rs @@ -89,7 +89,7 @@ impl<'a, 'gcx> CheckCrateVisitor<'a, 'gcx> { self.promotable = false; } - if self.tcx.type_needs_drop_given_env(ty, &self.param_env) { + if ty.needs_drop(self.tcx, &self.param_env) { self.promotable = false; } } diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index fd9ff17cc6489..1d1921bf7b96d 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -392,7 +392,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { } pub fn type_needs_drop(&self, ty: Ty<'tcx>) -> bool { - self.tcx.type_needs_drop_given_env(ty, &self.empty_param_env) + ty.needs_drop(self.tcx, &self.empty_param_env) } pub fn type_is_sized(&self, ty: Ty<'tcx>) -> bool {