Auto merge of #40178 - arielb1:provide-destructors, r=eddyb

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.

r? @eddyb

Author: bors

src/librustc/dep_graph/dep_node.rs

@@ -79,8 +79,6 @@ pub enum DepNode<D: Clone + Debug> {
     Variance,
     WfCheck(D),
     TypeckItemType(D),
-    Dropck,
-    DropckImpl(D),
     UnusedTraitCheck,
     CheckConst(D),
     Privacy,
@@ -114,6 +112,7 @@ pub enum DepNode<D: Clone + Debug> {
     ItemSignature(D),
     TypeParamPredicates((D, D)),
     SizedConstraint(D),
+    AdtDestructor(D),
     AssociatedItemDefIds(D),
     InherentImpls(D),
     TypeckBodiesKrate,
@@ -229,7 +228,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),
@@ -256,7 +254,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),
@@ -272,6 +269,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),
@@ -303,4 +301,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);
-

src/librustc/ty/maps.rs

@@ -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

src/librustc/ty/mod.rs

@@ -1273,17 +1273,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,
     }
 }
 
@@ -1325,8 +1339,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,
 }
 
@@ -1425,32 +1438,24 @@ 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()
     }
 
     #[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.
@@ -1486,29 +1491,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.
@@ -1568,38 +1570,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();
@@ -1621,6 +1591,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.
     ///

src/librustc/ty/util.rs

@@ -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 {

src/librustc_metadata/cstore_impl.rs

@@ -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![];

src/librustc_trans/collector.rs

@@ -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);

src/librustc_trans/glue.rs

@@ -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;