From 91cf0e741186a9fa3bf31b07a65dc89324c10296 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Jan 2020 20:24:44 +0000 Subject: [PATCH 1/9] Don't requery the param_env of a union Union fields have the ParamEnv of the union. --- src/librustc_typeck/check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 0a917a1853eb5..8d2cfd9a335d1 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1545,11 +1545,11 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: DefId) -> bool { if let ty::Adt(def, substs) = item_type.kind { assert!(def.is_union()); let fields = &def.non_enum_variant().fields; + let param_env = tcx.param_env(item_def_id); for field in fields { let field_ty = field.ty(tcx, substs); // We are currently checking the type this field came from, so it must be local. let field_span = tcx.hir().span_if_local(field.did).unwrap(); - let param_env = tcx.param_env(field.did); if field_ty.needs_drop(tcx, param_env) { struct_span_err!( tcx.sess, From 570c1613c1225d5777af5603dcf526da9cf57e19 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Jan 2020 20:25:39 +0000 Subject: [PATCH 2/9] Remove unnecessary features in rustc_ty --- src/librustc_ty/lib.rs | 2 -- src/librustc_ty/ty.rs | 6 +++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc_ty/lib.rs b/src/librustc_ty/lib.rs index e5ec98743e0ae..e970faef02f28 100644 --- a/src/librustc_ty/lib.rs +++ b/src/librustc_ty/lib.rs @@ -6,9 +6,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/")] #![feature(bool_to_option)] -#![feature(in_band_lifetimes)] #![feature(nll)] -#![cfg_attr(bootstrap, feature(slice_patterns))] #![recursion_limit = "256"] #[macro_use] diff --git a/src/librustc_ty/ty.rs b/src/librustc_ty/ty.rs index 8f882be1a090e..a131b11b07ba3 100644 --- a/src/librustc_ty/ty.rs +++ b/src/librustc_ty/ty.rs @@ -9,7 +9,11 @@ use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc_span::symbol::Symbol; use rustc_span::Span; -fn sized_constraint_for_ty(tcx: TyCtxt<'tcx>, adtdef: &ty::AdtDef, ty: Ty<'tcx>) -> Vec> { +fn sized_constraint_for_ty<'tcx>( + tcx: TyCtxt<'tcx>, + adtdef: &ty::AdtDef, + ty: Ty<'tcx>, +) -> Vec> { use ty::TyKind::*; let result = match ty.kind { From 39733223fc817efba52a4204dd697192bf5da185 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Jan 2020 20:26:36 +0000 Subject: [PATCH 3/9] Add IS_MANUALLY_DROP to AdtFlags --- src/librustc/ty/mod.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index f417b907a3811..2b272d7fe08af 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1792,19 +1792,22 @@ bitflags! { const IS_STRUCT = 1 << 2; /// Indicates whether the ADT is a struct and has a constructor. const HAS_CTOR = 1 << 3; - /// Indicates whether the type is a `PhantomData`. + /// Indicates whether the type is `PhantomData`. const IS_PHANTOM_DATA = 1 << 4; /// Indicates whether the type has a `#[fundamental]` attribute. const IS_FUNDAMENTAL = 1 << 5; - /// Indicates whether the type is a `Box`. + /// Indicates whether the type is `Box`. const IS_BOX = 1 << 6; + /// Indicates whether the type is `ManuallyDrop`. + const IS_MANUALLY_DROP = 1 << 7; + // FIXME(matthewjasper) replace these with diagnostic items /// Indicates whether the type is an `Arc`. - const IS_ARC = 1 << 7; + const IS_ARC = 1 << 8; /// Indicates whether the type is an `Rc`. - const IS_RC = 1 << 8; + const IS_RC = 1 << 9; /// Indicates whether the variant list of this ADT is `#[non_exhaustive]`. /// (i.e., this flag is never set unless this ADT is an enum). - const IS_VARIANT_LIST_NON_EXHAUSTIVE = 1 << 9; + const IS_VARIANT_LIST_NON_EXHAUSTIVE = 1 << 10; } } @@ -2180,6 +2183,9 @@ impl<'tcx> AdtDef { if Some(did) == tcx.lang_items().owned_box() { flags |= AdtFlags::IS_BOX; } + if Some(did) == tcx.lang_items().manually_drop() { + flags |= AdtFlags::IS_MANUALLY_DROP; + } if Some(did) == tcx.lang_items().arc() { flags |= AdtFlags::IS_ARC; } @@ -2280,6 +2286,12 @@ impl<'tcx> AdtDef { self.flags.contains(AdtFlags::IS_BOX) } + /// Returns `true` if this is ManuallyDrop. + #[inline] + pub fn is_manually_drop(&self) -> bool { + self.flags.contains(AdtFlags::IS_MANUALLY_DROP) + } + /// Returns `true` if this type has a destructor. pub fn has_dtor(&self, tcx: TyCtxt<'tcx>) -> bool { self.destructor(tcx).is_some() From d1965216a34dc2831cf44d2e15ad9d78403d10cc Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 30 Jan 2020 20:28:16 +0000 Subject: [PATCH 4/9] Improve needs_drop query * Handle cycles in `needs_drop` correctly * Normalize types when computing `needs_drop` * Move queries from rustc to rustc_ty --- src/librustc/query/mod.rs | 17 +- src/librustc/traits/misc.rs | 132 -------------- src/librustc/traits/mod.rs | 1 - src/librustc/ty/query/mod.rs | 2 +- src/librustc/ty/query/values.rs | 7 - src/librustc/ty/util.rs | 81 ++++++++- src/librustc_ty/common_traits.rs | 40 ++++ src/librustc_ty/lib.rs | 4 + src/librustc_ty/needs_drop.rs | 171 ++++++++++++++++++ .../ui/type-alias-impl-trait/issue-65918.rs | 2 + 10 files changed, 304 insertions(+), 153 deletions(-) create mode 100644 src/librustc_ty/common_traits.rs create mode 100644 src/librustc_ty/needs_drop.rs diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index 37d5e23535b81..c705956f4540a 100644 --- a/src/librustc/query/mod.rs +++ b/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>, AlwaysRequiresDrop> {} + query layout_raw( env: ty::ParamEnvAnd<'tcx, Ty<'tcx>> ) -> Result<&'tcx ty::layout::LayoutDetails, ty::layout::LayoutError<'tcx>> { diff --git a/src/librustc/traits/misc.rs b/src/librustc/traits/misc.rs index 08c3a77bf3aca..3fd0d12c626aa 100644 --- a/src/librustc/traits/misc.rs +++ b/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 - }; -} diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index daaba95cf6b13..f6c939c1ce5a2 100644 --- a/src/librustc/traits/mod.rs +++ b/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, diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 973cd81014616..4126b161a821b 100644 --- a/src/librustc/ty/query/mod.rs +++ b/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; diff --git a/src/librustc/ty/query/values.rs b/src/librustc/ty/query/values.rs index 900a91fe5cf59..b01d15c29b2db 100644 --- a/src/librustc/ty/query/values.rs +++ b/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])) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 4dfff85d53147..eaa7a43b09174 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -18,6 +18,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; @@ -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; 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; diff --git a/src/librustc_ty/common_traits.rs b/src/librustc_ty/common_traits.rs new file mode 100644 index 0000000000000..9fe8a19311fb6 --- /dev/null +++ b/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 }; +} diff --git a/src/librustc_ty/lib.rs b/src/librustc_ty/lib.rs index e970faef02f28..7eef19b94e401 100644 --- a/src/librustc_ty/lib.rs +++ b/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); } diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs new file mode 100644 index 0000000000000..1a65acb1f984b --- /dev/null +++ b/src/librustc_ty/needs_drop.rs @@ -0,0 +1,171 @@ +//! Check whether a type has (potentially) non-trivial drop glue. + +use rustc::ty::subst::Subst; +use rustc::ty::util::{needs_drop_components, AlwaysRequiresDrop}; +use rustc::ty::{self, Ty, TyCtxt}; +use rustc_data_structures::fx::FxHashSet; +use rustc_hir::def_id::DefId; +use rustc_span::DUMMY_SP; + +type NeedsDropResult = Result; + +fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { + let adt_fields = + move |adt_def: &ty::AdtDef| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter().copied()); + // If we don't know a type doesn't need drop, say 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_fields).next().is_some(); + debug!("needs_drop_raw({:?}) = {:?}", query, res); + res +} + +struct NeedsDropTypes<'tcx, F> { + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + query_ty: Ty<'tcx>, + seen_tys: FxHashSet>, + /// A stack of types left to process. Each round, we pop something from the + /// stack and check if it needs drop. If the result depends on whether some + /// other types need drop we push them onto the stack. + unchecked_tys: Vec<(Ty<'tcx>, usize)>, + recursion_limit: usize, + adt_components: F, +} + +impl<'tcx, F> NeedsDropTypes<'tcx, F> { + fn new( + tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + ty: Ty<'tcx>, + adt_components: F, + ) -> Self { + let mut seen_tys = FxHashSet::default(); + seen_tys.insert(ty); + let recursion_limit = *tcx.sess.recursion_limit.get(); + Self { + tcx, + param_env, + seen_tys, + query_ty: ty, + unchecked_tys: vec![(ty, 0)], + recursion_limit, + adt_components, + } + } +} + +impl<'tcx, F, I> Iterator for NeedsDropTypes<'tcx, F> +where + F: Fn(&ty::AdtDef) -> NeedsDropResult, + I: Iterator>, +{ + type Item = NeedsDropResult>; + + fn next(&mut self) -> Option>> { + let tcx = self.tcx; + + while let Some((ty, level)) = self.unchecked_tys.pop() { + if level > self.recursion_limit { + // Not having a `Span` isn't great. But there's hopefully some other + // recursion limit error as well. + tcx.sess.span_err( + DUMMY_SP, + &format!("overflow while checking whether `{}` requires drop", self.query_ty), + ); + return Some(Err(AlwaysRequiresDrop)); + } + + let components = match needs_drop_components(ty) { + Err(e) => return Some(Err(e)), + Ok(components) => components, + }; + debug!("needs_drop_components({:?}) = {:?}", ty, components); + + for component in components { + match component.kind { + _ if component.is_copy_modulo_regions(tcx, self.param_env, DUMMY_SP) => (), + + ty::Array(elem_ty, len) => { + // Zero-length arrays never contain anything to drop. + if len.try_eval_usize(tcx, self.param_env) != Some(0) { + if self.seen_tys.insert(elem_ty) { + self.unchecked_tys.push((elem_ty, level + 1)); + } + } + } + + ty::Closure(def_id, substs) => { + for upvar_ty in substs.as_closure().upvar_tys(def_id, tcx) { + if self.seen_tys.insert(upvar_ty) { + self.unchecked_tys.push((upvar_ty, level + 1)); + } + } + } + + // Check for a `Drop` impl and whether this is a union or + // `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) => { + let tys = match (self.adt_components)(adt_def) { + 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), + ); + if self.seen_tys.insert(subst_ty) { + self.unchecked_tys.push((subst_ty, level + 1)); + } + } + } + ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => { + if ty == component { + // Return the type to the caller so they can decide + // what to do with it. + return Some(Ok(component)); + } else if self.seen_tys.insert(component) { + // Store the type for later. We can't return here + // because we would then lose any other components + // of the type. + self.unchecked_tys.push((component, level + 1)); + } + } + _ => return Some(Err(AlwaysRequiresDrop)), + } + } + } + + return None; + } +} + +fn adt_drop_tys(tcx: TyCtxt<'_>, def_id: DefId) -> Result<&ty::List>, AlwaysRequiresDrop> { + let adt_components = move |adt_def: &ty::AdtDef| { + if adt_def.is_manually_drop() { + debug!("adt_drop_tys: `{:?}` is manually drop", adt_def); + return Ok(Vec::new().into_iter()); + } else if adt_def.destructor(tcx).is_some() { + debug!("adt_drop_tys: `{:?}` implements `Drop`", adt_def); + return Err(AlwaysRequiresDrop); + } else if adt_def.is_union() { + 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::>().into_iter()) + }; + + let adt_ty = tcx.type_of(def_id); + let param_env = tcx.param_env(def_id); + let res: Result, _> = + 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)) +} + +pub(crate) fn provide(providers: &mut ty::query::Providers<'_>) { + *providers = ty::query::Providers { needs_drop_raw, adt_drop_tys, ..*providers }; +} diff --git a/src/test/ui/type-alias-impl-trait/issue-65918.rs b/src/test/ui/type-alias-impl-trait/issue-65918.rs index 97efb85ef64c7..4ba778d53ac08 100644 --- a/src/test/ui/type-alias-impl-trait/issue-65918.rs +++ b/src/test/ui/type-alias-impl-trait/issue-65918.rs @@ -1,3 +1,5 @@ +// ignore-test: This now ICEs again. + // build-pass #![feature(type_alias_impl_trait)] From d20646b2d8033f31423b5bda3e56776df115e144 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 31 Jan 2020 22:22:30 +0000 Subject: [PATCH 5/9] Address review comments * Handle arrays with const-generic lengths * Use closure for repeated code. --- src/librustc/ty/util.rs | 33 ++++++++++++++++++++++----------- src/librustc_ty/needs_drop.rs | 35 ++++++++++++++--------------------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index eaa7a43b09174..6191d304719cb 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -18,6 +18,7 @@ use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_macros::HashStable; use rustc_span::Span; +use rustc_target::abi::TargetDataLayout; use smallvec::SmallVec; use std::{cmp, fmt}; use syntax::ast; @@ -726,7 +727,7 @@ impl<'tcx> ty::TyS<'tcx> { #[inline] pub fn needs_drop(&'tcx self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { // Avoid querying in simple cases. - match needs_drop_components(self) { + match needs_drop_components(self, &tcx.data_layout) { Err(AlwaysRequiresDrop) => true, Ok(components) => { let query_ty = match *components { @@ -736,7 +737,7 @@ impl<'tcx> ty::TyS<'tcx> { [component_ty] => component_ty, _ => self, }; - // This doesn't depend on regions, so try to minimize distinct. + // 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)) @@ -992,7 +993,10 @@ 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; 4]>, AlwaysRequiresDrop> { +pub fn needs_drop_components( + ty: Ty<'tcx>, + target_layout: &TargetDataLayout, +) -> Result; 2]>, AlwaysRequiresDrop> { match ty.kind { ty::Infer(ty::FreshIntTy(_)) | ty::Infer(ty::FreshFloatTy(_)) @@ -1017,18 +1021,25 @@ pub fn needs_drop_components(ty: Ty<'tcx>) -> Result; 4]>, Al // 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) { + ty::Slice(ty) => needs_drop_components(ty, target_layout), + ty::Array(elem_ty, size) => { + match needs_drop_components(elem_ty, target_layout) { 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]), + res => match size.val.try_to_bits(target_layout.pointer_size) { + // Arrays of size zero don't need drop, even if their element + // type does. + Some(0) => Ok(SmallVec::new()), + Some(_) => res, + // We don't know which of the cases above we are in, so + // return the whole type and let the caller decide what to + // do. + None => 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)?); + ty::Tuple(..) => ty.tuple_fields().try_fold(SmallVec::new(), move |mut acc, elem| { + acc.extend(needs_drop_components(elem, target_layout)?); Ok(acc) }), diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs index 1a65acb1f984b..c01b3e384aec5 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -76,30 +76,25 @@ where return Some(Err(AlwaysRequiresDrop)); } - let components = match needs_drop_components(ty) { + let components = match needs_drop_components(ty, &tcx.data_layout) { Err(e) => return Some(Err(e)), Ok(components) => components, }; debug!("needs_drop_components({:?}) = {:?}", ty, components); + let queue_type = move |this: &mut Self, component: Ty<'tcx>| { + if this.seen_tys.insert(component) { + this.unchecked_tys.push((component, level + 1)); + } + }; + for component in components { match component.kind { _ if component.is_copy_modulo_regions(tcx, self.param_env, DUMMY_SP) => (), - ty::Array(elem_ty, len) => { - // Zero-length arrays never contain anything to drop. - if len.try_eval_usize(tcx, self.param_env) != Some(0) { - if self.seen_tys.insert(elem_ty) { - self.unchecked_tys.push((elem_ty, level + 1)); - } - } - } - ty::Closure(def_id, substs) => { for upvar_ty in substs.as_closure().upvar_tys(def_id, tcx) { - if self.seen_tys.insert(upvar_ty) { - self.unchecked_tys.push((upvar_ty, level + 1)); - } + queue_type(self, upvar_ty); } } @@ -116,21 +111,19 @@ where self.param_env, required_ty.subst(tcx, substs), ); - if self.seen_tys.insert(subst_ty) { - self.unchecked_tys.push((subst_ty, level + 1)); - } + queue_type(self, subst_ty); } } - ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => { + ty::Array(..) | ty::Opaque(..) | ty::Projection(..) | ty::Param(_) => { if ty == component { - // Return the type to the caller so they can decide - // what to do with it. + // Return the type to the caller: they may be able + // to normalize further than we can. return Some(Ok(component)); - } else if self.seen_tys.insert(component) { + } else { // Store the type for later. We can't return here // because we would then lose any other components // of the type. - self.unchecked_tys.push((component, level + 1)); + queue_type(self, component); } } _ => return Some(Err(AlwaysRequiresDrop)), From 465b86253ce828e215d564fde53adf8742f0e3f6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 1 Feb 2020 11:41:58 +0000 Subject: [PATCH 6/9] Use correct `ParamEnv` in `Instance::resolve` --- src/librustc/ty/instance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/ty/instance.rs b/src/librustc/ty/instance.rs index 51a18f8eae274..f73e069df36c7 100644 --- a/src/librustc/ty/instance.rs +++ b/src/librustc/ty/instance.rs @@ -285,7 +285,7 @@ impl<'tcx> Instance<'tcx> { _ => { if Some(def_id) == tcx.lang_items().drop_in_place_fn() { let ty = substs.type_at(0); - if ty.needs_drop(tcx, ty::ParamEnv::reveal_all()) { + if ty.needs_drop(tcx, param_env.with_reveal_all()) { debug!(" => nontrivial drop glue"); ty::InstanceDef::DropGlue(def_id, Some(ty)) } else { From 3eb524188451fcec6cd5ed7e3cba2404021b75eb Mon Sep 17 00:00:00 2001 From: matthewjasper <20113453+matthewjasper@users.noreply.github.com> Date: Sun, 9 Feb 2020 10:16:57 +0000 Subject: [PATCH 7/9] Apply suggestions from code review Co-Authored-By: varkor --- src/librustc/ty/mod.rs | 2 +- src/librustc/ty/util.rs | 4 ++-- src/librustc_ty/needs_drop.rs | 13 +++++++------ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 2b272d7fe08af..85b9cbf3de15a 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2286,7 +2286,7 @@ impl<'tcx> AdtDef { self.flags.contains(AdtFlags::IS_BOX) } - /// Returns `true` if this is ManuallyDrop. + /// Returns `true` if this is `ManuallyDrop`. #[inline] pub fn is_manually_drop(&self) -> bool { self.flags.contains(AdtFlags::IS_MANUALLY_DROP) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 6191d304719cb..4011c010c0355 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -1013,12 +1013,12 @@ pub fn needs_drop_components( | ty::Ref(..) | ty::Str => Ok(SmallVec::new()), - // Foreign types can never have destructors + // 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 + // state transformation pass. ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop), ty::Slice(ty) => needs_drop_components(ty, target_layout), diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs index c01b3e384aec5..0f71246c73759 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -12,9 +12,9 @@ type NeedsDropResult = Result; fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> bool { let adt_fields = move |adt_def: &ty::AdtDef| tcx.adt_drop_tys(adt_def.did).map(|tys| tys.iter().copied()); - // If we don't know a type doesn't need drop, say it's a type parameter - // without a `Copy` bound, then we conservatively return that it needs - // drop. + // 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_fields).next().is_some(); debug!("needs_drop_raw({:?}) = {:?}", query, res); res @@ -25,9 +25,10 @@ struct NeedsDropTypes<'tcx, F> { param_env: ty::ParamEnv<'tcx>, query_ty: Ty<'tcx>, seen_tys: FxHashSet>, - /// A stack of types left to process. Each round, we pop something from the - /// stack and check if it needs drop. If the result depends on whether some - /// other types need drop we push them onto the stack. + /// A stack of types left to process, and the recursion depth when we + /// pushed that type. Each round, we pop something from the stack and check + /// if it needs drop. If the result depends on whether some other types + /// need drop we push them onto the stack. unchecked_tys: Vec<(Ty<'tcx>, usize)>, recursion_limit: usize, adt_components: F, From 842938a82f673f0483e40328709c3887a2091534 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 9 Feb 2020 10:56:18 +0000 Subject: [PATCH 8/9] cache adt_drop_tys --- src/librustc/query/mod.rs | 4 +++- src/librustc/ty/util.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index c705956f4540a..39046df56e5b8 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -670,7 +670,9 @@ rustc_queries! { /// 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>, AlwaysRequiresDrop> {} + query adt_drop_tys(_: DefId) -> Result<&'tcx ty::List>, AlwaysRequiresDrop> { + cache_on_disk_if { true } + } query layout_raw( env: ty::ParamEnvAnd<'tcx, Ty<'tcx>> diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 4011c010c0355..5a77bac8cf204 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -1056,5 +1056,5 @@ pub fn needs_drop_components( } } -#[derive(Copy, Clone, Debug, HashStable)] +#[derive(Copy, Clone, Debug, HashStable, RustcEncodable, RustcDecodable)] pub struct AlwaysRequiresDrop; From 30a8353f372f7cc719d1de6811996ce5215183a6 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 10 Feb 2020 22:31:19 +0000 Subject: [PATCH 9/9] Specify overflow checks behaviour in test --- src/test/ui/recursion/recursion.rs | 1 + src/test/ui/recursion/recursion.stderr | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/ui/recursion/recursion.rs b/src/test/ui/recursion/recursion.rs index 9d939e131823c..bf1eaef367d69 100644 --- a/src/test/ui/recursion/recursion.rs +++ b/src/test/ui/recursion/recursion.rs @@ -1,4 +1,5 @@ // build-fail +// compile-flags:-C overflow-checks=off enum Nil {NilValue} struct Cons {head:isize, tail:T} diff --git a/src/test/ui/recursion/recursion.stderr b/src/test/ui/recursion/recursion.stderr index 17293720a43ea..1a65b0e84f6a3 100644 --- a/src/test/ui/recursion/recursion.stderr +++ b/src/test/ui/recursion/recursion.stderr @@ -1,5 +1,5 @@ error: reached the recursion limit while instantiating `test::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` - --> $DIR/recursion.rs:14:1 + --> $DIR/recursion.rs:15:1 | LL | / fn test (n:isize, i:isize, first:T, second:T) ->isize { LL | | match n { 0 => {first.dot(second)}