Skip to content

Commit f7772f9

Browse files
pnkfelixalexcrichton
authored andcommitted
dropck: must assume Box<Trait + 'a> has a destructor of interest.
Implements this (previously overlooked) note from [RFC 769]: > (Note: When encountering a D of the form `Box<Trait+'b>`, we > conservatively assume that such a type has a Drop implementation > parametric in 'b.) Fix #25199. [breaking-change] The breakage here falls into both obvious and non-obvious cases. The obvious case: if you were relying on the unsoundness this exposes (namely being able to reference dead storage from a destructor, by doing it via a boxed trait object bounded by the lifetime of the dead storage), then this change disallows that. The non-obvious cases: The way dropck works, it causes lifetimes to be extended to longer extents than they covered before. I.e. lifetimes that are attached as trait-bounds may become longer than they were previously. * This includes lifetimes that are only *implicitly* attached as trait-bounds (due to [RFC 599]). So you may have code that was e.g. taking a parameter of type `&'a Box<Trait>` (which expands to `&'a Box<Trait+'a>`), that now may need to be assigned type `&'a Box<Trait+'static>` to ensure that `'a` is not inadvertantly inferred to a region that is actually too long. (See earlier commit in this PR for an example of this.) [RFC 769]: https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#the-drop-check-rule [RFC 599]: https://github.com/rust-lang/rfcs/blob/master/text/0599-default-object-bound.md Conflicts: src/librustc_typeck/check/dropck.rs
1 parent 484eae4 commit f7772f9

File tree

1 file changed

+152
-10
lines changed

1 file changed

+152
-10
lines changed

src/librustc_typeck/check/dropck.rs

+152-10
Original file line numberDiff line numberDiff line change
@@ -396,19 +396,24 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
396396
}
397397
};
398398

399-
let opt_type_did = match typ.sty {
400-
ty::ty_struct(struct_did, _) => Some(struct_did),
401-
ty::ty_enum(enum_did, _) => Some(enum_did),
402-
_ => None,
399+
let dtor_kind = match typ.sty {
400+
ty::ty_enum(def_id, _) |
401+
ty::ty_struct(def_id, _) => {
402+
match destructor_for_type.get(&def_id) {
403+
Some(def_id) => DtorKind::KnownDropMethod(*def_id),
404+
None => DtorKind::PureRecur,
405+
}
406+
}
407+
ty::ty_trait(ref ty_trait) => {
408+
DtorKind::Unknown(ty_trait.bounds.clone())
409+
}
410+
_ => DtorKind::PureRecur,
403411
};
404412

405-
let opt_dtor =
406-
opt_type_did.and_then(|did| destructor_for_type.get(&did));
407-
408413
debug!("iterate_over_potentially_unsafe_regions_in_type \
409-
{}typ: {} scope: {:?} opt_dtor: {:?} xref: {}",
414+
{}typ: {} scope: {:?} xref: {}",
410415
(0..depth).map(|_| ' ').collect::<String>(),
411-
typ.repr(rcx.tcx()), scope, opt_dtor, xref_depth);
416+
typ.repr(rcx.tcx()), scope, xref_depth);
412417

413418
// If `typ` has a destructor, then we must ensure that all
414419
// borrowed data reachable via `typ` must outlive the parent
@@ -608,7 +613,7 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
608613

