Call default_span lazily when query cycles occur instead of in the hot path for queries

src/librustc/ty/maps/plumbing.rs

@@ -17,6 +17,7 @@ use errors::DiagnosticBuilder;
 use errors::Level;
 use ty::tls;
 use ty::{TyCtxt};
+use ty::maps::Query;
 use ty::maps::config::QueryDescription;
 use ty::maps::job::{QueryResult, QueryInfo};
 use ty::item_path;
@@ -63,6 +64,7 @@ pub(super) trait GetCacheInternal<'tcx>: QueryDescription<'tcx> + Sized {
 
 #[derive(Clone)]
 pub(super) struct CycleError<'tcx> {
+    /// The span of the reason the first query in `cycle` ran the last query in `cycle`
     pub(super) span: Span,
     pub(super) cycle: Vec<QueryInfo<'tcx>>,
 }
@@ -79,27 +81,31 @@ pub(super) enum TryGetLock<'a, 'tcx: 'a, T, D: QueryDescription<'tcx> + 'a> {
 }
 
 impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
-    pub(super) fn report_cycle(self, CycleError { span, cycle: stack }: CycleError)
+    pub(super) fn report_cycle(self, CycleError { span, cycle: stack }: CycleError<'gcx>)
         -> DiagnosticBuilder<'a>
     {
         assert!(!stack.is_empty());
 
+        let fix_span = |span: Span, query: &Query<'gcx>| {
+            self.sess.codemap().def_span(query.default_span(self, span))
+        };
+
         // 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 span = self.sess.codemap().def_span(span);
+            let span = fix_span(span, &stack.first().unwrap().query);
             let mut err =
                 struct_span_err!(self.sess, span, E0391,
                                  "cyclic dependency detected");
             err.span_label(span, "cyclic reference");
 
-            err.span_note(self.sess.codemap().def_span(stack[0].span),
+            err.span_note(fix_span(stack[0].span, &stack[0].query),
                           &format!("the cycle begins when {}...", stack[0].query.describe(self)));
 
             for &QueryInfo { span, ref query, .. } in &stack[1..] {
-                err.span_note(self.sess.codemap().def_span(span),
+                err.span_note(fix_span(span, query),
                               &format!("...which then requires {}...", query.describe(self)));
             }
 
@@ -266,6 +272,22 @@ macro_rules! define_maps {
                     r
                 }
             }
+
+            // FIXME(eddyb) Get more valid Span's on queries.
+            pub fn default_span(&self, tcx: TyCtxt<'_, $tcx, '_>, span: Span) -> Span {
+                if span != DUMMY_SP {
+                    return span;
+                }
+                // The def_span query is used to calculate default_span,
+                // so exit to avoid infinite recursion
+                match *self {
+                    Query::def_span(..) => return span,
+                    _ => ()
+                }
+                match *self {
+                    $(Query::$name(key) => key.default_span(tcx),)*
+                }
+            }
         }
 
         pub mod queries {
@@ -303,7 +325,7 @@ macro_rules! define_maps {
             /// If the query already executed and panicked, this will fatal error / silently panic
             fn try_get_lock(
                 tcx: TyCtxt<'a, $tcx, 'lcx>,
-                mut span: Span,
+                span: Span,
                 key: &$K
             ) -> TryGetLock<'a, $tcx, $V, Self>
             {
@@ -329,13 +351,6 @@ macro_rules! define_maps {
                     };
                     mem::drop(lock);
 
-                    // This just matches the behavior of `try_get_with` so the span when
-                    // we await matches the span we would use when executing.
-                    // See the FIXME there.
-                    if span == DUMMY_SP && stringify!($name) != "def_span" {
-                        span = key.default_span(tcx);
-                    }
-
                     if let Err(cycle) = job.await(tcx, span) {
                         return TryGetLock::JobCompleted(Err(cycle));
                     }
@@ -343,7 +358,7 @@ macro_rules! define_maps {
             }
 
             fn try_get_with(tcx: TyCtxt<'a, $tcx, 'lcx>,
-                            mut span: Span,
+                            span: Span,
                             key: $K)
                             -> Result<$V, CycleError<$tcx>>
             {
@@ -377,18 +392,6 @@ macro_rules! define_maps {
 
                 let mut lock = get_lock_or_return!();
 
-                // FIXME(eddyb) Get more valid Span's on queries.
-                // def_span guard is necessary to prevent a recursive loop,
-                // default_span calls def_span query internally.
-                if span == DUMMY_SP && stringify!($name) != "def_span" {
-                    // This might deadlock if we hold the map lock since we might be
-                    // waiting for the def_span query and switch to some other fiber
-                    // So we drop the lock here and reacquire it
-                    mem::drop(lock);
-                    span = key.default_span(tcx);
-                    lock = get_lock_or_return!();
-                }
-
                 // Fast path for when incr. comp. is off. `to_dep_node` is
                 // expensive for some DepKinds.
                 if !tcx.dep_graph.is_fully_enabled() {