From f35ff22fe8e2c4b7e05fda0820c6b84c9de86518 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 18 Apr 2017 10:53:35 -0400 Subject: [PATCH 1/6] do not access `inherited_impls` map directly --- src/librustc/middle/dead.rs | 15 +++++++-------- src/librustc_metadata/encoder.rs | 16 ++++++++-------- src/librustc_typeck/coherence/inherent_impls.rs | 6 +++++- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index 0840495ff77a8..622bf4dd0bd08 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -475,14 +475,13 @@ impl<'a, 'tcx> DeadVisitor<'a, 'tcx> { // This is done to handle the case where, for example, the static // method of a private type is used, but the type itself is never // called directly. - if let Some(impl_list) = - self.tcx.maps.inherent_impls.borrow().get(&self.tcx.hir.local_def_id(id)) { - for &impl_did in impl_list.iter() { - for &item_did in &self.tcx.associated_item_def_ids(impl_did)[..] { - if let Some(item_node_id) = self.tcx.hir.as_local_node_id(item_did) { - if self.live_symbols.contains(&item_node_id) { - return true; - } + let def_id = self.tcx.hir.local_def_id(id); + let inherent_impls = self.tcx.inherent_impls(def_id); + for &impl_did in inherent_impls.iter() { + for &item_did in &self.tcx.associated_item_def_ids(impl_did)[..] { + if let Some(item_node_id) = self.tcx.hir.as_local_node_id(item_did) { + if self.live_symbols.contains(&item_node_id) { + return true; } } } diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 3676e5a7f0f72..06df23f7cd0fb 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -626,14 +626,14 @@ impl<'a, 'b: 'a, 'tcx: 'b> EntryBuilder<'a, 'b, 'tcx> { // Encodes the inherent implementations of a structure, enumeration, or trait. fn encode_inherent_implementations(&mut self, def_id: DefId) -> LazySeq { debug!("EntryBuilder::encode_inherent_implementations({:?})", def_id); - match self.tcx.maps.inherent_impls.borrow().get(&def_id) { - None => LazySeq::empty(), - Some(implementations) => { - self.lazy_seq(implementations.iter().map(|&def_id| { - assert!(def_id.is_local()); - def_id.index - })) - } + let implementations = self.tcx.inherent_impls(def_id); + if implementations.is_empty() { + LazySeq::empty() + } else { + self.lazy_seq(implementations.iter().map(|&def_id| { + assert!(def_id.is_local()); + def_id.index + })) } } diff --git a/src/librustc_typeck/coherence/inherent_impls.rs b/src/librustc_typeck/coherence/inherent_impls.rs index 400aaf82fe428..238952865c7bd 100644 --- a/src/librustc_typeck/coherence/inherent_impls.rs +++ b/src/librustc_typeck/coherence/inherent_impls.rs @@ -66,11 +66,15 @@ pub fn inherent_impls<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // // [the plan]: https://github.com/rust-lang/rust-roadmap/issues/4 + thread_local! { + static EMPTY_DEF_ID_VEC: Rc> = Rc::new(vec![]) + } + let result = tcx.dep_graph.with_ignore(|| { let crate_map = tcx.crate_inherent_impls(ty_def_id.krate); match crate_map.inherent_impls.get(&ty_def_id) { Some(v) => v.clone(), - None => Rc::new(vec![]), + None => EMPTY_DEF_ID_VEC.with(|v| v.clone()) } }); From b11da34afbcd1f476c73e307158c41bd18a41bd8 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 18 Apr 2017 10:54:47 -0400 Subject: [PATCH 2/6] do not access `associated_item` map directly --- src/librustc/ty/mod.rs | 39 ++++++++++++++++++++++++-------- src/librustc_lint/bad_style.rs | 22 ++++++++---------- src/librustc_lint/builtin.rs | 2 +- src/librustc_metadata/decoder.rs | 2 +- 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 238791fb6edb3..3e3bb1f16720d 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2139,6 +2139,26 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { }) } + pub fn opt_associated_item(self, def_id: DefId) -> Option { + let is_associated_item = if let Some(node_id) = self.hir.as_local_node_id(def_id) { + match self.hir.get(node_id) { + hir_map::NodeTraitItem(_) | hir_map::NodeImplItem(_) => true, + _ => false, + } + } else { + match self.sess.cstore.describe_def(def_id).expect("no def for def-id") { + Def::AssociatedConst(_) | Def::Method(_) | Def::AssociatedTy(_) => true, + _ => false, + } + }; + + if is_associated_item { + Some(self.associated_item(def_id)) + } else { + None + } + } + fn associated_item_from_trait_item_ref(self, parent_def_id: DefId, parent_vis: &hir::Visibility, @@ -2391,7 +2411,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { None } } else { - self.maps.associated_item.borrow().get(&def_id).cloned() + self.opt_associated_item(def_id) }; match item { @@ -2412,15 +2432,13 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { if def_id.krate != LOCAL_CRATE { return self.sess.cstore.trait_of_item(def_id); } - match self.maps.associated_item.borrow().get(&def_id) { - Some(associated_item) => { + self.opt_associated_item(def_id) + .and_then(|associated_item| { match associated_item.container { TraitContainer(def_id) => Some(def_id), ImplContainer(_) => None } - } - None => None - } + }) } /// Construct a parameter environment suitable for static contexts or other contexts where there @@ -2588,11 +2606,12 @@ fn associated_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) } } - ref r => { - panic!("unexpected container of associated items: {:?}", r) - } + _ => { } } - panic!("associated item not found for def_id: {:?}", def_id); + + span_bug!(parent_item.span, + "unexpected parent of trait or impl item or item not found: {:?}", + parent_item.node) } /// Calculates the Sized-constraint. diff --git a/src/librustc_lint/bad_style.rs b/src/librustc_lint/bad_style.rs index c4220e9a0d3dc..d4b8f0a492461 100644 --- a/src/librustc_lint/bad_style.rs +++ b/src/librustc_lint/bad_style.rs @@ -27,19 +27,15 @@ pub enum MethodLateContext { PlainImpl, } -pub fn method_context(cx: &LateContext, id: ast::NodeId, span: Span) -> MethodLateContext { +pub fn method_context(cx: &LateContext, id: ast::NodeId) -> MethodLateContext { let def_id = cx.tcx.hir.local_def_id(id); - match cx.tcx.maps.associated_item.borrow().get(&def_id) { - None => span_bug!(span, "missing method descriptor?!"), - Some(item) => { - match item.container { - ty::TraitContainer(..) => MethodLateContext::TraitDefaultImpl, - ty::ImplContainer(cid) => { - match cx.tcx.impl_trait_ref(cid) { - Some(_) => MethodLateContext::TraitImpl, - None => MethodLateContext::PlainImpl, - } - } + let item = cx.tcx.associated_item(def_id); + match item.container { + ty::TraitContainer(..) => MethodLateContext::TraitDefaultImpl, + ty::ImplContainer(cid) => { + match cx.tcx.impl_trait_ref(cid) { + Some(_) => MethodLateContext::TraitImpl, + None => MethodLateContext::PlainImpl, } } } @@ -244,7 +240,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonSnakeCase { id: ast::NodeId) { match fk { FnKind::Method(name, ..) => { - match method_context(cx, id, span) { + match method_context(cx, id) { MethodLateContext::PlainImpl => { self.check_snake_case(cx, "method", &name.as_str(), Some(span)) } diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index c8644820ac0d0..57ed298809635 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -432,7 +432,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc { fn check_impl_item(&mut self, cx: &LateContext, impl_item: &hir::ImplItem) { // If the method is an impl for a trait, don't doc. - if method_context(cx, impl_item.id, impl_item.span) == MethodLateContext::TraitImpl { + if method_context(cx, impl_item.id) == MethodLateContext::TraitImpl { return; } diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 37913dad7eeb5..e7887d75daa33 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -827,7 +827,7 @@ impl<'a, 'tcx> CrateMetadata { EntryKind::AssociatedType(container) => { (ty::AssociatedKind::Type, container, false) } - _ => bug!() + _ => bug!("cannot get associated-item of `{:?}`", def_key) }; ty::AssociatedItem { From d26a7a48fe305a96b22afe8fe1da5ad01ed629cf Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 18 Apr 2017 10:55:19 -0400 Subject: [PATCH 3/6] do not access `typeck_tables` map directly --- src/librustc_save_analysis/dump_visitor.rs | 14 +++++--------- src/librustc_typeck/check_unused.rs | 12 ++++-------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 4639847651c74..34aae78ec5a87 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -114,15 +114,11 @@ impl<'l, 'tcx: 'l, 'll, D: Dump + 'll> DumpVisitor<'l, 'tcx, 'll, D> { where F: FnOnce(&mut DumpVisitor<'l, 'tcx, 'll, D>) { let item_def_id = self.tcx.hir.local_def_id(item_id); - match self.tcx.maps.typeck_tables_of.borrow().get(&item_def_id) { - Some(tables) => { - let old_tables = self.save_ctxt.tables; - self.save_ctxt.tables = tables; - f(self); - self.save_ctxt.tables = old_tables; - } - None => f(self), - } + let tables = self.tcx.typeck_tables_of(item_def_id); + let old_tables = self.save_ctxt.tables; + self.save_ctxt.tables = tables; + f(self); + self.save_ctxt.tables = old_tables; } pub fn dump_crate_info(&mut self, name: &str, krate: &ast::Crate) { diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index a985ac61cb3a9..e4f7f01735e73 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -66,14 +66,10 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let item_id = tcx.hir.body_owner(body_id); let item_def_id = tcx.hir.local_def_id(item_id); - // this will have been written by the main typeck pass - if let Some(tables) = tcx.maps.typeck_tables_of.borrow().get(&item_def_id) { - let imports = &tables.used_trait_imports; - debug!("GatherVisitor: item_def_id={:?} with imports {:#?}", item_def_id, imports); - used_trait_imports.extend(imports); - } else { - debug!("GatherVisitor: item_def_id={:?} with no imports", item_def_id); - } + let tables = tcx.typeck_tables_of(item_def_id); + let imports = &tables.used_trait_imports; + debug!("GatherVisitor: item_def_id={:?} with imports {:#?}", item_def_id, imports); + used_trait_imports.extend(imports); } let mut visitor = CheckVisitor { tcx, used_trait_imports }; From 1ac7eb8649a499177a2931221d77677e905100d0 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 18 Apr 2017 10:55:40 -0400 Subject: [PATCH 4/6] allow maps to be made private or public Didn't get around to removing all public access. --- src/librustc/ty/maps.rs | 76 ++++++++++++++++++++--------------------- src/librustc/ty/mod.rs | 2 +- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 096d69aa3760a..c6e5b5da0d614 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -274,11 +274,11 @@ impl<'tcx> QueryDescription for queries::describe_def<'tcx> { macro_rules! define_maps { (<$tcx:tt> $($(#[$attr:meta])* - pub $name:ident: $node:ident($K:ty) -> $V:ty),*) => { + [$($pub:tt)*] $name:ident: $node:ident($K:ty) -> $V:ty),*) => { pub struct Maps<$tcx> { providers: IndexVec>, query_stack: RefCell)>>, - $($(#[$attr])* pub $name: RefCell>>),* + $($(#[$attr])* $($pub)* $name: RefCell>>),* } impl<$tcx> Maps<$tcx> { @@ -441,12 +441,12 @@ macro_rules! define_maps { // the driver creates (using several `rustc_*` crates). define_maps! { <'tcx> /// Records the type of every item. - pub type_of: ItemSignature(DefId) -> Ty<'tcx>, + [pub] type_of: ItemSignature(DefId) -> Ty<'tcx>, /// Maps from the def-id of an item (trait/struct/enum/fn) to its /// associated generics and predicates. - pub generics_of: ItemSignature(DefId) -> &'tcx ty::Generics, - pub predicates_of: ItemSignature(DefId) -> ty::GenericPredicates<'tcx>, + [] generics_of: ItemSignature(DefId) -> &'tcx ty::Generics, + [] predicates_of: ItemSignature(DefId) -> ty::GenericPredicates<'tcx>, /// Maps from the def-id of a trait to the list of /// super-predicates. This is a subset of the full list of @@ -454,39 +454,39 @@ define_maps! { <'tcx> /// evaluate them even during type conversion, often before the /// full predicates are available (note that supertraits have /// additional acyclicity requirements). - pub super_predicates_of: ItemSignature(DefId) -> ty::GenericPredicates<'tcx>, + [] super_predicates_of: ItemSignature(DefId) -> ty::GenericPredicates<'tcx>, /// To avoid cycles within the predicates of a single item we compute /// per-type-parameter predicates for resolving `T::AssocTy`. - pub type_param_predicates: TypeParamPredicates((DefId, DefId)) + [] type_param_predicates: TypeParamPredicates((DefId, DefId)) -> ty::GenericPredicates<'tcx>, - pub trait_def: ItemSignature(DefId) -> &'tcx ty::TraitDef, - pub adt_def: ItemSignature(DefId) -> &'tcx ty::AdtDef, - pub adt_destructor: AdtDestructor(DefId) -> Option, - pub adt_sized_constraint: SizedConstraint(DefId) -> &'tcx [Ty<'tcx>], - pub adt_dtorck_constraint: DtorckConstraint(DefId) -> ty::DtorckConstraint<'tcx>, + [] trait_def: ItemSignature(DefId) -> &'tcx ty::TraitDef, + [] adt_def: ItemSignature(DefId) -> &'tcx ty::AdtDef, + [] adt_destructor: AdtDestructor(DefId) -> Option, + [] adt_sized_constraint: SizedConstraint(DefId) -> &'tcx [Ty<'tcx>], + [] adt_dtorck_constraint: DtorckConstraint(DefId) -> ty::DtorckConstraint<'tcx>, /// True if this is a foreign item (i.e., linked via `extern { ... }`). - pub is_foreign_item: IsForeignItem(DefId) -> bool, + [] is_foreign_item: IsForeignItem(DefId) -> bool, /// Maps from def-id of a type or region parameter to its /// (inferred) variance. - pub variances_of: ItemSignature(DefId) -> Rc>, + [pub] variances_of: ItemSignature(DefId) -> Rc>, /// Maps from an impl/trait def-id to a list of the def-ids of its items - pub associated_item_def_ids: AssociatedItemDefIds(DefId) -> Rc>, + [] associated_item_def_ids: AssociatedItemDefIds(DefId) -> Rc>, /// Maps from a trait item to the trait item "descriptor" - pub associated_item: AssociatedItems(DefId) -> ty::AssociatedItem, + [] associated_item: AssociatedItems(DefId) -> ty::AssociatedItem, - pub impl_trait_ref: ItemSignature(DefId) -> Option>, - pub impl_polarity: ItemSignature(DefId) -> hir::ImplPolarity, + [pub] impl_trait_ref: ItemSignature(DefId) -> Option>, + [] impl_polarity: ItemSignature(DefId) -> hir::ImplPolarity, /// 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) -> Rc>, + [] 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 @@ -495,59 +495,59 @@ define_maps! { <'tcx> /// /// Note that cross-crate MIR appears to be always borrowed /// (in the `RefCell` sense) to prevent accidental mutation. - pub mir: Mir(DefId) -> &'tcx RefCell>, + [pub] mir: Mir(DefId) -> &'tcx RefCell>, /// Maps DefId's that have an associated Mir to the result /// of the MIR qualify_consts pass. The actual meaning of /// the value isn't known except to the pass itself. - pub mir_const_qualif: Mir(DefId) -> u8, + [] mir_const_qualif: Mir(DefId) -> u8, /// Records the type of each closure. The def ID is the ID of the /// expression defining the closure. - pub closure_kind: ItemSignature(DefId) -> ty::ClosureKind, + [] closure_kind: ItemSignature(DefId) -> ty::ClosureKind, /// Records the type of each closure. The def ID is the ID of the /// expression defining the closure. - pub closure_type: ItemSignature(DefId) -> ty::PolyFnSig<'tcx>, + [] closure_type: ItemSignature(DefId) -> ty::PolyFnSig<'tcx>, /// Caches CoerceUnsized kinds for impls on custom types. - pub coerce_unsized_info: ItemSignature(DefId) + [] coerce_unsized_info: ItemSignature(DefId) -> ty::adjustment::CoerceUnsizedInfo, - pub typeck_item_bodies: typeck_item_bodies_dep_node(CrateNum) -> CompileResult, + [] typeck_item_bodies: typeck_item_bodies_dep_node(CrateNum) -> CompileResult, - pub typeck_tables_of: TypeckTables(DefId) -> &'tcx ty::TypeckTables<'tcx>, + [] typeck_tables_of: TypeckTables(DefId) -> &'tcx ty::TypeckTables<'tcx>, - pub coherent_trait: coherent_trait_dep_node((CrateNum, DefId)) -> (), + [] coherent_trait: coherent_trait_dep_node((CrateNum, DefId)) -> (), - pub borrowck: BorrowCheck(DefId) -> (), + [] borrowck: BorrowCheck(DefId) -> (), /// 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, + [] 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) -> (), + [] crate_inherent_impls_overlap_check: crate_inherent_impls_dep_node(CrateNum) -> (), /// Results of evaluating const items or constants embedded in /// other items (such as enum variant explicit discriminants). - pub const_eval: const_eval_dep_node((DefId, &'tcx Substs<'tcx>)) + [] const_eval: const_eval_dep_node((DefId, &'tcx Substs<'tcx>)) -> const_val::EvalResult<'tcx>, /// Performs the privacy check and computes "access levels". - pub privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Rc, + [] privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Rc, - pub reachable_set: reachability_dep_node(CrateNum) -> Rc, + [] reachable_set: reachability_dep_node(CrateNum) -> Rc, - pub mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell>, + [] mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell>, - pub def_symbol_name: SymbolName(DefId) -> ty::SymbolName, - pub symbol_name: symbol_name_dep_node(ty::Instance<'tcx>) -> ty::SymbolName, + [] def_symbol_name: SymbolName(DefId) -> ty::SymbolName, + [] symbol_name: symbol_name_dep_node(ty::Instance<'tcx>) -> ty::SymbolName, - pub describe_def: meta_data_node(DefId) -> Option + [] describe_def: meta_data_node(DefId) -> Option } fn coherent_trait_dep_node((_, def_id): (CrateNum, DefId)) -> DepNode { @@ -582,4 +582,4 @@ fn const_eval_dep_node((def_id, _): (DefId, &Substs)) -> DepNode { fn meta_data_node(def_id: DefId) -> DepNode { DepNode::MetaData(def_id) -} \ No newline at end of file +} diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 3e3bb1f16720d..ff4bded012e9e 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2146,7 +2146,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { _ => false, } } else { - match self.sess.cstore.describe_def(def_id).expect("no def for def-id") { + match self.describe_def(def_id).expect("no def for def-id") { Def::AssociatedConst(_) | Def::Method(_) | Def::AssociatedTy(_) => true, _ => false, } From 2cca2567d9a39f159e392c47c8be6d94087242c9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 18 Apr 2017 11:50:21 -0400 Subject: [PATCH 5/6] make `ty` and `impl_trait_ref` private This requires copying out the cycle error to avoid a cyclic borrow. Is this a problem? Are there paths where we expect cycles to arise and not result in errors? (In such cases, we could add a faster way to test for cycle.) --- src/librustc/ty/item_path.rs | 43 ++++++++++++++++++++-------- src/librustc/ty/maps.rs | 55 ++++++++++++++++++++++++------------ 2 files changed, 68 insertions(+), 30 deletions(-) diff --git a/src/librustc/ty/item_path.rs b/src/librustc/ty/item_path.rs index eb31dfba4a4d8..16d5d1187fc8b 100644 --- a/src/librustc/ty/item_path.rs +++ b/src/librustc/ty/item_path.rs @@ -13,16 +13,19 @@ use hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use ty::{self, Ty, TyCtxt}; use syntax::ast; use syntax::symbol::Symbol; +use syntax_pos::DUMMY_SP; use std::cell::Cell; thread_local! { - static FORCE_ABSOLUTE: Cell = Cell::new(false) + static FORCE_ABSOLUTE: Cell = Cell::new(false); + static FORCE_IMPL_FILENAME_LINE: Cell = Cell::new(false); } -/// Enforces that item_path_str always returns an absolute path. -/// This is useful when building symbols that contain types, -/// where we want the crate name to be part of the symbol. +/// Enforces that item_path_str always returns an absolute path and +/// also enables "type-based" impl paths. This is used when building +/// symbols that contain types, where we want the crate name to be +/// part of the symbol. pub fn with_forced_absolute_paths R, R>(f: F) -> R { FORCE_ABSOLUTE.with(|force| { let old = force.get(); @@ -33,6 +36,20 @@ pub fn with_forced_absolute_paths R, R>(f: F) -> R { }) } +/// Force us to name impls with just the filename/line number. We +/// normally try to use types. But at some points, notably while printing +/// cycle errors, this can result in extra or suboptimal error output, +/// so this variable disables that check. +pub fn with_forced_impl_filename_line R, R>(f: F) -> R { + FORCE_IMPL_FILENAME_LINE.with(|force| { + let old = force.get(); + force.set(true); + let result = f(); + force.set(old); + result + }) +} + impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { /// Returns a string identifying this def-id. This string is /// suitable for user output. It is relative to the current crate @@ -199,14 +216,16 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { { let parent_def_id = self.parent_def_id(impl_def_id).unwrap(); - let use_types = if !impl_def_id.is_local() { - // always have full types available for extern crates - true - } else { - // for local crates, check whether type info is - // available; typeck might not have completed yet - self.maps.impl_trait_ref.borrow().contains_key(&impl_def_id) && - self.maps.type_of.borrow().contains_key(&impl_def_id) + // Always use types for non-local impls, where types are always + // available, and filename/line-number is mostly uninteresting. + let use_types = !impl_def_id.is_local() || { + // Otherwise, use filename/line-number if forced. + let force_no_types = FORCE_IMPL_FILENAME_LINE.with(|f| f.get()); + !force_no_types && { + // Otherwise, use types if we can query them without inducing a cycle. + ty::queries::impl_trait_ref::try_get(self, DUMMY_SP, impl_def_id).is_ok() && + ty::queries::type_of::try_get(self, DUMMY_SP, impl_def_id).is_ok() + } }; if !use_types { diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index c6e5b5da0d614..93f3916d1b501 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -17,11 +17,13 @@ use middle::privacy::AccessLevels; use mir; use session::CompileResult; use ty::{self, CrateInherentImpls, Ty, TyCtxt}; +use ty::item_path; use ty::subst::Substs; use util::nodemap::NodeSet; use rustc_data_structures::indexed_vec::IndexVec; use std::cell::{RefCell, RefMut}; +use std::mem; use std::ops::Deref; use std::rc::Rc; use syntax_pos::{Span, DUMMY_SP}; @@ -139,24 +141,36 @@ pub struct CycleError<'a, 'tcx: 'a> { impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { pub fn report_cycle(self, CycleError { span, cycle }: CycleError) { - assert!(!cycle.is_empty()); - - let mut err = struct_span_err!(self.sess, span, E0391, - "unsupported cyclic reference between types/traits detected"); - err.span_label(span, &format!("cyclic reference")); - - err.span_note(cycle[0].0, &format!("the cycle begins when {}...", - cycle[0].1.describe(self))); - - for &(span, ref query) in &cycle[1..] { - err.span_note(span, &format!("...which then requires {}...", - query.describe(self))); - } + // Subtle: release the refcell lock before invoking `describe()` + // below by dropping `cycle`. + let stack = cycle.to_vec(); + mem::drop(cycle); + + assert!(!stack.is_empty()); + + // Disable naming impls with types in this path, since that + // sometimes cycles itself, leading to extra cycle errors. + // (And cycle errors around impls tend to occur during the + // collect/coherence phases anyhow.) + item_path::with_forced_impl_filename_line(|| { + let mut err = + struct_span_err!(self.sess, span, E0391, + "unsupported cyclic reference between types/traits detected"); + err.span_label(span, &format!("cyclic reference")); + + err.span_note(stack[0].0, &format!("the cycle begins when {}...", + stack[0].1.describe(self))); + + for &(span, ref query) in &stack[1..] { + err.span_note(span, &format!("...which then requires {}...", + query.describe(self))); + } - err.note(&format!("...which then again requires {}, completing the cycle.", - cycle[0].1.describe(self))); + err.note(&format!("...which then again requires {}, completing the cycle.", + stack[0].1.describe(self))); - err.emit(); + err.emit(); + }); } fn cycle_check(self, span: Span, query: Query<'gcx>, compute: F) @@ -335,6 +349,11 @@ macro_rules! define_maps { -> Result> where F: FnOnce(&$V) -> R { + debug!("ty::queries::{}::try_get_with(key={:?}, span={:?})", + stringify!($name), + key, + span); + if let Some(result) = tcx.maps.$name.borrow().get(&key) { return Ok(f(result)); } @@ -441,7 +460,7 @@ macro_rules! define_maps { // the driver creates (using several `rustc_*` crates). define_maps! { <'tcx> /// Records the type of every item. - [pub] type_of: ItemSignature(DefId) -> Ty<'tcx>, + [] type_of: ItemSignature(DefId) -> Ty<'tcx>, /// Maps from the def-id of an item (trait/struct/enum/fn) to its /// associated generics and predicates. @@ -480,7 +499,7 @@ define_maps! { <'tcx> /// Maps from a trait item to the trait item "descriptor" [] associated_item: AssociatedItems(DefId) -> ty::AssociatedItem, - [pub] impl_trait_ref: ItemSignature(DefId) -> Option>, + [] impl_trait_ref: ItemSignature(DefId) -> Option>, [] impl_polarity: ItemSignature(DefId) -> hir::ImplPolarity, /// Maps a DefId of a type to a list of its inherent impls. From d7d3f197f62d7d0a1a612b7243f4617428cae53f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 28 Apr 2017 09:52:56 -0400 Subject: [PATCH 6/6] introduce ability to if we have typeck-tables for a given def-id And use this in save-analysis, which used to read the map directly. This is an attempt to sidestep the failure occuring on homu that I cannot reproduce. --- src/librustc/ty/maps.rs | 2 + src/librustc_save_analysis/dump_visitor.rs | 14 ++- src/librustc_typeck/check/mod.rs | 117 +++++++++++++-------- src/librustc_typeck/check_unused.rs | 4 +- 4 files changed, 84 insertions(+), 53 deletions(-) diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index 93f3916d1b501..1749c90b5892a 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -537,6 +537,8 @@ define_maps! { <'tcx> [] typeck_tables_of: TypeckTables(DefId) -> &'tcx ty::TypeckTables<'tcx>, + [] has_typeck_tables: TypeckTables(DefId) -> bool, + [] coherent_trait: coherent_trait_dep_node((CrateNum, DefId)) -> (), [] borrowck: BorrowCheck(DefId) -> (), diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index 34aae78ec5a87..4d75206ed9c7c 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -114,11 +114,15 @@ impl<'l, 'tcx: 'l, 'll, D: Dump + 'll> DumpVisitor<'l, 'tcx, 'll, D> { where F: FnOnce(&mut DumpVisitor<'l, 'tcx, 'll, D>) { let item_def_id = self.tcx.hir.local_def_id(item_id); - let tables = self.tcx.typeck_tables_of(item_def_id); - let old_tables = self.save_ctxt.tables; - self.save_ctxt.tables = tables; - f(self); - self.save_ctxt.tables = old_tables; + if self.tcx.has_typeck_tables(item_def_id) { + let tables = self.tcx.typeck_tables_of(item_def_id); + let old_tables = self.save_ctxt.tables; + self.save_ctxt.tables = tables; + f(self); + self.save_ctxt.tables = old_tables; + } else { + f(self) + } } pub fn dump_crate_info(&mut self, name: &str, krate: &ast::Crate) { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index fe003f3f24230..0186755e30a62 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -637,6 +637,7 @@ pub fn provide(providers: &mut Providers) { *providers = Providers { typeck_item_bodies, typeck_tables_of, + has_typeck_tables, closure_type, closure_kind, adt_destructor, @@ -664,55 +665,49 @@ fn adt_destructor<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tcx.calculate_dtor(def_id, &mut dropck::check_drop_impl) } -fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - def_id: DefId) - -> &'tcx ty::TypeckTables<'tcx> { - // Closures' tables come from their outermost function, - // as they are part of the same "inference environment". - let outer_def_id = tcx.closure_base_def_id(def_id); - if outer_def_id != def_id { - return tcx.typeck_tables_of(outer_def_id); - } - - let id = tcx.hir.as_local_node_id(def_id).unwrap(); - let span = tcx.hir.span(id); - let unsupported = || { - span_bug!(span, "can't type-check body of {:?}", def_id); - }; - - // Figure out what primary body this item has. - let mut fn_decl = None; - let body_id = match tcx.hir.get(id) { +/// If this def-id is a "primary tables entry", returns `Some((body_id, decl))` +/// with information about it's body-id and fn-decl (if any). Otherwise, +/// returns `None`. +/// +/// If this function returns "some", then `typeck_tables(def_id)` will +/// succeed; if it returns `None`, then `typeck_tables(def_id)` may or +/// may not succeed. In some cases where this function returns `None` +/// (notably closures), `typeck_tables(def_id)` would wind up +/// redirecting to the owning function. +fn primary_body_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + id: ast::NodeId) + -> Option<(hir::BodyId, Option<&'tcx hir::FnDecl>)> +{ + match tcx.hir.get(id) { hir::map::NodeItem(item) => { match item.node { hir::ItemConst(_, body) | - hir::ItemStatic(_, _, body) => body, - hir::ItemFn(ref decl, .., body) => { - fn_decl = Some(decl); - body - } - _ => unsupported() + hir::ItemStatic(_, _, body) => + Some((body, None)), + hir::ItemFn(ref decl, .., body) => + Some((body, Some(decl))), + _ => + None, } } hir::map::NodeTraitItem(item) => { match item.node { - hir::TraitItemKind::Const(_, Some(body)) => body, - hir::TraitItemKind::Method(ref sig, - hir::TraitMethod::Provided(body)) => { - fn_decl = Some(&sig.decl); - body - } - _ => unsupported() + hir::TraitItemKind::Const(_, Some(body)) => + Some((body, None)), + hir::TraitItemKind::Method(ref sig, hir::TraitMethod::Provided(body)) => + Some((body, Some(&sig.decl))), + _ => + None, } } hir::map::NodeImplItem(item) => { match item.node { - hir::ImplItemKind::Const(_, body) => body, - hir::ImplItemKind::Method(ref sig, body) => { - fn_decl = Some(&sig.decl); - body - } - _ => unsupported() + hir::ImplItemKind::Const(_, body) => + Some((body, None)), + hir::ImplItemKind::Method(ref sig, body) => + Some((body, Some(&sig.decl))), + _ => + None, } } hir::map::NodeExpr(expr) => { @@ -723,15 +718,47 @@ fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // Assume that everything other than closures // is a constant "initializer" expression. match expr.node { - hir::ExprClosure(..) => { - // We should've bailed out above for closures. - span_bug!(expr.span, "unexpected closure") - } - _ => hir::BodyId { node_id: expr.id } + hir::ExprClosure(..) => + None, + _ => + Some((hir::BodyId { node_id: expr.id }, None)), } } - _ => unsupported() - }; + _ => None, + } +} + +fn has_typeck_tables<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + def_id: DefId) + -> bool { + // Closures' tables come from their outermost function, + // as they are part of the same "inference environment". + let outer_def_id = tcx.closure_base_def_id(def_id); + if outer_def_id != def_id { + return tcx.has_typeck_tables(outer_def_id); + } + + let id = tcx.hir.as_local_node_id(def_id).unwrap(); + primary_body_of(tcx, id).is_some() +} + +fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + def_id: DefId) + -> &'tcx ty::TypeckTables<'tcx> { + // Closures' tables come from their outermost function, + // as they are part of the same "inference environment". + let outer_def_id = tcx.closure_base_def_id(def_id); + if outer_def_id != def_id { + return tcx.typeck_tables_of(outer_def_id); + } + + let id = tcx.hir.as_local_node_id(def_id).unwrap(); + let span = tcx.hir.span(id); + + // Figure out what primary body this item has. + let (body_id, fn_decl) = primary_body_of(tcx, id).unwrap_or_else(|| { + span_bug!(span, "can't type-check body of {:?}", def_id); + }); let body = tcx.hir.body(body_id); Inherited::build(tcx, id).enter(|inh| { diff --git a/src/librustc_typeck/check_unused.rs b/src/librustc_typeck/check_unused.rs index e4f7f01735e73..1af55d4d840d9 100644 --- a/src/librustc_typeck/check_unused.rs +++ b/src/librustc_typeck/check_unused.rs @@ -63,9 +63,7 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CheckVisitor<'a, 'tcx> { pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { let mut used_trait_imports = DefIdSet(); for &body_id in tcx.hir.krate().bodies.keys() { - let item_id = tcx.hir.body_owner(body_id); - let item_def_id = tcx.hir.local_def_id(item_id); - + let item_def_id = tcx.hir.body_owner_def_id(body_id); let tables = tcx.typeck_tables_of(item_def_id); let imports = &tables.used_trait_imports; debug!("GatherVisitor: item_def_id={:?} with imports {:#?}", item_def_id, imports);