fix a few errant Krate edges

src/librustc/dep_graph/README.md

@@ -341,6 +341,8 @@ path is found (as demonstrated above).
 
 ### Debugging the dependency graph
 
+#### Dumping the graph
+
 The compiler is also capable of dumping the dependency graph for your
 debugging pleasure. To do so, pass the `-Z dump-dep-graph` flag. The
 graph will be dumped to `dep_graph.{txt,dot}` in the current
@@ -392,6 +394,35 @@ This will dump out all the nodes that lead from `Hir(foo)` to
 `TypeckItemBody(bar)`, from which you can (hopefully) see the source
 of the erroneous edge.
 
+#### Tracking down incorrect edges
+
+Sometimes, after you dump the dependency graph, you will find some
+path that should not exist, but you will not be quite sure how it came
+to be. **When the compiler is built with debug assertions,** it can
+help you track that down. Simply set the `RUST_FORBID_DEP_GRAPH_EDGE`
+environment variable to a filter. Every edge created in the dep-graph
+will be tested against that filter -- if it matches, a `bug!` is
+reported, so you can easily see the backtrace (`RUST_BACKTRACE=1`).
+
+The syntax for these filters is the same as described in the previous
+section. However, note that this filter is applied to every **edge**
+and doesn't handle longer paths in the graph, unlike the previous
+section.
+
+Example:
+
+You find that there is a path from the `Hir` of `foo` to the type
+check of `bar` and you don't think there should be. You dump the
+dep-graph as described in the previous section and open `dep-graph.txt`
+to see something like:
+
+    Hir(foo) -> Collect(bar)
+    Collect(bar) -> TypeckItemBody(bar)
+
+That first edge looks suspicious to you. So you set
+`RUST_FORBID_DEP_GRAPH_EDGE` to `Hir&foo -> Collect&bar`, re-run, and
+then observe the backtrace. Voila, bug fixed!
+
 ### Inlining of HIR nodes
 
 For the time being, at least, we still sometimes "inline" HIR nodes

src/librustc/dep_graph/debug.rs

@@ -66,4 +66,11 @@ impl EdgeFilter {
             })
         }
     }
+
+    pub fn test<D: Clone + Debug>(&self,
+                                  source: &DepNode<D>,
+                                  target: &DepNode<D>)
+                                  -> bool {
+        self.source.test(source) && self.target.test(target)
+    }
 }

src/librustc/dep_graph/mod.rs

@@ -15,6 +15,7 @@ mod edges;
 mod graph;
 mod query;
 mod raii;
+mod shadow;
 mod thread;
 mod visit;
 

src/librustc/dep_graph/shadow.rs

@@ -0,0 +1,123 @@
+//! The "Shadow Graph" is maintained on the main thread and which
+//! tracks each message relating to the dep-graph and applies some
+//! sanity checks as they go by. If an error results, it means you get
+//! a nice stack-trace telling you precisely what caused the error.
+//!
+//! NOTE: This is a debugging facility which can potentially have non-trivial
+//! runtime impact. Therefore, it is largely compiled out if
+//! debug-assertions are not enabled.
+//!
+//! The basic sanity check, always enabled, is that there is always a
+//! task (or ignore) on the stack when you do read/write.
+//!
+//! Optionally, if you specify RUST_FORBID_DEP_GRAPH_EDGE, you can
+//! specify an edge filter to be applied to each edge as it is
+//! created.  See `./README.md` for details.
+
+use hir::def_id::DefId;
+use std::cell::{BorrowState, RefCell};
+use std::env;
+
+use super::DepNode;
+use super::thread::DepMessage;
+use super::debug::EdgeFilter;
+
+pub struct ShadowGraph {
+    // if you push None onto the stack, that corresponds to an Ignore
+    stack: RefCell<Vec<Option<DepNode<DefId>>>>,
+    forbidden_edge: Option<EdgeFilter>,
+}
+
+const ENABLED: bool = cfg!(debug_assertions);
+
+impl ShadowGraph {
+    pub fn new() -> Self {
+        let forbidden_edge = if !ENABLED {
+            None
+        } else {
+            match env::var("RUST_FORBID_DEP_GRAPH_EDGE") {
+                Ok(s) => {
+                    match EdgeFilter::new(&s) {
+                        Ok(f) => Some(f),
+                        Err(err) => bug!("RUST_FORBID_DEP_GRAPH_EDGE invalid: {}", err),
+                    }
+                }
+                Err(_) => None,
+            }
+        };
+
+        ShadowGraph {
+            stack: RefCell::new(vec![]),
+            forbidden_edge: forbidden_edge,
+        }
+    }
+
+    pub fn enqueue(&self, message: &DepMessage) {
+        if ENABLED {
+            match self.stack.borrow_state() {
+                BorrowState::Unused => {}
+                _ => {
+                    // When we apply edge filters, that invokes the
+                    // Debug trait on DefIds, which in turn reads from
+                    // various bits of state and creates reads! Ignore
+                    // those recursive reads.
+                    return;
+                }
+            }
+
+            let mut stack = self.stack.borrow_mut();
+            match *message {
+                DepMessage::Read(ref n) => self.check_edge(Some(Some(n)), top(&stack)),
+                DepMessage::Write(ref n) => self.check_edge(top(&stack), Some(Some(n))),
+                DepMessage::PushTask(ref n) => stack.push(Some(n.clone())),
+                DepMessage::PushIgnore => stack.push(None),
+                DepMessage::PopTask(_) |
+                DepMessage::PopIgnore => {
+                    // we could easily check that the stack is
+                    // well-formed here, but since we use closures and
+                    // RAII accessors, this bug basically never
+                    // happens, so it seems not worth the overhead
+                    stack.pop();
+                }
+                DepMessage::Query => (),
+            }
+        }
+    }
+
+    fn check_edge(&self,
+                  source: Option<Option<&DepNode<DefId>>>,
+                  target: Option<Option<&DepNode<DefId>>>) {
+        assert!(ENABLED);
+        match (source, target) {
+            // cannot happen, one side is always Some(Some(_))
+            (None, None) => unreachable!(),
+
+            // nothing on top of the stack
+            (None, Some(n)) | (Some(n), None) => bug!("read/write of {:?} but no current task", n),
+
+            // this corresponds to an Ignore being top of the stack
+            (Some(None), _) | (_, Some(None)) => (),
+
+            // a task is on top of the stack
+            (Some(Some(source)), Some(Some(target))) => {
+                if let Some(ref forbidden_edge) = self.forbidden_edge {
+                    if forbidden_edge.test(source, target) {
+                        bug!("forbidden edge {:?} -> {:?} created", source, target)
+                    }
+                }
+            }
+        }
+    }
+}
+
+// Do a little juggling: we get back a reference to an option at the
+// top of the stack, convert it to an optional reference.
+fn top<'s>(stack: &'s Vec<Option<DepNode<DefId>>>) -> Option<Option<&'s DepNode<DefId>>> {
+    stack.last()
+        .map(|n: &'s Option<DepNode<DefId>>| -> Option<&'s DepNode<DefId>> {
+            // (*)
+            // (*) type annotation just there to clarify what would
+            // otherwise be some *really* obscure code
+            n.as_ref()
+        })
+}

