From ecd5efa641b7282548f2fd16f3576621ebb34662 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Sun, 2 Dec 2018 03:01:12 +0100
Subject: [PATCH 1/4] Show usages of query cycles and correctly shift queries
in a cycle
---
src/librustc/ty/query/job.rs | 91 ++++++++++++++++++++++++------------
1 file changed, 61 insertions(+), 30 deletions(-)
diff --git a/src/librustc/ty/query/job.rs b/src/librustc/ty/query/job.rs
index d588bc8c0cb5c..d5bf3856bc432 100644
--- a/src/librustc/ty/query/job.rs
+++ b/src/librustc/ty/query/job.rs
@@ -326,19 +326,17 @@ fn connected_to_root<'tcx>(
query: Lrc<QueryJob<'tcx>>,
visited: &mut FxHashSet<*const QueryJob<'tcx>>
) -> bool {
- // This query is connected to the root (it has no query parent), return true
- if query.parent.is_none() {
- return true;
- }
-
// We already visited this or we're deliberately ignoring it
if visited.contains(&query.as_ptr()) {
return false;
}
- visited.insert(query.as_ptr());
+ // This query is connected to the root (it has no query parent), return true
+ if query.parent.is_none() {
+ return true;
+ }
- let mut connected = false;
+ visited.insert(query.as_ptr());
visit_waiters(query, |_, successor| {
if connected_to_root(successor, visited) {
@@ -349,6 +347,28 @@ fn connected_to_root<'tcx>(
}).is_some()
}
+// Deterministically pick an query from a list
+#[cfg(parallel_queries)]
+fn pick_query<'a, 'tcx, T, F: Fn(&T) -> (Span, Lrc<QueryJob<'tcx>>)>(
+ tcx: TyCtxt<'_, 'tcx, '_>,
+ queries: &'a [T],
+ f: F
+) -> &'a T {
+ // Deterministically pick an entry point
+ // FIXME: Sort this instead
+ let mut hcx = tcx.create_stable_hashing_context();
+ queries.iter().min_by_key(|v| {
+ let (span, query) = f(v);
+ let mut stable_hasher = StableHasher::<u64>::new();
+ query.info.query.hash_stable(&mut hcx, &mut stable_hasher);
+ // Prefer entry points which have valid spans for nicer error messages
+ // We add an integer to the tuple ensuring that entry points
+ // with valid spans are picked first
+ let span_cmp = if span == DUMMY_SP { 1 } else { 0 };
+ (span_cmp, stable_hasher.finish())
+ }).unwrap()
+}
+
/// Looks for query cycles starting from the last query in `jobs`.
/// If a cycle is found, all queries in the cycle is removed from `jobs` and
/// the function return true.
@@ -388,41 +408,52 @@ fn remove_cycle<'tcx>(
// Find the queries in the cycle which are
// connected to queries outside the cycle
- let entry_points = stack.iter().filter_map(|query| {
- // Mark all the other queries in the cycle as already visited
- let mut visited = FxHashSet::from_iter(stack.iter().filter_map(|q| {
- if q.1.as_ptr() != query.1.as_ptr() {
- Some(q.1.as_ptr())
- } else {
+ let entry_points: Vec<_> = stack.iter().filter_map(|(span, query)| {
+ if query.parent.is_none() {
+ // This query is connected to the root (it has no query parent)
+ Some((*span, query.clone(), None))
+ } else {
+ let mut waiters = Vec::new();
+ // Find all the direct waiters who lead to the root
+ visit_waiters(query.clone(), |span, waiter| {
+ // Mark all the other queries in the cycle as already visited
+ let mut visited = FxHashSet::from_iter(stack.iter().map(|q| q.1.as_ptr()));
+
+ if connected_to_root(waiter.clone(), &mut visited) {
+ waiters.push((span, waiter));
+ }
+
None
+ });
+ if waiters.is_empty() {
+ None
+ } else {
+ // Deterministically pick one of the waiters to show to the user
+ let waiter = pick_query(tcx, &waiters, |s| s.clone()).clone();
+ Some((*span, query.clone(), Some(waiter)))
}
- }));
-
- if connected_to_root(query.1.clone(), &mut visited) {
- Some(query.1.clone())
- } else {
- None
}
- });
+ }).collect();
+
+ let entry_points: Vec<(Span, Lrc<QueryJob<'tcx>>, Option<(Span, Lrc<QueryJob<'tcx>>)>)>
+ = entry_points;
// Deterministically pick an entry point
- // FIXME: Sort this instead
- let mut hcx = tcx.create_stable_hashing_context();
- let entry_point = entry_points.min_by_key(|q| {
- let mut stable_hasher = StableHasher::<u64>::new();
- q.info.query.hash_stable(&mut hcx, &mut stable_hasher);
- stable_hasher.finish()
- }).unwrap().as_ptr();
+ let (_, entry_point, usage) = pick_query(tcx, &entry_points, |e| (e.0, e.1.clone()));
// Shift the stack so that our entry point is first
- let entry_point_pos = stack.iter().position(|(_, query)| query.as_ptr() == entry_point);
+ let entry_point_pos = stack.iter().position(|(_, query)| {
+ query.as_ptr() == entry_point.as_ptr()
+ });
if let Some(pos) = entry_point_pos {
- stack.rotate_right(pos);
+ stack.rotate_left(pos);
}
+ let usage = usage.as_ref().map(|(span, query)| (*span, query.info.query.clone()));
+
// Create the cycle error
let mut error = CycleError {
- usage: None,
+ usage,
cycle: stack.iter().map(|&(s, ref q)| QueryInfo {
span: s,
query: q.info.query.clone(),
From 4fac7396a2da7fdc663ff8b26cdbbfe40a0eb9bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Sun, 2 Dec 2018 21:57:47 +0100
Subject: [PATCH 2/4] Fix a race condition
---
src/librustc/ty/query/job.rs | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/librustc/ty/query/job.rs b/src/librustc/ty/query/job.rs
index d5bf3856bc432..1439e41bb31fd 100644
--- a/src/librustc/ty/query/job.rs
+++ b/src/librustc/ty/query/job.rs
@@ -103,8 +103,11 @@ impl<'tcx> QueryJob<'tcx> {
condvar: Condvar::new(),
});
self.latch.await(&waiter);
-
- match Lrc::get_mut(&mut waiter).unwrap().cycle.get_mut().take() {
+ // FIXME: Get rid of this lock. We have ownership of the QueryWaiter
+ // although another thread may still have a Lrc reference so we cannot
+ // use Lrc::get_mut
+ let mut cycle = waiter.cycle.lock();
+ match cycle.take() {
None => Ok(()),
Some(cycle) => Err(cycle)
}
From 93294ea4565d209e43484cf9bb6ddd66bb7007ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Mon, 3 Dec 2018 19:07:16 +0100
Subject: [PATCH 3/4] Fix
---
src/librustc_driver/test.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/librustc_driver/test.rs b/src/librustc_driver/test.rs
index b8cd9bda7f020..2b6bb34b146cb 100644
--- a/src/librustc_driver/test.rs
+++ b/src/librustc_driver/test.rs
@@ -98,7 +98,7 @@ fn errors(msgs: &[&str]) -> (Box<dyn Emitter + sync::Send>, usize) {
fn test_env<F>(source_string: &str, args: (Box<dyn Emitter + sync::Send>, usize), body: F)
where
- F: FnOnce(Env),
+ F: FnOnce(Env) + sync::Send,
{
syntax::with_globals(|| {
let mut options = config::Options::default();
From 813b484d1e8185b746ab67b067fe3ca331b9b07d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= <john.kare.alsaker@gmail.com>
Date: Mon, 3 Dec 2018 19:45:06 +0100
Subject: [PATCH 4/4] Fix printing of spans with no TyCtxt
---
src/librustc/ty/context.rs | 8 ++++++--
src/libsyntax_pos/lib.rs | 2 +-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
index f041192413107..bd8b3e678d851 100644
--- a/src/librustc/ty/context.rs
+++ b/src/librustc/ty/context.rs
@@ -1942,8 +1942,12 @@ pub mod tls {
/// This is a callback from libsyntax as it cannot access the implicit state
/// in librustc otherwise
fn span_debug(span: syntax_pos::Span, f: &mut fmt::Formatter<'_>) -> fmt::Result {
- with(|tcx| {
- write!(f, "{}", tcx.sess.source_map().span_to_string(span))
+ with_opt(|tcx| {
+ if let Some(tcx) = tcx {
+ write!(f, "{}", tcx.sess.source_map().span_to_string(span))
+ } else {
+ syntax_pos::default_span_debug(span, f)
+ }
})
}
diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs
index 4d42b85ea7548..4e3d1e89a7219 100644
--- a/src/libsyntax_pos/lib.rs
+++ b/src/libsyntax_pos/lib.rs
@@ -611,7 +611,7 @@ impl serialize::UseSpecializedDecodable for Span {
}
}
-fn default_span_debug(span: Span, f: &mut fmt::Formatter) -> fmt::Result {
+pub fn default_span_debug(span: Span, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_struct("Span")
.field("lo", &span.lo())
.field("hi", &span.hi())