Rollup merge of #41368 - nikomatsakis:incr-comp-dep-tracking-map, r=eddyb

make *most* maps private

Currently we access the `DepTrackingMap` fields directly rather than using the query accessors. This seems bad. This branch removes several such uses, but not all, and extends the macro so that queries can hide their maps (so we can prevent regressions). The extension to the macro is kind of ugly :/ but couldn't find a simple way to do it otherwise (I guess I could use a nested macro...). Anyway I figure it's only temporary.

r? @eddyb

Author: frewsxcv

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;
                     }
                 }
             }

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<bool> = Cell::new(false)
+    static FORCE_ABSOLUTE: Cell<bool> = Cell::new(false);
+    static FORCE_IMPL_FILENAME_LINE: Cell<bool> = 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<F: FnOnce() -> R, R>(f: F) -> R {
     FORCE_ABSOLUTE.with(|force| {
         let old = force.get();
@@ -33,6 +36,20 @@ pub fn with_forced_absolute_paths<F: FnOnce() -> 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<F: FnOnce() -> 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
@@ -196,14 +213,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 {

src/librustc/ty/maps.rs

@@ -16,11 +16,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};
@@ -122,24 +124,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<F, R>(self, span: Span, query: Query<'gcx>, compute: F)
@@ -245,11 +259,11 @@ impl<'tcx> QueryDescription for queries::const_eval<'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<CrateNum, Providers<$tcx>>,
             query_stack: RefCell<Vec<(Span, Query<$tcx>)>>,
-            $($(#[$attr])* pub $name: RefCell<DepTrackingMap<queries::$name<$tcx>>>),*
+            $($(#[$attr])* $($pub)* $name: RefCell<DepTrackingMap<queries::$name<$tcx>>>),*
         }
 
         impl<$tcx> Maps<$tcx> {
@@ -306,6 +320,11 @@ macro_rules! define_maps {
                                   -> Result<R, CycleError<'a, $tcx>>
                 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));
                 }
@@ -412,52 +431,52 @@ 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.
-    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
     /// predicates. We store these in a separate map because we must
     /// 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<ty::Destructor>,
-    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<ty::Destructor>,
+    [] 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<Vec<ty::Variance>>,
+    [pub] variances_of: ItemSignature(DefId) -> Rc<Vec<ty::Variance>>,
 
     /// 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<Vec<DefId>>,
+    [] associated_item_def_ids: AssociatedItemDefIds(DefId) -> Rc<Vec<DefId>>,
 
     /// 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<ty::TraitRef<'tcx>>,
-    pub impl_polarity: ItemSignature(DefId) -> hir::ImplPolarity,
+    [] impl_trait_ref: ItemSignature(DefId) -> Option<ty::TraitRef<'tcx>>,
+    [] 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<Vec<DefId>>,
+    [] inherent_impls: InherentImpls(DefId) -> Rc<Vec<DefId>>,
 
     /// Maps from the def-id of a function/method or const/static
     /// to its MIR. Mutation is done at an item granularity to
@@ -466,54 +485,54 @@ 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<mir::Mir<'tcx>>,
+    [pub] mir: Mir(DefId) -> &'tcx RefCell<mir::Mir<'tcx>>,
 
     /// 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<AccessLevels>,
+    [] privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Rc<AccessLevels>,
 
-    pub reachable_set: reachability_dep_node(CrateNum) -> Rc<NodeSet>,
+    [] reachable_set: reachability_dep_node(CrateNum) -> Rc<NodeSet>,
 
-    pub mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell<mir::Mir<'tcx>>
+    [] mir_shims: mir_shim_dep_node(ty::InstanceDef<'tcx>) -> &'tcx RefCell<mir::Mir<'tcx>>
 }
 
 fn coherent_trait_dep_node((_, def_id): (CrateNum, DefId)) -> DepNode<DefId> {

src/librustc/ty/mod.rs

@@ -2138,6 +2138,26 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         })
     }
 
+    pub fn opt_associated_item(self, def_id: DefId) -> Option<AssociatedItem> {
+        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,
@@ -2390,7 +2410,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 {
@@ -2411,15 +2431,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
@@ -2587,11 +2605,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.