609614
ty::ty_rptr(..) | ty::ty_ptr(_) | ty::ty_bare_fn(..) => {
610615
// Don't recurse, since references, pointers,
611-
// boxes, and bare functions don't own instances
616+
// and bare functions don't own instances
612617
// of the types appearing within them.
613618
walker.skip_current_subtree();
614619
}
@@ -627,3 +632,140 @@ fn iterate_over_potentially_unsafe_regions_in_type<'a, 'tcx>(
627632

628633
return Ok(());
629634
}
635+
636+
enum DtorKind<'tcx> {
637+
// Type has an associated drop method with this def id
638+
KnownDropMethod(ast::DefId),
639+
640+
// Type has no destructor (or its dtor is known to be pure
641+
// with respect to lifetimes), though its *substructure*
642+
// may carry a destructor.
643+
PureRecur,
644+
645+
// Type may have impure destructor that is unknown;
646+
// e.g. `Box<Trait+'a>`
647+
Unknown(ty::ExistentialBounds<'tcx>),
648+
}
649+
650+
fn has_dtor_of_interest<'tcx>(tcx: &ty::ctxt<'tcx>,
651+
dtor_kind: DtorKind,
652+
typ: ty::Ty<'tcx>,
653+
span: Span) -> bool {
654+
let has_dtor_of_interest: bool;
655+
656+
match dtor_kind {
657+
DtorKind::PureRecur => {
658+
has_dtor_of_interest = false;
659+
debug!("typ: {} has no dtor, and thus is uninteresting",
660+
typ.repr(tcx));
661+
}
662+
DtorKind::Unknown(bounds) => {
663+
match bounds.region_bound {
664+
ty::ReStatic => {
665+
debug!("trait: {} has 'static bound, and thus is uninteresting",
666+
typ.repr(tcx));
667+
has_dtor_of_interest = false;
668+
}
669+
ty::ReEmpty => {
670+
debug!("trait: {} has empty region bound, and thus is uninteresting",
671+
typ.repr(tcx));
672+
has_dtor_of_interest = false;
673+
}
674+
r => {
675+
debug!("trait: {} has non-static bound: {}; assumed interesting",
676+
typ.repr(tcx), r.repr(tcx));
677+
has_dtor_of_interest = true;
678+
}
679+
}
680+
}
681+
DtorKind::KnownDropMethod(dtor_method_did) => {
682+
let impl_did = ty::impl_of_method(tcx, dtor_method_did)
683+
.unwrap_or_else(|| {
684+
tcx.sess.span_bug(
685+
span, "no Drop impl found for drop method")
686+
});
687+
688+
let dtor_typescheme = ty::lookup_item_type(tcx, impl_did);
689+
let dtor_generics = dtor_typescheme.generics;
690+
691+
let mut has_pred_of_interest = false;
692+
693+
let mut seen_items = Vec::new();
694+
let mut items_to_inspect = vec![impl_did];
695+
'items: while let Some(item_def_id) = items_to_inspect.pop() {
696+
if seen_items.contains(&item_def_id) {
697+
continue;
698+
}
699+
700+
for pred in ty::lookup_predicates(tcx, item_def_id).predicates {
701+
let result = match pred {
702+
ty::Predicate::Equate(..) |
703+
ty::Predicate::RegionOutlives(..) |
704+
ty::Predicate::TypeOutlives(..) |
705+
ty::Predicate::Projection(..) => {
706+
// For now, assume all these where-clauses
707+
// may give drop implementation capabilty
708+
// to access borrowed data.
709+
true
710+
}
711+
712+
ty::Predicate::Trait(ty::Binder(ref t_pred)) => {
713+
let def_id = t_pred.trait_ref.def_id;
714+
if ty::trait_items(tcx, def_id).len() != 0 {
715+
// If trait has items, assume it adds
716+
// capability to access borrowed data.
717+
true
718+
} else {
719+
// Trait without items is itself
720+
// uninteresting from POV of dropck.
721+
//
722+
// However, may have parent w/ items;
723+
// so schedule checking of predicates,
724+
items_to_inspect.push(def_id);
725+
// and say "no capability found" for now.
726+
false
727+
}
728+
}
729+
};
730+
731+
if result {
732+
has_pred_of_interest = true;
733+
debug!("typ: {} has interesting dtor due to generic preds, e.g. {}",
734+
typ.repr(tcx), pred.repr(tcx));
735+
break 'items;
736+
}
737+
}
738+
739+
seen_items.push(item_def_id);
740+
}
741+
742+
// In `impl<'a> Drop ...`, we automatically assume
743+
// `'a` is meaningful and thus represents a bound
744+
// through which we could reach borrowed data.
745+
//
746+
// FIXME (pnkfelix): In the future it would be good to
747+
// extend the language to allow the user to express,
748+
// in the impl signature, that a lifetime is not
749+
// actually used (something like `where 'a: ?Live`).
750+
let has_region_param_of_interest =
751+
dtor_generics.has_region_params(subst::TypeSpace);
752+
753+
has_dtor_of_interest =
754+
has_region_param_of_interest ||
755+
has_pred_of_interest;
756+
757+
if has_dtor_of_interest {
758+
debug!("typ: {} has interesting dtor, due to \
759+
region params: {} or pred: {}",
760+
typ.repr(tcx),
761+
has_region_param_of_interest,
762+
has_pred_of_interest);
763+
} else {
764+
debug!("typ: {} has dtor, but it is uninteresting",
765+
typ.repr(tcx));
766+
}
767+
}
768+
}
769+
770+
return has_dtor_of_interest;
771+
}

0 commit comments

Comments
 (0)