From 1a87fc2635660bbf2947c17901dbd7dec75a63cb Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 17 Mar 2017 16:17:45 -0400 Subject: [PATCH 1/3] convert `custom_coerce_unsized_kind` into a `coerce_unsized_info` This "on-demand" task both checks for errors and computes the custom unsized kind, if any. This task is only defined on impls of `CoerceUnsized`; invoking it on any other kind of impl results in a bug. This is just to avoid having an `Option`, could easily be changed. --- src/librustc/ty/adjustment.rs | 15 ++++++++ src/librustc/ty/maps.rs | 4 +-- src/librustc/ty/mod.rs | 4 +-- src/librustc_metadata/cstore_impl.rs | 6 ++-- src/librustc_metadata/decoder.rs | 8 ++--- src/librustc_metadata/encoder.rs | 18 +++++++--- src/librustc_metadata/schema.rs | 4 ++- src/librustc_trans/monomorphize.rs | 2 +- src/librustc_typeck/coherence/builtin.rs | 46 ++++++++++++++++-------- src/librustc_typeck/coherence/mod.rs | 3 ++ 10 files changed, 77 insertions(+), 33 deletions(-) diff --git a/src/librustc/ty/adjustment.rs b/src/librustc/ty/adjustment.rs index 34977822bc69d..d8ca30477205c 100644 --- a/src/librustc/ty/adjustment.rs +++ b/src/librustc/ty/adjustment.rs @@ -139,6 +139,21 @@ pub enum AutoBorrow<'tcx> { RawPtr(hir::Mutability), } +/// Information for `CoerceUnsized` impls, storing information we +/// have computed about the coercion. +/// +/// This struct can be obtained via the `coerce_impl_info` query. +/// Demanding this struct also has the side-effect of reporting errors +/// for inappropriate impls. +#[derive(Clone, Copy, RustcEncodable, RustcDecodable, Debug)] +pub struct CoerceUnsizedInfo { + /// If this is a "custom coerce" impl, then what kind of custom + /// coercion is it? This applies to impls of `CoerceUnsized` for + /// structs, primarily, where we store a bit of info about which + /// fields need to be coerced. + pub custom_kind: Option +} + #[derive(Clone, Copy, RustcEncodable, RustcDecodable, Debug)] pub enum CustomCoerceUnsized { /// Records the index of the field being coerced. diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index ac8c38c7d5856..d4214dc429ced 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -393,8 +393,8 @@ define_maps! { <'tcx> pub closure_type: ItemSignature(DefId) -> ty::PolyFnSig<'tcx>, /// Caches CoerceUnsized kinds for impls on custom types. - pub custom_coerce_unsized_kind: ItemSignature(DefId) - -> ty::adjustment::CustomCoerceUnsized, + pub coerce_unsized_info: ItemSignature(DefId) + -> ty::adjustment::CoerceUnsizedInfo, pub typeck_tables: TypeckTables(DefId) -> &'tcx ty::TypeckTables<'tcx>, diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 36d1ae74e9114..d140426995812 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2054,8 +2054,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { }) } - pub fn custom_coerce_unsized_kind(self, did: DefId) -> adjustment::CustomCoerceUnsized { - queries::custom_coerce_unsized_kind::get(self, DUMMY_SP, did) + pub fn coerce_unsized_info(self, did: DefId) -> adjustment::CoerceUnsizedInfo { + queries::coerce_unsized_info::get(self, DUMMY_SP, did) } pub fn associated_item(self, def_id: DefId) -> AssociatedItem { diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 2a67b79eaa52e..7cac201f14f76 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -88,9 +88,9 @@ provide! { <'tcx> tcx, def_id, cdata } associated_item => { cdata.get_associated_item(def_id.index) } impl_trait_ref => { cdata.get_impl_trait(def_id.index, tcx) } - custom_coerce_unsized_kind => { - cdata.get_custom_coerce_unsized_kind(def_id.index).unwrap_or_else(|| { - bug!("custom_coerce_unsized_kind: `{:?}` is missing its kind", def_id); + coerce_unsized_info => { + cdata.get_coerce_unsized_info(def_id.index).unwrap_or_else(|| { + bug!("coerce_unsized_info: `{:?}` is missing its info", def_id); }) } mir => { diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 6ccdf8092f210..3de1e3442c69d 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -643,10 +643,10 @@ impl<'a, 'tcx> CrateMetadata { self.get_impl_data(id).polarity } - pub fn get_custom_coerce_unsized_kind(&self, - id: DefIndex) - -> Option { - self.get_impl_data(id).coerce_unsized_kind + pub fn get_coerce_unsized_info(&self, + id: DefIndex) + -> Option { + self.get_impl_data(id).coerce_unsized_info } pub fn get_impl_trait(&self, diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 044ed529ef74c..5bc84759f8797 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -695,7 +695,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let data = ImplData { polarity: hir::ImplPolarity::Positive, parent_impl: None, - coerce_unsized_kind: None, + coerce_unsized_info: None, trait_ref: tcx.impl_trait_ref(def_id).map(|trait_ref| self.lazy(&trait_ref)), }; @@ -715,13 +715,21 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { None }; + // if this is an impl of `CoerceUnsized`, create its + // "unsized info", else just store None + let coerce_unsized_info = + trait_ref.and_then(|t| { + if Some(t.def_id) == tcx.lang_items.coerce_unsized_trait() { + Some(ty::queries::coerce_unsized_info::get(tcx, item.span, def_id)) + } else { + None + } + }); + let data = ImplData { polarity: polarity, parent_impl: parent, - coerce_unsized_kind: tcx.maps.custom_coerce_unsized_kind - .borrow() - .get(&def_id) - .cloned(), + coerce_unsized_info: coerce_unsized_info, trait_ref: trait_ref.map(|trait_ref| self.lazy(&trait_ref)), }; diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index 4a20913d0b3fd..abb482a50ebc2 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -285,7 +285,9 @@ pub struct TraitData<'tcx> { pub struct ImplData<'tcx> { pub polarity: hir::ImplPolarity, pub parent_impl: Option, - pub coerce_unsized_kind: Option, + + /// This is `Some` only for impls of `CoerceUnsized`. + pub coerce_unsized_info: Option, pub trait_ref: Option>>, } diff --git a/src/librustc_trans/monomorphize.rs b/src/librustc_trans/monomorphize.rs index fcf6937d4b6d5..382ca8ef01001 100644 --- a/src/librustc_trans/monomorphize.rs +++ b/src/librustc_trans/monomorphize.rs @@ -287,7 +287,7 @@ pub fn custom_coerce_unsize_info<'scx, 'tcx>(scx: &SharedCrateContext<'scx, 'tcx match fulfill_obligation(scx, DUMMY_SP, trait_ref) { traits::VtableImpl(traits::VtableImplData { impl_def_id, .. }) => { - scx.tcx().custom_coerce_unsized_kind(impl_def_id) + scx.tcx().coerce_unsized_info(impl_def_id).custom_kind.unwrap() } vtable => { bug!("invalid CoerceUnsized vtable: {:?}", vtable); diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index 3cdf9fc93ae60..47b41a75cf531 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -18,6 +18,7 @@ use rustc::traits::{self, ObligationCause, Reveal}; use rustc::ty::{self, Ty, TyCtxt}; use rustc::ty::ParameterEnvironment; use rustc::ty::TypeFoldable; +use rustc::ty::adjustment::CoerceUnsizedInfo; use rustc::ty::subst::Subst; use rustc::ty::util::CopyImplementationError; use rustc::infer; @@ -159,11 +160,26 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - coerce_unsized_trait: DefId, + _: DefId, impl_did: DefId) { debug!("visit_implementation_of_coerce_unsized: impl_did={:?}", impl_did); + // Just compute this for the side-effects, in particular reporting + // errors; other parts of the code may demand it for the info of + // course. + if impl_did.is_local() { + let span = tcx.def_span(impl_did); + ty::queries::coerce_unsized_info::get(tcx, span, impl_did); + } +} + +pub fn coerce_unsized_info<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + impl_did: DefId) + -> CoerceUnsizedInfo { + debug!("compute_coerce_unsized_info(impl_did={:?})", impl_did); + let coerce_unsized_trait = tcx.lang_items.coerce_unsized_trait().unwrap(); + let unsize_trait = match tcx.lang_items.require(UnsizeTraitLangItem) { Ok(id) => id, Err(err) => { @@ -171,16 +187,14 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } }; - let impl_node_id = if let Some(n) = tcx.hir.as_local_node_id(impl_did) { - n - } else { - debug!("visit_implementation_of_coerce_unsized(): impl not \ - in this crate"); - return; - }; + // this provider should only get invoked for local def-ids + let impl_node_id = tcx.hir.as_local_node_id(impl_did).unwrap_or_else(|| { + bug!("coerce_unsized_info: invoked for non-local def-id {:?}", impl_did) + }); let source = tcx.item_type(impl_did); let trait_ref = tcx.impl_trait_ref(impl_did).unwrap(); + assert_eq!(trait_ref.def_id, coerce_unsized_trait); let target = trait_ref.substs.type_at(1); debug!("visit_implementation_of_coerce_unsized: {:?} -> {:?} (bound)", source, @@ -192,6 +206,8 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let target = target.subst(tcx, ¶m_env.free_substs); assert!(!source.has_escaping_regions()); + let err_info = CoerceUnsizedInfo { custom_kind: None }; + debug!("visit_implementation_of_coerce_unsized: {:?} -> {:?} (free)", source, target); @@ -234,7 +250,7 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, definition; expected {}, found {}", source_path, target_path); - return; + return err_info; } let fields = &def_a.struct_variant().fields; @@ -268,7 +284,7 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, "the trait `CoerceUnsized` may only be implemented \ for a coercion between structures with one field \ being coerced, none found"); - return; + return err_info; } else if diff_fields.len() > 1 { let item = tcx.hir.expect_item(impl_node_id); let span = if let ItemImpl(.., Some(ref t), _, _) = item.node { @@ -295,7 +311,7 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, .join(", "))); err.span_label(span, &format!("requires multiple coercions")); err.emit(); - return; + return err_info; } let (i, a, b) = diff_fields[0]; @@ -309,7 +325,7 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, E0376, "the trait `CoerceUnsized` may only be implemented \ for a coercion between structures"); - return; + return err_info; } }; @@ -331,8 +347,8 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, .caller_bounds); infcx.resolve_regions_and_report_errors(&free_regions, impl_node_id); - if let Some(kind) = kind { - tcx.maps.custom_coerce_unsized_kind.borrow_mut().insert(impl_did, kind); + CoerceUnsizedInfo { + custom_kind: kind } - }); + }) } diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 9ecf42daeaae5..6abd061e81f2a 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -102,9 +102,12 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt, impl_def_id: DefId, trait_d } pub fn provide(providers: &mut Providers) { + use self::builtin::coerce_unsized_info; + *providers = Providers { coherent_trait, coherent_inherent_impls, + coerce_unsized_info, ..*providers }; } From 8e6b10a6cb349deab30fa0fb507a8c73cae6ec68 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 20 Mar 2017 16:43:54 -0400 Subject: [PATCH 2/3] move `check` to the top of the file, where I would expect to find it Top-down, top-down! --- src/librustc_typeck/coherence/inherent.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/librustc_typeck/coherence/inherent.rs b/src/librustc_typeck/coherence/inherent.rs index e3b4ba9eb1b9e..9abf233de1f9e 100644 --- a/src/librustc_typeck/coherence/inherent.rs +++ b/src/librustc_typeck/coherence/inherent.rs @@ -19,6 +19,13 @@ use rustc::ty::{self, TyCtxt}; use syntax::ast; use syntax_pos::Span; +pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + tcx.visit_all_item_likes_in_krate(DepNode::CoherenceCheckImpl, + &mut InherentCollect { tcx }); + tcx.visit_all_item_likes_in_krate(DepNode::CoherenceOverlapCheckSpecial, + &mut InherentOverlapChecker { tcx }); +} + struct InherentCollect<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx> } @@ -348,9 +355,3 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentOverlapChecker<'a, 'tcx> { } } -pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - tcx.visit_all_item_likes_in_krate(DepNode::CoherenceCheckImpl, - &mut InherentCollect { tcx }); - tcx.visit_all_item_likes_in_krate(DepNode::CoherenceOverlapCheckSpecial, - &mut InherentOverlapChecker { tcx }); -} From a29ae3052aa9c84cecab226b9a1f1fee4e5b78d4 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 20 Mar 2017 18:35:16 -0400 Subject: [PATCH 3/3] convert inherent-impl-related things to on-demand queries There are now 3 queries: - inherent_impls(def-id): for a given type, get a `Rc>` with all its inherent impls. This internally uses `crate_inherent_impls`, doing some hacks to keep the current deps (which, btw, are not clearly correct). - crate_inherent_impls(crate): gathers up a map from types to `Rc>`, touching the entire krate, possibly generating errors. - crate_inherent_impls_overlap_check(crate): performs overlap checks between the inherent impls for a given type, generating errors. --- src/librustc/dep_graph/dep_node.rs | 2 - src/librustc/dep_graph/dep_tracking_map.rs | 15 -- src/librustc/middle/cstore.rs | 2 - src/librustc/ty/maps.rs | 26 ++- src/librustc/ty/mod.rs | 43 ++--- src/librustc_metadata/cstore_impl.rs | 7 +- src/librustc_typeck/check/method/probe.rs | 11 +- .../{inherent.rs => inherent_impls.rs} | 180 +++++++----------- .../coherence/inherent_impls_overlap.rs | 102 ++++++++++ src/librustc_typeck/coherence/mod.rs | 17 +- src/librustdoc/clean/inline.rs | 8 +- 11 files changed, 227 insertions(+), 186 deletions(-) rename src/librustc_typeck/coherence/{inherent.rs => inherent_impls.rs} (71%) create mode 100644 src/librustc_typeck/coherence/inherent_impls_overlap.rs diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 399af258e9251..c4cbbc17d51df 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -74,7 +74,6 @@ pub enum DepNode { CoherenceCheckImpl(D), CoherenceOverlapCheck(D), CoherenceOverlapCheckSpecial(D), - CoherenceOverlapInherentCheck(D), CoherenceOrphanCheck(D), Variance, WfCheck(D), @@ -251,7 +250,6 @@ impl DepNode { CoherenceCheckImpl(ref d) => op(d).map(CoherenceCheckImpl), CoherenceOverlapCheck(ref d) => op(d).map(CoherenceOverlapCheck), CoherenceOverlapCheckSpecial(ref d) => op(d).map(CoherenceOverlapCheckSpecial), - CoherenceOverlapInherentCheck(ref d) => op(d).map(CoherenceOverlapInherentCheck), CoherenceOrphanCheck(ref d) => op(d).map(CoherenceOrphanCheck), WfCheck(ref d) => op(d).map(WfCheck), TypeckItemType(ref d) => op(d).map(TypeckItemType), diff --git a/src/librustc/dep_graph/dep_tracking_map.rs b/src/librustc/dep_graph/dep_tracking_map.rs index 0f3108df9a822..b6a2360211cac 100644 --- a/src/librustc/dep_graph/dep_tracking_map.rs +++ b/src/librustc/dep_graph/dep_tracking_map.rs @@ -81,21 +81,6 @@ impl DepTrackingMap { pub fn keys(&self) -> Vec { self.map.keys().cloned().collect() } - - /// Append `elem` to the vector stored for `k`, creating a new vector if needed. - /// This is considered a write to `k`. - /// - /// NOTE: Caution is required when using this method. You should - /// be sure that nobody is **reading from the vector** while you - /// are writing to it. Eventually, it'd be nice to remove this. - pub fn push(&mut self, k: M::Key, elem: E) - where M: DepTrackingMapConfig> - { - self.write(&k); - self.map.entry(k) - .or_insert(Vec::new()) - .push(elem); - } } impl MemoizationMap for RefCell> { diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs index 225d6fc9bb2b2..50ec6f26d4b82 100644 --- a/src/librustc/middle/cstore.rs +++ b/src/librustc/middle/cstore.rs @@ -176,7 +176,6 @@ pub trait CrateStore { fn item_generics_cloned(&self, def: DefId) -> ty::Generics; fn item_attrs(&self, def_id: DefId) -> Vec; fn fn_arg_names(&self, did: DefId) -> Vec; - fn inherent_implementations_for_type(&self, def_id: DefId) -> Vec; // trait info fn implementations_of_trait(&self, filter: Option) -> Vec; @@ -310,7 +309,6 @@ impl CrateStore for DummyCrateStore { { bug!("item_generics_cloned") } fn item_attrs(&self, def_id: DefId) -> Vec { bug!("item_attrs") } fn fn_arg_names(&self, did: DefId) -> Vec { bug!("fn_arg_names") } - fn inherent_implementations_for_type(&self, def_id: DefId) -> Vec { vec![] } // trait info fn implementations_of_trait(&self, filter: Option) -> Vec { vec![] } diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index d4214dc429ced..87fe27d92fca6 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -12,7 +12,7 @@ use dep_graph::{DepGraph, DepNode, DepTrackingMap, DepTrackingMapConfig}; use hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use middle::const_val::ConstVal; use mir; -use ty::{self, Ty, TyCtxt}; +use ty::{self, CrateInherentImpls, Ty, TyCtxt}; use rustc_data_structures::indexed_vec::IndexVec; use std::cell::{RefCell, RefMut}; @@ -176,9 +176,15 @@ impl<'tcx> QueryDescription for queries::coherent_trait<'tcx> { } } -impl<'tcx> QueryDescription for queries::coherent_inherent_impls<'tcx> { +impl<'tcx> QueryDescription for queries::crate_inherent_impls<'tcx> { + fn describe(_: TyCtxt, k: CrateNum) -> String { + format!("all inherent impls defined in crate `{:?}`", k) + } +} + +impl<'tcx> QueryDescription for queries::crate_inherent_impls_overlap_check<'tcx> { fn describe(_: TyCtxt, _: CrateNum) -> String { - format!("coherence checking all inherent impls") + format!("check for overlap between inherent impls defined in this crate") } } @@ -368,7 +374,7 @@ define_maps! { <'tcx> /// Maps a DefId of a type to a list of its inherent impls. /// Contains implementations of methods that are inherent to a type. /// Methods in these implementations don't need to be exported. - pub inherent_impls: InherentImpls(DefId) -> Vec, + pub inherent_impls: InherentImpls(DefId) -> Rc>, /// Maps from the def-id of a function/method or const/static /// to its MIR. Mutation is done at an item granularity to @@ -400,7 +406,15 @@ define_maps! { <'tcx> pub coherent_trait: coherent_trait_dep_node((CrateNum, DefId)) -> (), - pub coherent_inherent_impls: coherent_inherent_impls_dep_node(CrateNum) -> (), + /// Gets a complete map from all types to their inherent impls. + /// Not meant to be used directly outside of coherence. + /// (Defined only for LOCAL_CRATE) + pub crate_inherent_impls: crate_inherent_impls_dep_node(CrateNum) -> CrateInherentImpls, + + /// Checks all types in the krate for overlap in their inherent impls. Reports errors. + /// Not meant to be used directly outside of coherence. + /// (Defined only for LOCAL_CRATE) + pub crate_inherent_impls_overlap_check: crate_inherent_impls_dep_node(CrateNum) -> (), /// Results of evaluating monomorphic constants embedded in /// other items, such as enum variant explicit discriminants. @@ -413,7 +427,7 @@ fn coherent_trait_dep_node((_, def_id): (CrateNum, DefId)) -> DepNode { DepNode::CoherenceCheckTrait(def_id) } -fn coherent_inherent_impls_dep_node(_: CrateNum) -> DepNode { +fn crate_inherent_impls_dep_node(_: CrateNum) -> DepNode { DepNode::Coherence } diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index d140426995812..d19f2ba2fadb1 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -31,7 +31,7 @@ use ty::subst::{Subst, Substs}; use ty::util::IntTypeExt; use ty::walk::TypeWalker; use util::common::MemoizationMap; -use util::nodemap::{NodeSet, FxHashMap}; +use util::nodemap::{NodeSet, DefIdMap, FxHashMap}; use serialize::{self, Encodable, Encoder}; use std::borrow::Cow; @@ -2345,34 +2345,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { def.flags.get().intersects(TraitFlags::HAS_DEFAULT_IMPL) } - /// Populates the type context with all the inherent implementations for - /// the given type if necessary. - pub fn populate_inherent_implementations_for_type_if_necessary(self, - span: Span, - type_id: DefId) { - if type_id.is_local() { - // Make sure coherence of inherent impls ran already. - ty::queries::coherent_inherent_impls::force(self, span, LOCAL_CRATE); - return - } - - // The type is not local, hence we are reading this out of - // metadata and don't need to track edges. - let _ignore = self.dep_graph.in_ignore(); - - if self.populated_external_types.borrow().contains(&type_id) { - return - } - - debug!("populate_inherent_implementations_for_type_if_necessary: searching for {:?}", - type_id); - - let inherent_impls = self.sess.cstore.inherent_implementations_for_type(type_id); - - self.maps.inherent_impls.borrow_mut().insert(type_id, inherent_impls); - self.populated_external_types.borrow_mut().insert(type_id); - } - /// Populates the type context with all the implementations for the given /// trait if necessary. pub fn populate_implementations_for_trait_if_necessary(self, trait_id: DefId) { @@ -2637,3 +2609,16 @@ pub fn provide(providers: &mut ty::maps::Providers) { ..*providers }; } + + +/// A map for the local crate mapping each type to a vector of its +/// inherent impls. This is not meant to be used outside of coherence; +/// rather, you should request the vector for a specific type via +/// `ty::queries::inherent_impls::get(def_id)` so as to minimize your +/// dependencies (constructing this map requires touching the entire +/// crate). +#[derive(Clone, Debug)] +pub struct CrateInherentImpls { + pub inherent_impls: DefIdMap>>, +} + diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 7cac201f14f76..9b781d28f88dc 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -109,6 +109,7 @@ provide! { <'tcx> tcx, def_id, cdata typeck_tables => { cdata.item_body_tables(def_id.index, tcx) } closure_kind => { cdata.closure_kind(def_id.index) } closure_type => { cdata.closure_ty(def_id.index, tcx) } + inherent_impls => { Rc::new(cdata.get_inherent_implementations_for_type(def_id.index)) } } impl CrateStore for cstore::CStore { @@ -162,12 +163,6 @@ impl CrateStore for cstore::CStore { self.get_crate_data(did.krate).get_fn_arg_names(did.index) } - fn inherent_implementations_for_type(&self, def_id: DefId) -> Vec - { - self.dep_graph.read(DepNode::MetaData(def_id)); - self.get_crate_data(def_id.krate).get_inherent_implementations_for_type(def_id.index) - } - fn implementations_of_trait(&self, filter: Option) -> Vec { if let Some(def_id) = filter { diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index dfa7ababca0bb..5b0418921563a 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -479,14 +479,9 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { } fn assemble_inherent_impl_candidates_for_type(&mut self, def_id: DefId) { - // Read the inherent implementation candidates for this type from the - // metadata if necessary. - self.tcx.populate_inherent_implementations_for_type_if_necessary(self.span, def_id); - - if let Some(impl_infos) = self.tcx.maps.inherent_impls.borrow().get(&def_id) { - for &impl_def_id in impl_infos.iter() { - self.assemble_inherent_impl_probe(impl_def_id); - } + let impl_def_ids = ty::queries::inherent_impls::get(self.tcx, self.span, def_id); + for &impl_def_id in impl_def_ids.iter() { + self.assemble_inherent_impl_probe(impl_def_id); } } diff --git a/src/librustc_typeck/coherence/inherent.rs b/src/librustc_typeck/coherence/inherent_impls.rs similarity index 71% rename from src/librustc_typeck/coherence/inherent.rs rename to src/librustc_typeck/coherence/inherent_impls.rs index 9abf233de1f9e..3a39df505eb07 100644 --- a/src/librustc_typeck/coherence/inherent.rs +++ b/src/librustc_typeck/coherence/inherent_impls.rs @@ -8,26 +8,82 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +//! The code in this module gathers up all of the inherent impls in +//! the current crate and organizes them in a map. It winds up +//! touching the whole crate and thus must be recomputed completely +//! for any change, but it is very cheap to compute. In practice, most +//! code in the compiler never *directly* requests this map. Instead, +//! it requests the inherent impls specific to some type (via +//! `ty::queries::inherent_impls::get(def_id)`). That value, however, +//! is computed by selecting an idea from this table. + use rustc::dep_graph::DepNode; -use rustc::hir::def_id::DefId; +use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; -use rustc::lint; -use rustc::traits::{self, Reveal}; -use rustc::ty::{self, TyCtxt}; +use rustc::ty::{self, CrateInherentImpls, TyCtxt}; +use rustc::util::nodemap::DefIdMap; +use std::rc::Rc; use syntax::ast; -use syntax_pos::Span; +use syntax_pos::{DUMMY_SP, Span}; + +/// On-demand query: yields a map containing all types mapped to their inherent impls. +pub fn crate_inherent_impls<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + crate_num: CrateNum) + -> CrateInherentImpls { + assert_eq!(crate_num, LOCAL_CRATE); + + let krate = tcx.hir.krate(); + let mut collect = InherentCollect { + tcx, + impls_map: CrateInherentImpls { + inherent_impls: DefIdMap() + } + }; + krate.visit_all_item_likes(&mut collect); + collect.impls_map +} + +/// On-demand query: yields a vector of the inherent impls for a specific type. +pub fn inherent_impls<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + ty_def_id: DefId) + -> Rc> { + assert!(ty_def_id.is_local()); + + // NB. Until we adopt the red-green dep-tracking algorithm (see + // [the plan] for details on that), we do some hackery here to get + // the dependencies correct. Basically, we use a `with_ignore` to + // read the result we want. If we didn't have the `with_ignore`, + // we would wind up with a dependency on the entire crate, which + // we don't want. Then we go and add dependencies on all the impls + // in the result (which is what we wanted). + // + // The result is a graph with an edge from `Hir(I)` for every impl + // `I` defined on some type `T` to `CoherentInherentImpls(T)`, + // thus ensuring that if any of those impls change, the set of + // inherent impls is considered dirty. + // + // [the plan]: https://github.com/rust-lang/rust-roadmap/issues/4 + + let result = tcx.dep_graph.with_ignore(|| { + let crate_map = ty::queries::crate_inherent_impls::get(tcx, DUMMY_SP, ty_def_id.krate); + match crate_map.inherent_impls.get(&ty_def_id) { + Some(v) => v.clone(), + None => Rc::new(vec![]), + } + }); + + for &impl_def_id in &result[..] { + tcx.dep_graph.read(DepNode::Hir(impl_def_id)); + } -pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - tcx.visit_all_item_likes_in_krate(DepNode::CoherenceCheckImpl, - &mut InherentCollect { tcx }); - tcx.visit_all_item_likes_in_krate(DepNode::CoherenceOverlapCheckSpecial, - &mut InherentOverlapChecker { tcx }); + result } struct InherentCollect<'a, 'tcx: 'a> { - tcx: TyCtxt<'a, 'tcx, 'tcx> + tcx: TyCtxt<'a, 'tcx, 'tcx>, + impls_map: CrateInherentImpls, } impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentCollect<'a, 'tcx> { @@ -216,25 +272,19 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentCollect<'a, 'tcx> { } impl<'a, 'tcx> InherentCollect<'a, 'tcx> { - fn check_def_id(&self, item: &hir::Item, def_id: DefId) { + fn check_def_id(&mut self, item: &hir::Item, def_id: DefId) { if def_id.is_local() { // Add the implementation to the mapping from implementation to base // type def ID, if there is a base type for this implementation and // the implementation does not have any associated traits. let impl_def_id = self.tcx.hir.local_def_id(item.id); + let mut rc_vec = self.impls_map.inherent_impls + .entry(def_id) + .or_insert_with(|| Rc::new(vec![])); - // Subtle: it'd be better to collect these into a local map - // and then write the vector only once all items are known, - // but that leads to degenerate dep-graphs. The problem is - // that the write of that big vector winds up having reads - // from *all* impls in the krate, since we've lost the - // precision basically. This would be ok in the firewall - // model so once we've made progess towards that we can modify - // the strategy here. In the meantime, using `push` is ok - // because we are doing this as a pre-pass before anyone - // actually reads from `inherent_impls` -- and we know this is - // true beacuse we hold the refcell lock. - self.tcx.maps.inherent_impls.borrow_mut().push(def_id, impl_def_id); + // At this point, there should not be any clones of the + // `Rc`, so we can still safely push into it in place: + Rc::get_mut(&mut rc_vec).unwrap().push(impl_def_id); } else { struct_span_err!(self.tcx.sess, item.span, @@ -273,85 +323,3 @@ impl<'a, 'tcx> InherentCollect<'a, 'tcx> { } } -struct InherentOverlapChecker<'a, 'tcx: 'a> { - tcx: TyCtxt<'a, 'tcx, 'tcx> -} - -impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { - fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId) { - #[derive(Copy, Clone, PartialEq)] - enum Namespace { - Type, - Value, - } - - let name_and_namespace = |def_id| { - let item = self.tcx.associated_item(def_id); - (item.name, match item.kind { - ty::AssociatedKind::Type => Namespace::Type, - ty::AssociatedKind::Const | - ty::AssociatedKind::Method => Namespace::Value, - }) - }; - - let impl_items1 = self.tcx.associated_item_def_ids(impl1); - let impl_items2 = self.tcx.associated_item_def_ids(impl2); - - for &item1 in &impl_items1[..] { - let (name, namespace) = name_and_namespace(item1); - - for &item2 in &impl_items2[..] { - if (name, namespace) == name_and_namespace(item2) { - let msg = format!("duplicate definitions with name `{}`", name); - let node_id = self.tcx.hir.as_local_node_id(item1).unwrap(); - self.tcx.sess.add_lint(lint::builtin::OVERLAPPING_INHERENT_IMPLS, - node_id, - self.tcx.span_of_impl(item1).unwrap(), - msg); - } - } - } - } - - fn check_for_overlapping_inherent_impls(&self, ty_def_id: DefId) { - let _task = self.tcx.dep_graph.in_task(DepNode::CoherenceOverlapInherentCheck(ty_def_id)); - - let inherent_impls = self.tcx.maps.inherent_impls.borrow(); - let impls = match inherent_impls.get(&ty_def_id) { - Some(impls) => impls, - None => return, - }; - - for (i, &impl1_def_id) in impls.iter().enumerate() { - for &impl2_def_id in &impls[(i + 1)..] { - self.tcx.infer_ctxt((), Reveal::UserFacing).enter(|infcx| { - if traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id).is_some() { - self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id) - } - }); - } - } - } -} - -impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentOverlapChecker<'a, 'tcx> { - fn visit_item(&mut self, item: &'v hir::Item) { - match item.node { - hir::ItemEnum(..) | - hir::ItemStruct(..) | - hir::ItemTrait(..) | - hir::ItemUnion(..) => { - let type_def_id = self.tcx.hir.local_def_id(item.id); - self.check_for_overlapping_inherent_impls(type_def_id); - } - _ => {} - } - } - - fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem) { - } - - fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) { - } -} - diff --git a/src/librustc_typeck/coherence/inherent_impls_overlap.rs b/src/librustc_typeck/coherence/inherent_impls_overlap.rs new file mode 100644 index 0000000000000..4b36072243c81 --- /dev/null +++ b/src/librustc_typeck/coherence/inherent_impls_overlap.rs @@ -0,0 +1,102 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE}; +use rustc::hir; +use rustc::hir::itemlikevisit::ItemLikeVisitor; +use rustc::lint; +use rustc::traits::{self, Reveal}; +use rustc::ty::{self, TyCtxt}; + +use syntax_pos::DUMMY_SP; + +pub fn crate_inherent_impls_overlap_check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + crate_num: CrateNum) { + assert_eq!(crate_num, LOCAL_CRATE); + let krate = tcx.hir.krate(); + krate.visit_all_item_likes(&mut InherentOverlapChecker { tcx }); +} + +struct InherentOverlapChecker<'a, 'tcx: 'a> { + tcx: TyCtxt<'a, 'tcx, 'tcx> +} + +impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> { + fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId) { + #[derive(Copy, Clone, PartialEq)] + enum Namespace { + Type, + Value, + } + + let name_and_namespace = |def_id| { + let item = self.tcx.associated_item(def_id); + (item.name, match item.kind { + ty::AssociatedKind::Type => Namespace::Type, + ty::AssociatedKind::Const | + ty::AssociatedKind::Method => Namespace::Value, + }) + }; + + let impl_items1 = self.tcx.associated_item_def_ids(impl1); + let impl_items2 = self.tcx.associated_item_def_ids(impl2); + + for &item1 in &impl_items1[..] { + let (name, namespace) = name_and_namespace(item1); + + for &item2 in &impl_items2[..] { + if (name, namespace) == name_and_namespace(item2) { + let msg = format!("duplicate definitions with name `{}`", name); + let node_id = self.tcx.hir.as_local_node_id(item1).unwrap(); + self.tcx.sess.add_lint(lint::builtin::OVERLAPPING_INHERENT_IMPLS, + node_id, + self.tcx.span_of_impl(item1).unwrap(), + msg); + } + } + } + } + + fn check_for_overlapping_inherent_impls(&self, ty_def_id: DefId) { + let impls = ty::queries::inherent_impls::get(self.tcx, DUMMY_SP, ty_def_id); + + for (i, &impl1_def_id) in impls.iter().enumerate() { + for &impl2_def_id in &impls[(i + 1)..] { + self.tcx.infer_ctxt((), Reveal::UserFacing).enter(|infcx| { + if traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id).is_some() { + self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id) + } + }); + } + } + } +} + +impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentOverlapChecker<'a, 'tcx> { + fn visit_item(&mut self, item: &'v hir::Item) { + match item.node { + hir::ItemEnum(..) | + hir::ItemStruct(..) | + hir::ItemTrait(..) | + hir::ItemUnion(..) => { + let type_def_id = self.tcx.hir.local_def_id(item.id); + self.check_for_overlapping_inherent_impls(type_def_id); + } + _ => {} + } + } + + fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem) { + } + + fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) { + } +} + diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 6abd061e81f2a..b3a7b612dd5b8 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -24,7 +24,8 @@ use syntax::ast; use syntax_pos::DUMMY_SP; mod builtin; -mod inherent; +mod inherent_impls; +mod inherent_impls_overlap; mod orphan; mod overlap; mod unsafety; @@ -103,10 +104,14 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt, impl_def_id: DefId, trait_d pub fn provide(providers: &mut Providers) { use self::builtin::coerce_unsized_info; + use self::inherent_impls::{crate_inherent_impls, inherent_impls}; + use self::inherent_impls_overlap::crate_inherent_impls_overlap_check; *providers = Providers { coherent_trait, - coherent_inherent_impls, + crate_inherent_impls, + inherent_impls, + crate_inherent_impls_overlap_check, coerce_unsized_info, ..*providers }; @@ -126,10 +131,6 @@ fn coherent_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, builtin::check_trait(tcx, def_id); } -fn coherent_inherent_impls<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, _: CrateNum) { - inherent::check(tcx); -} - pub fn check_coherence<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let _task = tcx.dep_graph.in_task(DepNode::Coherence); for &trait_def_id in tcx.hir.krate().trait_impls.keys() { @@ -140,5 +141,7 @@ pub fn check_coherence<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { orphan::check(tcx); overlap::check_default_impls(tcx); - ty::queries::coherent_inherent_impls::get(tcx, DUMMY_SP, LOCAL_CRATE); + // these queries are executed for side-effects (error reporting): + ty::queries::crate_inherent_impls::get(tcx, DUMMY_SP, LOCAL_CRATE); + ty::queries::crate_inherent_impls_overlap_check::get(tcx, DUMMY_SP, LOCAL_CRATE); } diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index c4476483186c7..cc30fdf56fc34 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -232,14 +232,12 @@ fn build_type_alias(cx: &DocContext, did: DefId) -> clean::Typedef { pub fn build_impls(cx: &DocContext, did: DefId) -> Vec { let tcx = cx.tcx; - tcx.populate_inherent_implementations_for_type_if_necessary(DUMMY_SP, did); let mut impls = Vec::new(); - if let Some(i) = tcx.maps.inherent_impls.borrow().get(&did) { - for &did in i.iter() { - build_impl(cx, did, &mut impls); - } + for &did in ty::queries::inherent_impls::get(tcx, DUMMY_SP, did).iter() { + build_impl(cx, did, &mut impls); } + // If this is the first time we've inlined something from another crate, then // we inline *all* impls from all the crates into this crate. Note that there's // currently no way for us to filter this based on type, and we likely need