Improve needs_drop query

* Handle cycles in `needs_drop` correctly
* Normalize types when computing `needs_drop`
* Move queries from rustc to rustc_ty

Author: matthewjasper

src/librustc/query/mod.rs

@@ -651,26 +651,27 @@ rustc_queries! {
             no_force
             desc { "computing whether `{}` is `Copy`", env.value }
         }
+        /// Query backing `TyS::is_sized`.
         query is_sized_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
             no_force
             desc { "computing whether `{}` is `Sized`", env.value }
         }
+        /// Query backing `TyS::is_freeze`.
         query is_freeze_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
             no_force
             desc { "computing whether `{}` is freeze", env.value }
         }
-
-        // The cycle error here should be reported as an error by `check_representable`.
-        // We consider the type as not needing drop in the meanwhile to avoid
-        // further errors (done in impl Value for NeedsDrop).
-        // Use `cycle_delay_bug` to delay the cycle error here to be emitted later
-        // in case we accidentally otherwise don't emit an error.
-        query needs_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> NeedsDrop {
-            cycle_delay_bug
+        /// Query backing `TyS::needs_drop`.
+        query needs_drop_raw(env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
             no_force
             desc { "computing whether `{}` needs drop", env.value }
         }
 
+        /// A list of types where the ADT requires drop if and only if any of
+        /// those types require drop. If the ADT is known to always need drop
+        /// then `Err(AlwaysRequiresDrop)` is returned.
+        query adt_drop_tys(_: DefId) -> Result<&'tcx ty::List<Ty<'tcx>>, AlwaysRequiresDrop> {}
+
         query layout_raw(
             env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>
         ) -> Result<&'tcx ty::layout::LayoutDetails, ty::layout::LayoutError<'tcx>> {

src/librustc/traits/misc.rs

@@ -1,12 +1,9 @@
 //! Miscellaneous type-system utilities that are too small to deserve their own modules.
 
-use crate::middle::lang_items;
 use crate::traits::{self, ObligationCause};
-use crate::ty::util::NeedsDrop;
 use crate::ty::{self, Ty, TyCtxt, TypeFoldable};
 
 use rustc_hir as hir;
-use rustc_span::DUMMY_SP;
 
 #[derive(Clone)]
 pub enum CopyImplementationError<'tcx> {
@@ -71,132 +68,3 @@ pub fn can_type_implement_copy(
         Ok(())
     })
 }
-
-fn is_copy_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
-    is_item_raw(tcx, query, lang_items::CopyTraitLangItem)
-}
-
-fn is_sized_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
-    is_item_raw(tcx, query, lang_items::SizedTraitLangItem)
-}
-
-fn is_freeze_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
-    is_item_raw(tcx, query, lang_items::FreezeTraitLangItem)
-}
-
-fn is_item_raw<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
-    item: lang_items::LangItem,
-) -> bool {
-    let (param_env, ty) = query.into_parts();
-    let trait_def_id = tcx.require_lang_item(item, None);
-    tcx.infer_ctxt().enter(|infcx| {
-        traits::type_known_to_meet_bound_modulo_regions(
-            &infcx,
-            param_env,
-            ty,
-            trait_def_id,
-            DUMMY_SP,
-        )
-    })
-}
-
-fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> NeedsDrop {
-    let (param_env, ty) = query.into_parts();
-
-    let needs_drop = |ty: Ty<'tcx>| -> bool { tcx.needs_drop_raw(param_env.and(ty)).0 };
-
-    assert!(!ty.needs_infer());
-
-    NeedsDrop(match ty.kind {
-        // Fast-path for primitive types
-        ty::Infer(ty::FreshIntTy(_))
-        | ty::Infer(ty::FreshFloatTy(_))
-        | ty::Bool
-        | ty::Int(_)
-        | ty::Uint(_)
-        | ty::Float(_)
-        | ty::Never
-        | ty::FnDef(..)
-        | ty::FnPtr(_)
-        | ty::Char
-        | ty::GeneratorWitness(..)
-        | ty::RawPtr(_)
-        | ty::Ref(..)
-        | ty::Str => false,
-
-        // Foreign types can never have destructors
-        ty::Foreign(..) => false,
-
-        // `ManuallyDrop` doesn't have a destructor regardless of field types.
-        ty::Adt(def, _) if Some(def.did) == tcx.lang_items().manually_drop() => false,
-
-        // Issue #22536: We first query `is_copy_modulo_regions`.  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 ty.is_copy_modulo_regions(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::Adt(def, _) if def.has_dtor(tcx) => true,
-
-        // Can refer to a type which may drop.
-        // FIXME(eddyb) check this against a ParamEnv.
-        ty::Dynamic(..)
-        | ty::Projection(..)
-        | ty::Param(_)
-        | ty::Bound(..)
-        | ty::Placeholder(..)
-        | ty::Opaque(..)
-        | ty::Infer(_)
-        | ty::Error => true,
-
-        ty::UnnormalizedProjection(..) => bug!("only used with chalk-engine"),
-
-        // Zero-length arrays never contain anything to drop.
-        ty::Array(_, len) if len.try_eval_usize(tcx, param_env) == Some(0) => false,
-
-        // Structural recursion.
-        ty::Array(ty, _) | ty::Slice(ty) => needs_drop(ty),
-
-        ty::Closure(def_id, ref substs) => {
-            substs.as_closure().upvar_tys(def_id, tcx).any(needs_drop)
-        }
-
-        // Pessimistically assume that all generators will require destructors
-        // as we don't know if a destructor is a noop or not until after the MIR
-        // state transformation pass
-        ty::Generator(..) => true,
-
-        ty::Tuple(..) => ty.tuple_fields().any(needs_drop),
-
-        // unions don't have destructors because of the child types,
-        // only if they manually implement `Drop` (handled above).
-        ty::Adt(def, _) if def.is_union() => false,
-
-        ty::Adt(def, substs) => def
-            .variants
-            .iter()
-            .any(|variant| variant.fields.iter().any(|field| needs_drop(field.ty(tcx, substs)))),
-    })
-}
-
-pub fn provide(providers: &mut ty::query::Providers<'_>) {
-    *providers = ty::query::Providers {
-        is_copy_raw,
-        is_sized_raw,
-        is_freeze_raw,
-        needs_drop_raw,
-        ..*providers
-    };
-}

