convert AdtDef::destructor to on-demand

This removes the Cell from AdtDef. Also, moving destructor validity
checking to on-demand (forced during item-type checking) ensures that
invalid destructors can't cause ICEs.

Fixes #38868.
Fixes #40132.
This commit is contained in:
Ariel Ben-Yehuda 2017-03-01 18:42:26 +02:00
parent e1cb9ba221
commit e294fd5ecb
11 changed files with 133 additions and 129 deletions

View File

@ -79,8 +79,6 @@ pub enum DepNode<D: Clone + Debug> {
Variance,
WfCheck(D),
TypeckItemType(D),
Dropck,
DropckImpl(D),
UnusedTraitCheck,
CheckConst(D),
Privacy,
@ -112,6 +110,7 @@ pub enum DepNode<D: Clone + Debug> {
ItemSignature(D),
TypeParamPredicates((D, D)),
SizedConstraint(D),
AdtDestructor(D),
AssociatedItemDefIds(D),
InherentImpls(D),
TypeckTables(D),
@ -223,7 +222,6 @@ impl<D: Clone + Debug> DepNode<D> {
EntryPoint => Some(EntryPoint),
CheckEntryFn => Some(CheckEntryFn),
Variance => Some(Variance),
Dropck => Some(Dropck),
UnusedTraitCheck => Some(UnusedTraitCheck),
Privacy => Some(Privacy),
Reachability => Some(Reachability),
@ -250,7 +248,6 @@ impl<D: Clone + Debug> DepNode<D> {
CoherenceOrphanCheck(ref d) => op(d).map(CoherenceOrphanCheck),
WfCheck(ref d) => op(d).map(WfCheck),
TypeckItemType(ref d) => op(d).map(TypeckItemType),
DropckImpl(ref d) => op(d).map(DropckImpl),
CheckConst(ref d) => op(d).map(CheckConst),
IntrinsicCheck(ref d) => op(d).map(IntrinsicCheck),
MatchCheck(ref d) => op(d).map(MatchCheck),
@ -266,6 +263,7 @@ impl<D: Clone + Debug> DepNode<D> {
Some(TypeParamPredicates((try_opt!(op(item)), try_opt!(op(param)))))
}
SizedConstraint(ref d) => op(d).map(SizedConstraint),
AdtDestructor(ref d) => op(d).map(AdtDestructor),
AssociatedItemDefIds(ref d) => op(d).map(AssociatedItemDefIds),
InherentImpls(ref d) => op(d).map(InherentImpls),
TypeckTables(ref d) => op(d).map(TypeckTables),
@ -297,4 +295,3 @@ impl<D: Clone + Debug> DepNode<D> {
/// them even in the absence of a tcx.)
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
pub struct WorkProductId(pub String);

View File

@ -333,6 +333,7 @@ define_maps! { <'tcx>
pub trait_def: ItemSignature(DefId) -> &'tcx ty::TraitDef,
pub adt_def: ItemSignature(DefId) -> &'tcx ty::AdtDef,
pub adt_destructor: AdtDestructor(DefId) -> Option<ty::Destructor>,
pub adt_sized_constraint: SizedConstraint(DefId) -> Ty<'tcx>,
/// Maps from def-id of a type or region parameter to its

View File

@ -1291,17 +1291,31 @@ impl<'a, 'tcx> ParameterEnvironment<'tcx> {
}
}
#[derive(Copy, Clone, Debug)]
pub struct Destructor {
/// The def-id of the destructor method
pub did: DefId,
/// Invoking the destructor of a dtorck type during usual cleanup
/// (e.g. the glue emitted for stack unwinding) requires all
/// lifetimes in the type-structure of `adt` to strictly outlive
/// the adt value itself.
///
/// If `adt` is not dtorck, then the adt's destructor can be
/// invoked even when there are lifetimes in the type-structure of
/// `adt` that do not strictly outlive the adt value itself.
/// (This allows programs to make cyclic structures without
/// resorting to unasfe means; see RFCs 769 and 1238).
pub is_dtorck: bool,
}
bitflags! {
flags AdtFlags: u32 {
const NO_ADT_FLAGS = 0,
const IS_ENUM = 1 << 0,
const IS_DTORCK = 1 << 1, // is this a dtorck type?
const IS_DTORCK_VALID = 1 << 2,
const IS_PHANTOM_DATA = 1 << 3,
const IS_FUNDAMENTAL = 1 << 4,
const IS_UNION = 1 << 5,
const IS_BOX = 1 << 6,
const IS_DTOR_VALID = 1 << 7,
const IS_PHANTOM_DATA = 1 << 1,
const IS_FUNDAMENTAL = 1 << 2,
const IS_UNION = 1 << 3,
const IS_BOX = 1 << 4,
}
}
@ -1343,8 +1357,7 @@ pub struct FieldDef {
pub struct AdtDef {
pub did: DefId,
pub variants: Vec<VariantDef>,
destructor: Cell<Option<DefId>>,
flags: Cell<AdtFlags>,
flags: AdtFlags,
pub repr: ReprOptions,
}
@ -1436,19 +1449,11 @@ impl<'a, 'gcx, 'tcx> AdtDef {
AdtDef {
did: did,
variants: variants,
flags: Cell::new(flags),
destructor: Cell::new(None),
flags: flags,
repr: repr,
}
}
fn calculate_dtorck(&'gcx self, tcx: TyCtxt) {
if tcx.is_adt_dtorck(self) {
self.flags.set(self.flags.get() | AdtFlags::IS_DTORCK);
}
self.flags.set(self.flags.get() | AdtFlags::IS_DTORCK_VALID)
}
#[inline]
pub fn is_struct(&self) -> bool {
!self.is_union() && !self.is_enum()
@ -1456,12 +1461,12 @@ impl<'a, 'gcx, 'tcx> AdtDef {
#[inline]
pub fn is_union(&self) -> bool {
self.flags.get().intersects(AdtFlags::IS_UNION)
self.flags.intersects(AdtFlags::IS_UNION)
}
#[inline]
pub fn is_enum(&self) -> bool {
self.flags.get().intersects(AdtFlags::IS_ENUM)
self.flags.intersects(AdtFlags::IS_ENUM)
}
/// Returns the kind of the ADT - Struct or Enum.
@ -1497,29 +1502,26 @@ impl<'a, 'gcx, 'tcx> AdtDef {
/// alive; Otherwise, only the contents are required to be.
#[inline]
pub fn is_dtorck(&'gcx self, tcx: TyCtxt) -> bool {
if !self.flags.get().intersects(AdtFlags::IS_DTORCK_VALID) {
self.calculate_dtorck(tcx)
}
self.flags.get().intersects(AdtFlags::IS_DTORCK)
self.destructor(tcx).map_or(false, |d| d.is_dtorck)
}
/// Returns whether this type is #[fundamental] for the purposes
/// of coherence checking.
#[inline]
pub fn is_fundamental(&self) -> bool {
self.flags.get().intersects(AdtFlags::IS_FUNDAMENTAL)
self.flags.intersects(AdtFlags::IS_FUNDAMENTAL)
}
/// Returns true if this is PhantomData<T>.
#[inline]
pub fn is_phantom_data(&self) -> bool {
self.flags.get().intersects(AdtFlags::IS_PHANTOM_DATA)
self.flags.intersects(AdtFlags::IS_PHANTOM_DATA)
}
/// Returns true if this is Box<T>.
#[inline]
pub fn is_box(&self) -> bool {
self.flags.get().intersects(AdtFlags::IS_BOX)
self.flags.intersects(AdtFlags::IS_BOX)
}
/// Returns whether this type has a destructor.
@ -1579,38 +1581,6 @@ impl<'a, 'gcx, 'tcx> AdtDef {
}
}
pub fn destructor(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> Option<DefId> {
if self.flags.get().intersects(AdtFlags::IS_DTOR_VALID) {
return self.destructor.get();
}
let dtor = self.destructor_uncached(tcx);
self.destructor.set(dtor);
self.flags.set(self.flags.get() | AdtFlags::IS_DTOR_VALID);
dtor
}
fn destructor_uncached(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> Option<DefId> {
let drop_trait = if let Some(def_id) = tcx.lang_items.drop_trait() {
def_id
} else {
return None;
};
queries::coherent_trait::get(tcx, DUMMY_SP, (LOCAL_CRATE, drop_trait));
let mut dtor = None;
let ty = tcx.item_type(self.did);
tcx.lookup_trait_def(drop_trait).for_each_relevant_impl(tcx, ty, |def_id| {
if let Some(item) = tcx.associated_items(def_id).next() {
dtor = Some(item.def_id);
}
});
dtor
}
pub fn discriminants(&'a self, tcx: TyCtxt<'a, 'gcx, 'tcx>)
-> impl Iterator<Item=ConstInt> + 'a {
let repr_type = self.repr.discr_type();
@ -1632,6 +1602,10 @@ impl<'a, 'gcx, 'tcx> AdtDef {
})
}
pub fn destructor(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> Option<Destructor> {
queries::adt_destructor::get(tcx, DUMMY_SP, self.did)
}
/// Returns a simpler type such that `Self: Sized` if and only
/// if that type is Sized, or `TyErr` if this type is recursive.
///

View File

@ -10,7 +10,7 @@
//! misc. type-system utilities too small to deserve their own file
use hir::def_id::DefId;
use hir::def_id::{DefId, LOCAL_CRATE};
use hir::map::DefPathData;
use infer::InferCtxt;
use hir::map as hir_map;
@ -20,6 +20,7 @@ use ty::{ParameterEnvironment};
use ty::fold::TypeVisitor;
use ty::layout::{Layout, LayoutError};
use ty::TypeVariants::*;
use util::common::ErrorReported;
use util::nodemap::FxHashMap;
use middle::lang_items;
@ -32,7 +33,7 @@ use std::hash::Hash;
use std::intrinsics;
use syntax::ast::{self, Name};
use syntax::attr::{self, SignedInt, UnsignedInt};
use syntax_pos::Span;
use syntax_pos::{Span, DUMMY_SP};
use hir;
@ -346,22 +347,33 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
hasher.finish()
}
/// Returns true if this ADT is a dtorck type.
///
/// Invoking the destructor of a dtorck type during usual cleanup
/// (e.g. the glue emitted for stack unwinding) requires all
/// lifetimes in the type-structure of `adt` to strictly outlive
/// the adt value itself.
///
/// If `adt` is not dtorck, then the adt's destructor can be
/// invoked even when there are lifetimes in the type-structure of
/// `adt` that do not strictly outlive the adt value itself.
/// (This allows programs to make cyclic structures without
/// resorting to unasfe means; see RFCs 769 and 1238).
pub fn is_adt_dtorck(self, adt: &ty::AdtDef) -> bool {
let dtor_method = match adt.destructor(self) {
/// Calculate the destructor of a given type.
pub fn calculate_dtor(
self,
adt_did: DefId,
validate: &mut FnMut(Self, DefId) -> Result<(), ErrorReported>
) -> Option<ty::Destructor> {
let drop_trait = if let Some(def_id) = self.lang_items.drop_trait() {
def_id
} else {
return None;
};
ty::queries::coherent_trait::get(self, DUMMY_SP, (LOCAL_CRATE, drop_trait));
let mut dtor_did = None;
let ty = self.item_type(adt_did);
self.lookup_trait_def(drop_trait).for_each_relevant_impl(self, ty, |impl_did| {
if let Some(item) = self.associated_items(impl_did).next() {
if let Ok(()) = validate(self, impl_did) {
dtor_did = Some(item.def_id);
}
}
});
let dtor_did = match dtor_did {
Some(dtor) => dtor,
None => return false
None => return None
};
// RFC 1238: if the destructor method is tagged with the
@ -373,7 +385,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
// Such access can be in plain sight (e.g. dereferencing
// `*foo.0` of `Foo<'a>(&'a u32)`) or indirectly hidden
// (e.g. calling `foo.0.clone()` of `Foo<T:Clone>`).
return !self.has_attr(dtor_method, "unsafe_destructor_blind_to_params");
let is_dtorck = !self.has_attr(dtor_did, "unsafe_destructor_blind_to_params");
Some(ty::Destructor { did: dtor_did, is_dtorck: is_dtorck })
}
pub fn closure_base_def_id(&self, def_id: DefId) -> DefId {

View File

@ -76,6 +76,10 @@ provide! { <'tcx> tcx, def_id, cdata
tcx.alloc_trait_def(cdata.get_trait_def(def_id.index, tcx))
}
adt_def => { cdata.get_adt_def(def_id.index, tcx) }
adt_destructor => {
let _ = cdata;
tcx.calculate_dtor(def_id, &mut |_,_| Ok(()))
}
variances => { Rc::new(cdata.get_item_variances(def_id.index)) }
associated_item_def_ids => {
let mut result = vec![];

View File

@ -753,12 +753,12 @@ fn find_drop_glue_neighbors<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
// If the type implements Drop, also add a translation item for the
// monomorphized Drop::drop() implementation.
let destructor_did = match ty.sty {
let destructor = match ty.sty {
ty::TyAdt(def, _) => def.destructor(scx.tcx()),
_ => None
};
if let (Some(destructor_did), false) = (destructor_did, ty.is_box()) {
if let (Some(destructor), false) = (destructor, ty.is_box()) {
use rustc::ty::ToPolyTraitRef;
let drop_trait_def_id = scx.tcx()
@ -778,9 +778,9 @@ fn find_drop_glue_neighbors<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>,
_ => bug!()
};
if should_trans_locally(scx.tcx(), destructor_did) {
if should_trans_locally(scx.tcx(), destructor.did) {
let trans_item = create_fn_trans_item(scx,
destructor_did,
destructor.did,
substs,
scx.tcx().intern_substs(&[]));
output.push(trans_item);

View File

@ -265,7 +265,7 @@ pub fn implement_drop_glue<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, g: DropGlueKi
traits::VtableImpl(data) => data,
_ => bug!("dtor for {:?} is not an impl???", t)
};
let dtor_did = def.destructor(tcx).unwrap();
let dtor_did = def.destructor(tcx).unwrap().did;
let callee = Callee::def(bcx.ccx, dtor_did, vtbl.substs);
let fn_ty = callee.direct_fn_type(bcx.ccx, &[]);
let llret;

View File

@ -17,6 +17,7 @@ use middle::region;
use rustc::ty::subst::{Subst, Substs};
use rustc::ty::{self, AdtKind, Ty, TyCtxt};
use rustc::traits::{self, ObligationCause, Reveal};
use util::common::ErrorReported;
use util::nodemap::FxHashSet;
use syntax::ast;
@ -40,7 +41,8 @@ use syntax_pos::Span;
/// cannot do `struct S<T>; impl<T:Clone> Drop for S<T> { ... }`).
///
pub fn check_drop_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
drop_impl_did: DefId) -> Result<(), ()> {
drop_impl_did: DefId)
-> Result<(), ErrorReported> {
let dtor_self_type = tcx.item_type(drop_impl_did);
let dtor_predicates = tcx.item_predicates(drop_impl_did);
match dtor_self_type.sty {
@ -72,7 +74,7 @@ fn ensure_drop_params_and_item_params_correspond<'a, 'tcx>(
drop_impl_did: DefId,
drop_impl_ty: Ty<'tcx>,
self_type_did: DefId)
-> Result<(), ()>
-> Result<(), ErrorReported>
{
let drop_impl_node_id = tcx.hir.as_local_node_id(drop_impl_did).unwrap();
let self_type_node_id = tcx.hir.as_local_node_id(self_type_did).unwrap();
@ -106,14 +108,14 @@ fn ensure_drop_params_and_item_params_correspond<'a, 'tcx>(
"Use same sequence of generic type and region \
parameters that is on the struct/enum definition")
.emit();
return Err(());
return Err(ErrorReported);
}
}
if let Err(ref errors) = fulfillment_cx.select_all_or_error(&infcx) {
// this could be reached when we get lazy normalization
infcx.report_fulfillment_errors(errors);
return Err(());
return Err(ErrorReported);
}
let free_regions = FreeRegionMap::new();
@ -130,8 +132,9 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'a, 'tcx>(
dtor_predicates: &ty::GenericPredicates<'tcx>,
self_type_did: DefId,
self_to_impl_substs: &Substs<'tcx>)
-> Result<(), ()>
-> Result<(), ErrorReported>
{
let mut result = Ok(());
// Here is an example, analogous to that from
// `compare_impl_method`.
@ -207,13 +210,11 @@ fn ensure_drop_predicates_are_implied_by_item_defn<'a, 'tcx>(
"The same requirement must be part of \
the struct/enum definition")
.emit();
result = Err(ErrorReported);
}
}
if tcx.sess.has_errors() {
return Err(());
}
Ok(())
result
}
/// check_safety_of_destructor_if_necessary confirms that the type
@ -556,7 +557,7 @@ fn has_dtor_of_interest<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
// attributes attached to the impl's generics.
let dtor_method = adt_def.destructor(tcx)
.expect("dtorck type without destructor impossible");
let method = tcx.associated_item(dtor_method);
let method = tcx.associated_item(dtor_method.did);
let impl_def_id = method.container.id();
let revised_ty = revise_self_ty(tcx, adt_def, impl_def_id, substs);
return DropckKind::RevisedSelf(revised_ty);

View File

@ -609,31 +609,12 @@ pub fn check_item_bodies<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CompileResult
})
}
pub fn check_drop_impls<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> CompileResult {
tcx.sess.track_errors(|| {
let _task = tcx.dep_graph.in_task(DepNode::Dropck);
let drop_trait = match tcx.lang_items.drop_trait() {
Some(id) => tcx.lookup_trait_def(id), None => { return }
};
drop_trait.for_each_impl(tcx, |drop_impl_did| {
let _task = tcx.dep_graph.in_task(DepNode::DropckImpl(drop_impl_did));
if drop_impl_did.is_local() {
match dropck::check_drop_impl(tcx, drop_impl_did) {
Ok(()) => {}
Err(()) => {
assert!(tcx.sess.has_errors());
}
}
}
});
})
}
pub fn provide(providers: &mut Providers) {
*providers = Providers {
typeck_tables,
closure_type,
closure_kind,
adt_destructor,
..*providers
};
}
@ -652,6 +633,12 @@ fn closure_kind<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
tcx.item_tables(def_id).closure_kinds[&node_id]
}
fn adt_destructor<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> Option<ty::Destructor> {
tcx.calculate_dtor(def_id, &mut dropck::check_drop_impl)
}
fn typeck_tables<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> &'tcx ty::TypeckTables<'tcx> {
@ -901,9 +888,11 @@ fn check_struct<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
id: ast::NodeId,
span: Span) {
let def_id = tcx.hir.local_def_id(id);
let def = tcx.lookup_adt_def(def_id);
def.destructor(tcx); // force the destructor to be evaluated
check_representable(tcx, span, def_id);
if tcx.lookup_adt_def(def_id).repr.simd {
if def.repr.simd {
check_simd(tcx, span, def_id);
}
}
@ -911,7 +900,10 @@ fn check_struct<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
fn check_union<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
id: ast::NodeId,
span: Span) {
check_representable(tcx, span, tcx.hir.local_def_id(id));
let def_id = tcx.hir.local_def_id(id);
let def = tcx.lookup_adt_def(def_id);
def.destructor(tcx); // force the destructor to be evaluated
check_representable(tcx, span, def_id);
}
pub fn check_item_type<'a,'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, it: &'tcx hir::Item) {
@ -926,10 +918,10 @@ pub fn check_item_type<'a,'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, it: &'tcx hir::Item
tcx.item_tables(tcx.hir.local_def_id(it.id));
}
hir::ItemEnum(ref enum_definition, _) => {
check_enum_variants(tcx,
it.span,
&enum_definition.variants,
it.id);
check_enum(tcx,
it.span,
&enum_definition.variants,
it.id);
}
hir::ItemFn(..) => {} // entirely within check_item_body
hir::ItemImpl(.., ref impl_item_refs) => {
@ -1322,12 +1314,13 @@ pub fn check_simd<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, sp: Span, def_id: DefId
}
#[allow(trivial_numeric_casts)]
pub fn check_enum_variants<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
sp: Span,
vs: &'tcx [hir::Variant],
id: ast::NodeId) {
pub fn check_enum<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
sp: Span,
vs: &'tcx [hir::Variant],
id: ast::NodeId) {
let def_id = tcx.hir.local_def_id(id);
let def = tcx.lookup_adt_def(def_id);
def.destructor(tcx); // force the destructor to be evaluated
if vs.is_empty() && tcx.has_attr(def_id, "repr") {
struct_span_err!(

View File

@ -322,8 +322,6 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>)
time(time_passes, "item-bodies checking", || check::check_item_bodies(tcx))?;
time(time_passes, "drop-impl checking", || check::check_drop_impls(tcx))?;
check_unused::check_crate(tcx);
check_for_entry_fn(tcx);

View File

@ -0,0 +1,23 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
pub struct List<T> {
head: T,
}
impl Drop for List<i32> { //~ ERROR E0366
fn drop(&mut self) {
panic!()
}
}
fn main() {
List { head: 0 };
}