From 9ad3b94847db8cbee27947d7d62a6add28a9c0e9 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Mon, 17 Apr 2017 23:26:48 +0300 Subject: [PATCH] rustc: replace TypeContents::needs_drop with Ty::may_drop. --- src/librustc/ty/contents.rs | 139 ------------------------------------ src/librustc/ty/context.rs | 4 -- src/librustc/ty/mod.rs | 19 +++-- src/librustc/ty/util.rs | 79 +++++++++++++++++++- 4 files changed, 87 insertions(+), 154 deletions(-) delete mode 100644 src/librustc/ty/contents.rs diff --git a/src/librustc/ty/contents.rs b/src/librustc/ty/contents.rs deleted file mode 100644 index a1cb848213a8f..0000000000000 --- a/src/librustc/ty/contents.rs +++ /dev/null @@ -1,139 +0,0 @@ -// Copyright 2012-2015 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 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -use ty::{self, Ty, TyCtxt}; -use util::common::MemoizationMap; -use util::nodemap::FxHashMap; - -bitflags! { - /// Type contents is how the type checker reasons about kinds. - /// They track what kinds of things are found within a type. You can - /// think of them as kind of an "anti-kind". They track the kinds of values - /// and thinks that are contained in types. Having a larger contents for - /// a type tends to rule that type *out* from various kinds. For example, - /// a type that contains a reference is not sendable. - /// - /// The reason we compute type contents and not kinds is that it is - /// easier for me (nmatsakis) to think about what is contained within - /// a type than to think about what is *not* contained within a type. - flags TypeContents: u8 { - const OWNS_DTOR = 0b1, - } -} - -impl TypeContents { - pub fn when(&self, cond: bool) -> TypeContents { - if cond {*self} else {TypeContents::empty()} - } - - pub fn needs_drop(&self, _: TyCtxt) -> bool { - self.intersects(TypeContents::OWNS_DTOR) - } - - pub fn union(v: I, mut f: F) -> TypeContents where - I: IntoIterator, - F: FnMut(T) -> TypeContents, - { - v.into_iter().fold(TypeContents::empty(), |tc, ty| tc | f(ty)) - } -} - -impl<'a, 'tcx> ty::TyS<'tcx> { - pub fn type_contents(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> TypeContents { - return tcx.tc_cache.memoize(self, || tc_ty(tcx, self, &mut FxHashMap())); - - fn tc_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - ty: Ty<'tcx>, - cache: &mut FxHashMap, TypeContents>) -> TypeContents - { - // Subtle: Note that we are *not* using tcx.tc_cache here but rather a - // private cache for this walk. This is needed in the case of cyclic - // types like: - // - // struct List { next: Box>, ... } - // - // When computing the type contents of such a type, we wind up deeply - // recursing as we go. So when we encounter the recursive reference - // to List, we temporarily use TypeContents::empty() as its contents. Later we'll - // patch up the cache with the correct value, once we've computed it - // (this is basically a co-inductive process, if that helps). So in - // the end we'll compute TypeContents::OwnsOwned, in this case. - // - // The problem is, as we are doing the computation, we will also - // compute an *intermediate* contents for, e.g., Option of - // TypeContents::empty(). This is ok during the computation of List itself, but if - // we stored this intermediate value into tcx.tc_cache, then later - // requests for the contents of Option would also yield TypeContents::empty() - // which is incorrect. This value was computed based on the crutch - // value for the type contents of list. The correct value is - // TypeContents::OwnsOwned. This manifested as issue #4821. - if let Some(tc) = cache.get(&ty) { - return *tc; - } - // Must check both caches! - if let Some(tc) = tcx.tc_cache.borrow().get(&ty) { - return *tc; - } - cache.insert(ty, TypeContents::empty()); - - let result = match ty.sty { - ty::TyInfer(ty::FreshIntTy(_)) | ty::TyInfer(ty::FreshFloatTy(_)) | - ty::TyBool | ty::TyInt(_) | ty::TyUint(_) | ty::TyFloat(_) | ty::TyNever | - ty::TyFnDef(..) | ty::TyFnPtr(_) | ty::TyChar | - ty::TyRawPtr(_) | ty::TyRef(..) | - ty::TyStr => TypeContents::empty(), - - ty::TyArray(ty, _) => { - tc_ty(tcx, ty, cache) - } - - ty::TySlice(ty) => { - tc_ty(tcx, ty, cache) - } - - ty::TyClosure(def_id, ref substs) => { - TypeContents::union( - substs.upvar_tys(def_id, tcx), - |ty| tc_ty(tcx, &ty, cache)) - } - - ty::TyTuple(ref tys, _) => { - TypeContents::union(&tys[..], - |ty| tc_ty(tcx, *ty, cache)) - } - - ty::TyAdt(def, substs) => { - TypeContents::union(&def.variants, |v| { - TypeContents::union(&v.fields, |f| { - tc_ty(tcx, f.ty(tcx, substs), cache) - }) - }) - - // unions don't have destructors regardless of the child types - - TypeContents::OWNS_DTOR.when(def.is_union()) - | TypeContents::OWNS_DTOR.when(def.has_dtor(tcx)) - } - - ty::TyDynamic(..) | - ty::TyProjection(..) | - ty::TyParam(_) | - ty::TyAnon(..) => TypeContents::OWNS_DTOR, - - ty::TyInfer(_) | - ty::TyError => { - bug!("asked to compute contents of error type"); - } - }; - - cache.insert(ty, result); - result - } - } -} diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 8b7438c0bfad2..a41629258716d 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -436,9 +436,6 @@ pub struct GlobalCtxt<'tcx> { // Internal cache for metadata decoding. No need to track deps on this. pub rcache: RefCell>>, - // Cache for the type-contents routine. FIXME -- track deps? - pub tc_cache: RefCell, ty::contents::TypeContents>>, - // FIXME dep tracking -- should be harmless enough pub normalized_cache: RefCell, Ty<'tcx>>>, @@ -708,7 +705,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { freevars: RefCell::new(resolutions.freevars), maybe_unused_trait_imports: resolutions.maybe_unused_trait_imports, rcache: RefCell::new(FxHashMap()), - tc_cache: RefCell::new(FxHashMap()), normalized_cache: RefCell::new(FxHashMap()), inhabitedness_cache: RefCell::new(FxHashMap()), lang_items: lang_items, diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 7325235ca7b76..b6e47568ff4db 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -71,7 +71,6 @@ pub use self::sty::InferTy::*; pub use self::sty::Region::*; pub use self::sty::TypeVariants::*; -pub use self::contents::TypeContents; pub use self::context::{TyCtxt, GlobalArenas, tls}; pub use self::context::{Lift, TypeckTables}; @@ -99,7 +98,6 @@ pub mod walk; pub mod wf; pub mod util; -mod contents; mod context; mod flags; mod instance; @@ -427,6 +425,8 @@ bitflags! { const MOVES_BY_DEFAULT = 1 << 19, const FREEZENESS_CACHED = 1 << 20, const IS_FREEZE = 1 << 21, + const MAY_DROP_CACHED = 1 << 22, + const MAY_DROP = 1 << 23, } } @@ -2400,19 +2400,18 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { if implements_copy { return false; } // ... (issue #22536 continued) but as an optimization, still use - // prior logic of asking if the `needs_drop` bit is set; we need - // not zero non-Copy types if they have no destructor. + // prior logic of asking for the structural `may_drop`. - // FIXME(#22815): Note that calling `ty::type_contents` is a - // conservative heuristic; it may report that `needs_drop` is set + // FIXME(#22815): Note that calling `ty::may_drop` is a + // conservative heuristic; it may report `true` ("may drop") // when actual type does not actually have a destructor associated // with it. But since `ty` absolutely did not have the `Copy` // bound attached (see above), it is sound to treat it as having a - // destructor (e.g. zero its memory on move). + // destructor. - let contents = ty.type_contents(tcx); - debug!("type_needs_drop ty={:?} contents={:?}", ty, contents.bits()); - contents.needs_drop(tcx) + let may_drop = ty.may_drop(tcx); + debug!("type_needs_drop ty={:?} may_drop={:?}", ty, may_drop); + may_drop } /// Get the attributes of a definition. diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index d43d570397b44..a3d2518909d74 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -21,7 +21,7 @@ use ty::fold::TypeVisitor; use ty::layout::{Layout, LayoutError}; use ty::TypeVariants::*; use util::common::ErrorReported; -use util::nodemap::FxHashMap; +use util::nodemap::{FxHashMap, FxHashSet}; use middle::lang_items; use rustc_const_math::{ConstInt, ConstIsize, ConstUsize}; @@ -699,6 +699,83 @@ impl<'a, 'tcx> ty::TyS<'tcx> { result } + #[inline] + pub fn may_drop(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { + if self.flags.get().intersects(TypeFlags::MAY_DROP_CACHED) { + return self.flags.get().intersects(TypeFlags::MAY_DROP); + } + + self.may_drop_inner(tcx, &mut FxHashSet()) + } + + fn may_drop_inner(&'tcx self, + tcx: TyCtxt<'a, 'tcx, 'tcx>, + visited: &mut FxHashSet>) + -> bool { + if self.flags.get().intersects(TypeFlags::MAY_DROP_CACHED) { + return self.flags.get().intersects(TypeFlags::MAY_DROP); + } + + // This should be reported as an error by `check_representable`. + // + // Consider the type as not needing drop in the meanwhile to avoid + // further errors. + if visited.replace(self).is_some() { + return false; + } + + assert!(!self.needs_infer()); + + let result = match self.sty { + // Fast-path for primitive types + ty::TyInfer(ty::FreshIntTy(_)) | ty::TyInfer(ty::FreshFloatTy(_)) | + ty::TyBool | ty::TyInt(_) | ty::TyUint(_) | ty::TyFloat(_) | ty::TyNever | + ty::TyFnDef(..) | ty::TyFnPtr(_) | ty::TyChar | + ty::TyRawPtr(_) | ty::TyRef(..) | ty::TyStr => false, + + // User destructors are the only way to have concrete drop types. + ty::TyAdt(def, _) if def.has_dtor(tcx) => true, + + // Can refer to a type which may drop. + // FIXME(eddyb) check this against a ParameterEnvironment. + ty::TyDynamic(..) | ty::TyProjection(..) | ty::TyParam(_) | + ty::TyAnon(..) | ty::TyInfer(_) | ty::TyError => true, + + // Structural recursion. + ty::TyArray(ty, _) | ty::TySlice(ty) => { + ty.may_drop_inner(tcx, visited) + } + + ty::TyClosure(def_id, ref substs) => { + substs.upvar_tys(def_id, tcx) + .any(|ty| ty.may_drop_inner(tcx, visited)) + } + + ty::TyTuple(ref tys, _) => { + tys.iter().any(|ty| ty.may_drop_inner(tcx, visited)) + } + + // unions don't have destructors regardless of the child types + ty::TyAdt(def, _) if def.is_union() => false, + + ty::TyAdt(def, substs) => { + def.variants.iter().any(|v| { + v.fields.iter().any(|f| { + f.ty(tcx, substs).may_drop_inner(tcx, visited) + }) + }) + } + }; + + self.flags.set(self.flags.get() | if result { + TypeFlags::MAY_DROP_CACHED | TypeFlags::MAY_DROP + } else { + TypeFlags::MAY_DROP_CACHED + }); + + result + } + #[inline] pub fn layout<'lcx>(&'tcx self, infcx: &InferCtxt<'a, 'tcx, 'lcx>) -> Result<&'tcx Layout, LayoutError<'tcx>> {