From 5841c687a39f7aed99cde543e60199b4dabff76b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Fri, 13 Apr 2018 22:20:10 +0200
Subject: [PATCH] Improve query cycle error message

---
 src/librustc/ty/maps/job.rs      | 14 +++++++++++--
 src/librustc/ty/maps/plumbing.rs | 34 ++++++++++++++++++--------------
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/src/librustc/ty/maps/job.rs b/src/librustc/ty/maps/job.rs
index 7d756fb16a4..374406158c1 100644
--- a/src/librustc/ty/maps/job.rs
+++ b/src/librustc/ty/maps/job.rs
@@ -31,6 +31,7 @@ pub(super) enum QueryResult<'tcx, T> {
 /// A span and a query key
 #[derive(Clone, Debug)]
 pub struct QueryInfo<'tcx> {
+    /// The span for a reason this query was required
     pub span: Span,
     pub query: Query<'tcx>,
 }
@@ -73,13 +74,22 @@ impl<'tcx> QueryJob<'tcx> {
             cycle.insert(0, job.info.clone());
 
             if &*job as *const _ == self as *const _ {
-                break;
+                // This is the end of the cycle
+                // The span entry we included was for the usage
+                // of the cycle itself, and not part of the cycle
+                // Replace it with the span which caused the cycle to form
+                cycle[0].span = span;
+                // Find out why the cycle itself was used
+                let usage = job.parent.as_ref().map(|parent| {
+                    (job.info.span, parent.info.query.clone())
+                });
+                return Err(CycleError { usage, cycle });
             }
 
             current_job = job.parent.clone();
         }
 
-        Err(CycleError { span, cycle })
+        panic!("did not find a cycle")
     }
 
     /// Signals to waiters that the query is complete.
diff --git a/src/librustc/ty/maps/plumbing.rs b/src/librustc/ty/maps/plumbing.rs
index 1475c36977a..003fe71b946 100644
--- a/src/librustc/ty/maps/plumbing.rs
+++ b/src/librustc/ty/maps/plumbing.rs
@@ -64,8 +64,8 @@ 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,
+    /// The query and related span which uses the cycle
+    pub(super) usage: Option<(Span, Query<'tcx>)>,
     pub(super) cycle: Vec<QueryInfo<'tcx>>,
 }
 
@@ -81,7 +81,7 @@ 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<'gcx>)
+    pub(super) fn report_cycle(self, CycleError { usage, cycle: stack }: CycleError<'gcx>)
         -> DiagnosticBuilder<'a>
     {
         assert!(!stack.is_empty());
@@ -95,23 +95,27 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
         // (And cycle errors around impls tend to occur during the
         // collect/coherence phases anyhow.)
         item_path::with_forced_impl_filename_line(|| {
-            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");
+            let span = fix_span(stack[1 % stack.len()].span, &stack[0].query);
+            let mut err = struct_span_err!(self.sess,
+                                           span,
+                                           E0391,
+                                           "cycle detected when {}",
+                                           stack[0].query.describe(self));
 
-            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(fix_span(span, query),
-                              &format!("...which then requires {}...", query.describe(self)));
+            for i in 1..stack.len() {
+                let query = &stack[i].query;
+                let span = fix_span(stack[(i + 1) % stack.len()].span, query);
+                err.span_note(span, &format!("...which requires {}...", query.describe(self)));
             }
 
-            err.note(&format!("...which then again requires {}, completing the cycle.",
+            err.note(&format!("...which again requires {}, completing the cycle",
                               stack[0].query.describe(self)));
 
+            if let Some((span, query)) = usage {
+                err.span_note(fix_span(span, &query),
+                              &format!("cycle used when {}", query.describe(self)));
+            }
+
             return err
         })
     }