Skip to content

Commit

Permalink
Fixes incorrect handling of ADT's drop requirements
Browse files Browse the repository at this point in the history
  • Loading branch information
JakobDegen committed Oct 23, 2021
1 parent 0119879 commit 9158fc2
Showing 1 changed file with 54 additions and 41 deletions.
95 changes: 54 additions & 41 deletions compiler/rustc_ty_utils/src/needs_drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@ use rustc_span::{sym, DUMMY_SP};
type NeedsDropResult<T> = Result<T, AlwaysRequiresDrop>;

fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
let adt_components =
move |adt_def: &ty::AdtDef, _| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter());

// If we don't know a type doesn't need drop, for example if it's a type
// parameter without a `Copy` bound, then we conservatively return that it
// needs drop.
let res =
NeedsDropTypes::new(tcx, query.param_env, query.value, adt_components).next().is_some();
let adt_has_dtor =
|adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
let res = drop_tys_helper(tcx, query.value, query.param_env, adt_has_dtor).next().is_some();

debug!("needs_drop_raw({:?}) = {:?}", query, res);
res
Expand All @@ -29,12 +27,10 @@ fn has_significant_drop_raw<'tcx>(
tcx: TyCtxt<'tcx>,
query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> bool {
let significant_drop_fields = move |adt_def: &ty::AdtDef, _| {
tcx.adt_significant_drop_tys(adt_def.did).map(|tys| tys.iter())
};
let res = NeedsDropTypes::new(tcx, query.param_env, query.value, significant_drop_fields)
.next()
.is_some();
let res =
drop_tys_helper(tcx, query.value, query.param_env, adt_consider_insignificant_dtor(tcx))
.next()
.is_some();
debug!("has_significant_drop_raw({:?}) = {:?}", query, res);
res
}
Expand Down Expand Up @@ -140,15 +136,14 @@ where
// `ManuallyDrop`. If it's a struct or enum without a `Drop`
// impl then check whether the field types need `Drop`.
ty::Adt(adt_def, substs) => {
debug!("Got value {:?} with substs {:?}", adt_def, substs);
let tys = match (self.adt_components)(adt_def, substs) {
Err(e) => return Some(Err(e)),
Ok(tys) => tys,
};
for required_ty in tys {
let subst_ty = tcx.normalize_erasing_regions(
self.param_env,
required_ty.subst(tcx, substs),
);
let subst_ty =
tcx.normalize_erasing_regions(self.param_env, required_ty);
queue_type(self, subst_ty);
}
}
Expand Down Expand Up @@ -187,11 +182,12 @@ enum DtorType {
// Depending on the implentation of `adt_has_dtor`, it is used to check if the
// ADT has a destructor or if the ADT only has a significant destructor. For
// understanding significant destructor look at `adt_significant_drop_tys`.
fn adt_drop_tys_helper<'tcx>(
fn drop_tys_helper<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: DefId,
ty: Ty<'tcx>,
param_env: rustc_middle::ty::ParamEnv<'tcx>,
adt_has_dtor: impl Fn(&ty::AdtDef) -> Option<DtorType>,
) -> Result<&ty::List<Ty<'tcx>>, AlwaysRequiresDrop> {
) -> impl Iterator<Item = NeedsDropResult<Ty<'tcx>>> {
let adt_components = move |adt_def: &ty::AdtDef, substs: SubstsRef<'tcx>| {
if adt_def.is_manually_drop() {
debug!("adt_drop_tys: `{:?}` is manually drop", adt_def);
Expand All @@ -215,31 +211,25 @@ fn adt_drop_tys_helper<'tcx>(
debug!("adt_drop_tys: `{:?}` is a union", adt_def);
return Ok(Vec::new().into_iter());
}
Ok(adt_def.all_fields().map(|field| tcx.type_of(field.did)).collect::<Vec<_>>().into_iter())
debug!("Path");
Ok(adt_def
.all_fields()
.map(|field| {
let r = tcx.type_of(field.did).subst(tcx, substs);
debug!("Subst into {:?} with {:?} gettng {:?}", field, substs, r);
r
})
.collect::<Vec<_>>()
.into_iter())
};

let adt_ty = tcx.type_of(def_id);
let param_env = tcx.param_env(def_id);
let res: Result<Vec<_>, _> =
NeedsDropTypes::new(tcx, param_env, adt_ty, adt_components).collect();

debug!("adt_drop_tys(`{}`) = `{:?}`", tcx.def_path_str(def_id), res);
res.map(|components| tcx.intern_type_list(&components))
NeedsDropTypes::new(tcx, param_env, ty, adt_components)
}

fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
// This is for the "needs_drop" query, that considers all `Drop` impls, therefore all dtors are
// significant.
let adt_has_dtor =
|adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
}

fn adt_significant_drop_tys(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
let adt_has_dtor = |adt_def: &ty::AdtDef| {
fn adt_consider_insignificant_dtor<'tcx>(
tcx: TyCtxt<'tcx>,
) -> impl Fn(&ty::AdtDef) -> Option<DtorType> + 'tcx {
move |adt_def: &ty::AdtDef| {
let is_marked_insig = tcx.has_attr(adt_def.did, sym::rustc_insignificant_dtor);
if is_marked_insig {
// In some cases like `std::collections::HashMap` where the struct is a wrapper around
Expand All @@ -256,8 +246,31 @@ fn adt_significant_drop_tys(
// treat this as the simple case of Drop impl for type.
None
}
};
adt_drop_tys_helper(tcx, def_id, adt_has_dtor)
}
}

fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
// This is for the "adt_drop_tys" query, that considers all `Drop` impls, therefore all dtors are
// significant.
let adt_has_dtor =
|adt_def: &ty::AdtDef| adt_def.destructor(tcx).map(|_| DtorType::Significant);
drop_tys_helper(tcx, tcx.type_of(def_id), tcx.param_env(def_id), adt_has_dtor)
.collect::<Result<Vec<_>, _>>()
.map(|components| tcx.intern_type_list(&components))
}

fn adt_significant_drop_tys(
tcx: TyCtxt<'_>,
def_id: DefId,
) -> Result<&ty::List<Ty<'_>>, AlwaysRequiresDrop> {
drop_tys_helper(
tcx,
tcx.type_of(def_id),
tcx.param_env(def_id),
adt_consider_insignificant_dtor(tcx),
)
.collect::<Result<Vec<_>, _>>()
.map(|components| tcx.intern_type_list(&components))
}

pub(crate) fn provide(providers: &mut ty::query::Providers) {
Expand Down

0 comments on commit 9158fc2

Please sign in to comment.