src/librustc/traits/mod.rs

@@ -1255,7 +1255,6 @@ impl<'tcx> TraitObligation<'tcx> {
 }
 
 pub fn provide(providers: &mut ty::query::Providers<'_>) {
-    misc::provide(providers);
     *providers = ty::query::Providers {
         is_object_safe: object_safety::is_object_safe_provider,
         specialization_graph_of: specialize::specialization_graph_provider,

src/librustc/ty/query/mod.rs

@@ -33,7 +33,7 @@ use crate::traits::Clauses;
 use crate::traits::{self, Vtable};
 use crate::ty::steal::Steal;
 use crate::ty::subst::SubstsRef;
-use crate::ty::util::NeedsDrop;
+use crate::ty::util::AlwaysRequiresDrop;
 use crate::ty::{self, AdtSizedConstraint, CrateInherentImpls, ParamEnvAnd, Ty, TyCtxt};
 use crate::util::common::ErrorReported;
 use rustc_data_structures::fingerprint::Fingerprint;

src/librustc/ty/query/values.rs

@@ -1,4 +1,3 @@
-use crate::ty::util::NeedsDrop;
 use crate::ty::{self, AdtSizedConstraint, Ty, TyCtxt};
 
 use rustc_span::symbol::Symbol;
@@ -26,12 +25,6 @@ impl<'tcx> Value<'tcx> for ty::SymbolName {
     }
 }
 
-impl<'tcx> Value<'tcx> for NeedsDrop {
-    fn from_cycle_error(_: TyCtxt<'tcx>) -> Self {
-        NeedsDrop(false)
-    }
-}
-
 impl<'tcx> Value<'tcx> for AdtSizedConstraint<'tcx> {
     fn from_cycle_error(tcx: TyCtxt<'tcx>) -> Self {
         AdtSizedConstraint(tcx.intern_type_list(&[tcx.types.err]))

src/librustc/ty/util.rs

@@ -17,6 +17,7 @@ use rustc_hir::def::DefKind;
 use rustc_hir::def_id::DefId;
 use rustc_macros::HashStable;
 use rustc_span::Span;
+use smallvec::SmallVec;
 use std::{cmp, fmt};
 use syntax::ast;
 use syntax::attr::{self, SignedInt, UnsignedInt};
@@ -724,7 +725,23 @@ impl<'tcx> ty::TyS<'tcx> {
     /// Note that this method is used to check eligible types in unions.
     #[inline]
     pub fn needs_drop(&'tcx self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool {
-        tcx.needs_drop_raw(param_env.and(self)).0
+        // Avoid querying in simple cases.
+        match needs_drop_components(self) {
+            Err(AlwaysRequiresDrop) => true,
+            Ok(components) => {
+                let query_ty = match *components {
+                    [] => return false,
+                    // If we've got a single component, call the query with that
+                    // to increase the chance that we hit the query cache.
+                    [component_ty] => component_ty,
+                    _ => self,
+                };
+                // This doesn't depend on regions, so try to minimize distinct.
+                // query keys used.
+                let erased = tcx.normalize_erasing_regions(param_env, query_ty);
+                tcx.needs_drop_raw(param_env.and(erased))
+            }
+        }
     }
 
     pub fn same_type(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
@@ -923,9 +940,6 @@ impl<'tcx> ty::TyS<'tcx> {
     }
 }
 
-#[derive(Clone, HashStable)]
-pub struct NeedsDrop(pub bool);
-
 pub enum ExplicitSelf<'tcx> {
     ByValue,
     ByReference(ty::Region<'tcx>, hir::Mutability),
@@ -974,3 +988,62 @@ impl<'tcx> ExplicitSelf<'tcx> {
         }
     }
 }
+
+/// Returns a list of types such that the given type needs drop if and only if
+/// *any* of the returned types need drop. Returns `Err(AlwaysRequiresDrop)` if
+/// this type always needs drop.
+pub fn needs_drop_components(ty: Ty<'tcx>) -> Result<SmallVec<[Ty<'tcx>; 4]>, AlwaysRequiresDrop> {
+    match ty.kind {
+        ty::Infer(ty::FreshIntTy(_))
+        | ty::Infer(ty::FreshFloatTy(_))
+        | ty::Bool
+        | ty::Int(_)
+        | ty::Uint(_)
+        | ty::Float(_)
+        | ty::Never
+        | ty::FnDef(..)
+        | ty::FnPtr(_)
+        | ty::Char
+        | ty::GeneratorWitness(..)
+        | ty::RawPtr(_)
+        | ty::Ref(..)
+        | ty::Str => Ok(SmallVec::new()),
+
+        // Foreign types can never have destructors
+        ty::Foreign(..) => Ok(SmallVec::new()),
+
+        // Pessimistically assume that all generators will require destructors
+        // as we don't know if a destructor is a noop or not until after the MIR
+        // state transformation pass
+        ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop),
+
+        ty::Slice(ty) => needs_drop_components(ty),
+        ty::Array(elem_ty, ..) => {
+            match needs_drop_components(elem_ty) {
+                Ok(v) if v.is_empty() => Ok(v),
+                // Arrays of size zero don't need drop, even if their element
+                // type does.
+                _ => Ok(smallvec![ty]),
+            }
+        }
+        // If any field needs drop, then the whole tuple does.
+        ty::Tuple(..) => ty.tuple_fields().try_fold(SmallVec::new(), |mut acc, elem| {
+            acc.extend(needs_drop_components(elem)?);
+            Ok(acc)
+        }),
+
+        // These require checking for `Copy` bounds or `Adt` destructors.
+        ty::Adt(..)
+        | ty::Projection(..)
+        | ty::UnnormalizedProjection(..)
+        | ty::Param(_)
+        | ty::Bound(..)
+        | ty::Placeholder(..)
+        | ty::Opaque(..)
+        | ty::Infer(_)
+        | ty::Closure(..) => Ok(smallvec![ty]),
+    }
+}
+
+#[derive(Copy, Clone, Debug, HashStable)]
+pub struct AlwaysRequiresDrop;

src/librustc_ty/common_traits.rs

@@ -0,0 +1,40 @@
+//! Queries for checking whether a type implements one of a few common traits.
+
+use rustc::middle::lang_items;
+use rustc::traits;
+use rustc::ty::{self, Ty, TyCtxt};
+use rustc_span::DUMMY_SP;
+
+fn is_copy_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
+    is_item_raw(tcx, query, lang_items::CopyTraitLangItem)
+}
+
+fn is_sized_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
+    is_item_raw(tcx, query, lang_items::SizedTraitLangItem)
+}
+
+fn is_freeze_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool {
+    is_item_raw(tcx, query, lang_items::FreezeTraitLangItem)
+}
+
+fn is_item_raw<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
+    item: lang_items::LangItem,
+) -> bool {
+    let (param_env, ty) = query.into_parts();
+    let trait_def_id = tcx.require_lang_item(item, None);
+    tcx.infer_ctxt().enter(|infcx| {
+        traits::type_known_to_meet_bound_modulo_regions(
+            &infcx,
+            param_env,
+            ty,
+            trait_def_id,
+            DUMMY_SP,
+        )
+    })
+}
+
+pub(crate) fn provide(providers: &mut ty::query::Providers<'_>) {
+    *providers = ty::query::Providers { is_copy_raw, is_sized_raw, is_freeze_raw, ..*providers };
+}

src/librustc_ty/lib.rs

@@ -16,8 +16,12 @@ extern crate log;
 
 use rustc::ty::query::Providers;
 
+mod common_traits;
+mod needs_drop;
 mod ty;
 
 pub fn provide(providers: &mut Providers<'_>) {
+    common_traits::provide(providers);
+    needs_drop::provide(providers);
     ty::provide(providers);
 }