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.)

Author: nikomatsakis

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
@@ -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 {

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<F, R>(self, span: Span, query: Query<'gcx>, compute: F)
@@ -335,6 +349,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));
                 }
@@ -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<ty::TraitRef<'tcx>>,
+    [] 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.