Skip to content

Commit

Permalink
rustc: combine type_needs_drop_given_env and may_drop into needs_drop.
Browse files Browse the repository at this point in the history
  • Loading branch information
eddyb committed Apr 20, 2017
1 parent 9ad3b94 commit 0adfd81
Show file tree
Hide file tree
Showing 10 changed files with 68 additions and 64 deletions.
37 changes: 2 additions & 35 deletions src/librustc/ty/mod.rs
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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) {
Expand Down
78 changes: 58 additions & 20 deletions src/librustc/ty/util.rs
Expand Up @@ -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<Ty<'tcx>>)
-> 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<Ty<'tcx>>)
-> 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<Ty<'tcx>>)
-> bool {
assert!(!self.needs_infer());

let result = match self.sty {
Expand All @@ -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,

Expand All @@ -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
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_borrowck/borrowck/mir/mod.rs
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/builtin.rs
Expand Up @@ -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, \
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/hair/cx/mod.rs
Expand Up @@ -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> {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/inline.rs
Expand Up @@ -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, &param_env) {
if ty.needs_drop(tcx, &param_env) {
cost += CALL_PENALTY;
if let Some(unwind) = unwind {
work_list.push(unwind);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/transform/qualify_consts.rs
Expand Up @@ -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;
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_mir/util/elaborate_drops.rs
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_passes/consts.rs
Expand Up @@ -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;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/context.rs
Expand Up @@ -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 {
Expand Down

0 comments on commit 0adfd81

Please sign in to comment.