Auto merge of #33998 - nikomatsakis:incr-comp-dep-node-trait, r=mw

Incr. comp. dep-node for traits, tests

Introduce new tests and also make dep-node for trait selection a bit more selective.

Fixes #33850

r? @michaelwoerister

Author: bors

src/librustc/dep_graph/dep_node.rs

@@ -10,7 +10,16 @@
 
 use std::fmt::Debug;
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
+macro_rules! try_opt {
+    ($e:expr) => (
+        match $e {
+            Some(r) => r,
+            None => return None,
+        }
+    )
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
 pub enum DepNode<D: Clone + Debug> {
     // The `D` type is "how definitions are identified".
     // During compilation, it is always `DefId`, but when serializing
@@ -116,7 +125,7 @@ pub enum DepNode<D: Clone + Debug> {
     // which would yield an overly conservative dep-graph.
     TraitItems(D),
     ReprHints(D),
-    TraitSelect(D),
+    TraitSelect(D, Vec<D>),
 }
 
 impl<D: Clone + Debug> DepNode<D> {
@@ -212,7 +221,11 @@ impl<D: Clone + Debug> DepNode<D> {
             TraitImpls(ref d) => op(d).map(TraitImpls),
             TraitItems(ref d) => op(d).map(TraitItems),
             ReprHints(ref d) => op(d).map(ReprHints),
-            TraitSelect(ref d) => op(d).map(TraitSelect),
+            TraitSelect(ref d, ref type_ds) => {
+                let d = try_opt!(op(d));
+                let type_ds = try_opt!(type_ds.iter().map(|d| op(d)).collect());
+                Some(TraitSelect(d, type_ds))
+            }
         }
     }
 }

src/librustc/dep_graph/query.rs

@@ -47,26 +47,26 @@ impl<D: Clone + Debug + Hash + Eq> DepGraphQuery<D> {
         self.indices.contains_key(&node)
     }
 
-    pub fn nodes(&self) -> Vec<DepNode<D>> {
+    pub fn nodes(&self) -> Vec<&DepNode<D>> {
         self.graph.all_nodes()
                   .iter()
-                  .map(|n| n.data.clone())
+                  .map(|n| &n.data)
                   .collect()
     }
 
-    pub fn edges(&self) -> Vec<(DepNode<D>,DepNode<D>)> {
+    pub fn edges(&self) -> Vec<(&DepNode<D>,&DepNode<D>)> {
         self.graph.all_edges()
                   .iter()
                   .map(|edge| (edge.source(), edge.target()))
-                  .map(|(s, t)| (self.graph.node_data(s).clone(),
-                                 self.graph.node_data(t).clone()))
+                  .map(|(s, t)| (self.graph.node_data(s),
+                                 self.graph.node_data(t)))
                   .collect()
     }
 
-    fn reachable_nodes(&self, node: DepNode<D>, direction: Direction) -> Vec<DepNode<D>> {
-        if let Some(&index) = self.indices.get(&node) {
+    fn reachable_nodes(&self, node: &DepNode<D>, direction: Direction) -> Vec<&DepNode<D>> {
+        if let Some(&index) = self.indices.get(node) {
             self.graph.depth_traverse(index, direction)
-                      .map(|s| self.graph.node_data(s).clone())
+                      .map(|s| self.graph.node_data(s))
                       .collect()
         } else {
             vec![]
@@ -75,20 +75,20 @@ impl<D: Clone + Debug + Hash + Eq> DepGraphQuery<D> {
 
     /// All nodes reachable from `node`. In other words, things that
     /// will have to be recomputed if `node` changes.
-    pub fn transitive_successors(&self, node: DepNode<D>) -> Vec<DepNode<D>> {
+    pub fn transitive_successors(&self, node: &DepNode<D>) -> Vec<&DepNode<D>> {
         self.reachable_nodes(node, OUTGOING)
     }
 
     /// All nodes that can reach `node`.
-    pub fn transitive_predecessors(&self, node: DepNode<D>) -> Vec<DepNode<D>> {
+    pub fn transitive_predecessors(&self, node: &DepNode<D>) -> Vec<&DepNode<D>> {
         self.reachable_nodes(node, INCOMING)
     }
 
     /// Just the outgoing edges from `node`.
-    pub fn immediate_successors(&self, node: DepNode<D>) -> Vec<DepNode<D>> {
+    pub fn immediate_successors(&self, node: &DepNode<D>) -> Vec<&DepNode<D>> {
         if let Some(&index) = self.indices.get(&node) {
             self.graph.successor_nodes(index)
-                      .map(|s| self.graph.node_data(s).clone())
+                      .map(|s| self.graph.node_data(s))
                       .collect()
         } else {
             vec![]

src/librustc/dep_graph/raii.rs

@@ -14,20 +14,20 @@ use super::thread::{DepGraphThreadData, DepMessage};
 
 pub struct DepTask<'graph> {
     data: &'graph DepGraphThreadData,
-    key: DepNode<DefId>,
+    key: Option<DepNode<DefId>>,
 }
 
 impl<'graph> DepTask<'graph> {
     pub fn new(data: &'graph DepGraphThreadData, key: DepNode<DefId>)
                -> DepTask<'graph> {
-        data.enqueue(DepMessage::PushTask(key));
-        DepTask { data: data, key: key }
+        data.enqueue(DepMessage::PushTask(key.clone()));
+        DepTask { data: data, key: Some(key) }
     }
 }
 
 impl<'graph> Drop for DepTask<'graph> {
     fn drop(&mut self) {
-        self.data.enqueue(DepMessage::PopTask(self.key));
+        self.data.enqueue(DepMessage::PopTask(self.key.take().unwrap()));
     }
 }
 

src/librustc/dep_graph/visit.rs

@@ -39,7 +39,7 @@ pub fn visit_all_items_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         fn visit_item(&mut self, i: &'tcx hir::Item) {
             let item_def_id = self.tcx.map.local_def_id(i.id);
             let task_id = (self.dep_node_fn)(item_def_id);
-            let _task = self.tcx.dep_graph.in_task(task_id);
+            let _task = self.tcx.dep_graph.in_task(task_id.clone());
             debug!("Started task {:?}", task_id);
             self.tcx.dep_graph.read(DepNode::Hir(item_def_id));
             self.visitor.visit_item(i);

src/librustc/ty/mod.rs

@@ -946,7 +946,28 @@ impl<'tcx> TraitPredicate<'tcx> {
 
     /// Creates the dep-node for selecting/evaluating this trait reference.
     fn dep_node(&self) -> DepNode<DefId> {
-        DepNode::TraitSelect(self.def_id())
+        // Ideally, the dep-node would just have all the input types
+        // in it.  But they are limited to including def-ids. So as an
+        // approximation we include the def-ids for all nominal types
+        // found somewhere. This means that we will e.g. conflate the
+        // dep-nodes for `u32: SomeTrait` and `u64: SomeTrait`, but we
+        // would have distinct dep-nodes for `Vec<u32>: SomeTrait`,
+        // `Rc<u32>: SomeTrait`, and `(Vec<u32>, Rc<u32>): SomeTrait`.
+        // Note that it's always sound to conflate dep-nodes, it just
+        // leads to more recompilation.
+        let def_ids: Vec<_> =
+            self.input_types()
+                .iter()
+                .flat_map(|t| t.walk())
+                .filter_map(|t| match t.sty {
+                    ty::TyStruct(adt_def, _) |
+                    ty::TyEnum(adt_def, _) =>
+                        Some(adt_def.did),
+                    _ =>
+                        None
+                })
+                .collect();
+        DepNode::TraitSelect(self.def_id(), def_ids)
     }
 
     pub fn input_types(&self) -> &[Ty<'tcx>] {
@@ -1768,9 +1789,8 @@ impl<'a, 'tcx> AdtDefData<'tcx, 'tcx> {
                                         stack: &mut Vec<AdtDefMaster<'tcx>>)
     {
 
-        let dep_node = DepNode::SizedConstraint(self.did);
-
-        if self.sized_constraint.get(dep_node).is_some() {
+        let dep_node = || DepNode::SizedConstraint(self.did);
+        if self.sized_constraint.get(dep_node()).is_some() {
             return;
         }
 
@@ -1780,7 +1800,7 @@ impl<'a, 'tcx> AdtDefData<'tcx, 'tcx> {
             //
             // Consider the type as Sized in the meanwhile to avoid
             // further errors.
-            self.sized_constraint.fulfill(dep_node, tcx.types.err);
+            self.sized_constraint.fulfill(dep_node(), tcx.types.err);
             return;
         }
 
@@ -1803,14 +1823,14 @@ impl<'a, 'tcx> AdtDefData<'tcx, 'tcx> {
             _ => tcx.mk_tup(tys)
         };
 
-        match self.sized_constraint.get(dep_node) {
+        match self.sized_constraint.get(dep_node()) {
             Some(old_ty) => {
                 debug!("calculate_sized_constraint: {:?} recurred", self);
                 assert_eq!(old_ty, tcx.types.err)
             }
             None => {
                 debug!("calculate_sized_constraint: {:?} => {:?}", self, ty);
-                self.sized_constraint.fulfill(dep_node, ty)
+                self.sized_constraint.fulfill(dep_node(), ty)
             }
         }
     }

src/librustc_incremental/assert_dep_graph.rs

@@ -195,7 +195,7 @@ fn check_paths<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             }
         };
 
-        for &(_, source_def_id, source_dep_node) in sources {
+        for &(_, source_def_id, ref source_dep_node) in sources {
             let dependents = query.transitive_successors(source_dep_node);
             for &(target_span, ref target_pass, _, ref target_dep_node) in targets {
                 if !dependents.contains(&target_dep_node) {
@@ -239,7 +239,7 @@ fn dump_graph(tcx: TyCtxt) {
     { // dump a .txt file with just the edges:
         let txt_path = format!("{}.txt", path);
         let mut file = File::create(&txt_path).unwrap();
-        for &(source, target) in &edges {
+        for &(ref source, ref target) in &edges {
             write!(file, "{:?} -> {:?}\n", source, target).unwrap();
         }
     }
@@ -252,51 +252,51 @@ fn dump_graph(tcx: TyCtxt) {
     }
 }
 
-pub struct GraphvizDepGraph(FnvHashSet<DepNode<DefId>>,
-                            Vec<(DepNode<DefId>, DepNode<DefId>)>);
+pub struct GraphvizDepGraph<'q>(FnvHashSet<&'q DepNode<DefId>>,
+                                Vec<(&'q DepNode<DefId>, &'q DepNode<DefId>)>);
 
-impl<'a, 'tcx> dot::GraphWalk<'a> for GraphvizDepGraph {
-    type Node = DepNode<DefId>;
-    type Edge = (DepNode<DefId>, DepNode<DefId>);
-    fn nodes(&self) -> dot::Nodes<DepNode<DefId>> {
+impl<'a, 'tcx, 'q> dot::GraphWalk<'a> for GraphvizDepGraph<'q> {
+    type Node = &'q DepNode<DefId>;
+    type Edge = (&'q DepNode<DefId>, &'q DepNode<DefId>);
+    fn nodes(&self) -> dot::Nodes<&'q DepNode<DefId>> {
         let nodes: Vec<_> = self.0.iter().cloned().collect();
         nodes.into_cow()
     }
-    fn edges(&self) -> dot::Edges<(DepNode<DefId>, DepNode<DefId>)> {
+    fn edges(&self) -> dot::Edges<(&'q DepNode<DefId>, &'q DepNode<DefId>)> {
         self.1[..].into_cow()
     }
-    fn source(&self, edge: &(DepNode<DefId>, DepNode<DefId>)) -> DepNode<DefId> {
+    fn source(&self, edge: &(&'q DepNode<DefId>, &'q DepNode<DefId>)) -> &'q DepNode<DefId> {
         edge.0
     }
-    fn target(&self, edge: &(DepNode<DefId>, DepNode<DefId>)) -> DepNode<DefId> {
+    fn target(&self, edge: &(&'q DepNode<DefId>, &'q DepNode<DefId>)) -> &'q DepNode<DefId> {
         edge.1
     }
 }
 
-impl<'a, 'tcx> dot::Labeller<'a> for GraphvizDepGraph {
-    type Node = DepNode<DefId>;
-    type Edge = (DepNode<DefId>, DepNode<DefId>);
+impl<'a, 'tcx, 'q> dot::Labeller<'a> for GraphvizDepGraph<'q> {
+    type Node = &'q DepNode<DefId>;
+    type Edge = (&'q DepNode<DefId>, &'q DepNode<DefId>);
     fn graph_id(&self) -> dot::Id {
         dot::Id::new("DependencyGraph").unwrap()
     }
-    fn node_id(&self, n: &DepNode<DefId>) -> dot::Id {
+    fn node_id(&self, n: &&'q DepNode<DefId>) -> dot::Id {
         let s: String =
             format!("{:?}", n).chars()
                               .map(|c| if c == '_' || c.is_alphanumeric() { c } else { '_' })
                               .collect();
         debug!("n={:?} s={:?}", n, s);
         dot::Id::new(s).unwrap()
     }
-    fn node_label(&self, n: &DepNode<DefId>) -> dot::LabelText {
+    fn node_label(&self, n: &&'q DepNode<DefId>) -> dot::LabelText {
         dot::LabelText::label(format!("{:?}", n))
     }
 }
 
 // Given an optional filter like `"x,y,z"`, returns either `None` (no
 // filter) or the set of nodes whose labels contain all of those
 // substrings.
-fn node_set(query: &DepGraphQuery<DefId>, filter: &DepNodeFilter)
-            -> Option<FnvHashSet<DepNode<DefId>>>
+fn node_set<'q>(query: &'q DepGraphQuery<DefId>, filter: &DepNodeFilter)
+                -> Option<FnvHashSet<&'q DepNode<DefId>>>
 {
     debug!("node_set(filter={:?})", filter);
 
@@ -307,10 +307,10 @@ fn node_set(query: &DepGraphQuery<DefId>, filter: &DepNodeFilter)
     Some(query.nodes().into_iter().filter(|n| filter.test(n)).collect())
 }
 
-fn filter_nodes(query: &DepGraphQuery<DefId>,
-                sources: &Option<FnvHashSet<DepNode<DefId>>>,
-                targets: &Option<FnvHashSet<DepNode<DefId>>>)
-                -> FnvHashSet<DepNode<DefId>>
+fn filter_nodes<'q>(query: &'q DepGraphQuery<DefId>,
+                    sources: &Option<FnvHashSet<&'q DepNode<DefId>>>,
+                    targets: &Option<FnvHashSet<&'q DepNode<DefId>>>)
+                    -> FnvHashSet<&'q DepNode<DefId>>
 {
     if let &Some(ref sources) = sources {
         if let &Some(ref targets) = targets {
@@ -325,21 +325,21 @@ fn filter_nodes(query: &DepGraphQuery<DefId>,
     }
 }
 
-fn walk_nodes(query: &DepGraphQuery<DefId>,
-              starts: &FnvHashSet<DepNode<DefId>>,
-              direction: Direction)
-              -> FnvHashSet<DepNode<DefId>>
+fn walk_nodes<'q>(query: &'q DepGraphQuery<DefId>,
+                  starts: &FnvHashSet<&'q DepNode<DefId>>,
+                  direction: Direction)
+                  -> FnvHashSet<&'q DepNode<DefId>>
 {
     let mut set = FnvHashSet();
-    for start in starts {
+    for &start in starts {
         debug!("walk_nodes: start={:?} outgoing?={:?}", start, direction == OUTGOING);
-        if set.insert(*start) {
+        if set.insert(start) {
             let mut stack = vec![query.indices[start]];
             while let Some(index) = stack.pop() {
                 for (_, edge) in query.graph.adjacent_edges(index, direction) {
                     let neighbor_index = edge.source_or_target(direction);
                     let neighbor = query.graph.node_data(neighbor_index);
-                    if set.insert(*neighbor) {
+                    if set.insert(neighbor) {
                         stack.push(neighbor_index);
                     }
                 }
@@ -349,10 +349,10 @@ fn walk_nodes(query: &DepGraphQuery<DefId>,
     set
 }
 
-fn walk_between(query: &DepGraphQuery<DefId>,
-                sources: &FnvHashSet<DepNode<DefId>>,
-                targets: &FnvHashSet<DepNode<DefId>>)
-                -> FnvHashSet<DepNode<DefId>>
+fn walk_between<'q>(query: &'q DepGraphQuery<DefId>,
+                    sources: &FnvHashSet<&'q DepNode<DefId>>,
+                    targets: &FnvHashSet<&'q DepNode<DefId>>)
+                    -> FnvHashSet<&'q DepNode<DefId>>
 {
     // This is a bit tricky. We want to include a node only if it is:
     // (a) reachable from a source and (b) will reach a target. And we
@@ -365,16 +365,16 @@ fn walk_between(query: &DepGraphQuery<DefId>,
     let mut node_states = vec![State::Undecided; query.graph.len_nodes()];
 
     for &target in targets {
-        node_states[query.indices[&target].0] = State::Included;
+        node_states[query.indices[target].0] = State::Included;
     }
 
-    for source in sources.iter().map(|n| query.indices[n]) {
+    for source in sources.iter().map(|&n| query.indices[n]) {
         recurse(query, &mut node_states, source);
     }
 
     return query.nodes()
                 .into_iter()
-                .filter(|n| {
+                .filter(|&n| {
                     let index = query.indices[n];
                     node_states[index.0] == State::Included
                 })
@@ -417,12 +417,12 @@ fn walk_between(query: &DepGraphQuery<DefId>,
     }
 }
 
-fn filter_edges(query: &DepGraphQuery<DefId>,
-                nodes: &FnvHashSet<DepNode<DefId>>)
-                -> Vec<(DepNode<DefId>, DepNode<DefId>)>
+fn filter_edges<'q>(query: &'q DepGraphQuery<DefId>,
+                    nodes: &FnvHashSet<&'q DepNode<DefId>>)
+                    -> Vec<(&'q DepNode<DefId>, &'q DepNode<DefId>)>
 {
     query.edges()
          .into_iter()
-         .filter(|&(source, target)| nodes.contains(&source) && nodes.contains(&target))
+         .filter(|&(source, target)| nodes.contains(source) && nodes.contains(target))
          .collect()
 }