src/librustc/dep_graph/thread.rs

@@ -20,13 +20,13 @@
 
 use hir::def_id::DefId;
 use rustc_data_structures::veccell::VecCell;
-use std::cell::Cell;
 use std::sync::mpsc::{self, Sender, Receiver};
 use std::thread;
 
 use super::DepGraphQuery;
 use super::DepNode;
 use super::edges::DepGraphEdges;
+use super::shadow::ShadowGraph;
 
 #[derive(Debug)]
 pub enum DepMessage {
@@ -42,12 +42,16 @@ pub enum DepMessage {
 pub struct DepGraphThreadData {
     enabled: bool,
 
-    // Local counter that just tracks how many tasks are pushed onto the
-    // stack, so that we still get an error in the case where one is
-    // missing. If dep-graph construction is enabled, we'd get the same
-    // error when processing tasks later on, but that's annoying because
-    // it lacks precision about the source of the error.
-    tasks_pushed: Cell<usize>,
+    // The "shadow graph" is a debugging aid. We give it each message
+    // in real time as it arrives and it checks for various errors
+    // (for example, a read/write when there is no current task; it
+    // can also apply user-defined filters; see `shadow` module for
+    // details). This only occurs if debug-assertions are enabled.
+    //
+    // Note that in some cases the same errors will occur when the
+    // data is processed off the main thread, but that's annoying
+    // because it lacks precision about the source of the error.
+    shadow_graph: ShadowGraph,
 
     // current buffer, where we accumulate messages
     messages: VecCell<DepMessage>,
@@ -76,7 +80,7 @@ impl DepGraphThreadData {
 
         DepGraphThreadData {
             enabled: enabled,
-            tasks_pushed: Cell::new(0),
+            shadow_graph: ShadowGraph::new(),
             messages: VecCell::with_capacity(INITIAL_CAPACITY),
             swap_in: rx2,
             swap_out: tx1,
@@ -118,21 +122,7 @@ impl DepGraphThreadData {
     /// the buffer is full, this may swap.)
     #[inline]
     pub fn enqueue(&self, message: DepMessage) {
-        // Regardless of whether dep graph construction is enabled, we
-        // still want to check that we always have a valid task on the
-        // stack when a read/write/etc event occurs.
-        match message {
-            DepMessage::Read(_) | DepMessage::Write(_) =>
-                if self.tasks_pushed.get() == 0 {
-                    self.invalid_message("read/write but no current task")
-                },
-            DepMessage::PushTask(_) | DepMessage::PushIgnore =>
-                self.tasks_pushed.set(self.tasks_pushed.get() + 1),
-            DepMessage::PopTask(_) | DepMessage::PopIgnore =>
-                self.tasks_pushed.set(self.tasks_pushed.get() - 1),
-            DepMessage::Query =>
-                (),
-        }
+        self.shadow_graph.enqueue(&message);
 
         if self.enabled {
             self.enqueue_enabled(message);
@@ -147,11 +137,6 @@ impl DepGraphThreadData {
             self.swap();
         }
     }
-
-    // Outline this too.
-    fn invalid_message(&self, string: &str) {
-        bug!("{}; see src/librustc/dep_graph/README.md for more information", string)
-    }
 }
 
 /// Definition of the depgraph thread.

src/librustc/lib.rs

@@ -24,6 +24,7 @@
 #![cfg_attr(not(stage0), deny(warnings))]
 
 #![feature(associated_consts)]
+#![feature(borrow_state)]
 #![feature(box_patterns)]
 #![feature(box_syntax)]
 #![feature(collections)]