restructure CollectItem dep-node to separate fn sigs from bodies

Setup two tasks, one of which only processes the signatures, in order to
isolate the typeck entries for signatures from those for bodies.

Fixes rust-lang#36078
Fixes rust-lang#37720

Author: nikomatsakis

src/librustc/dep_graph/dep_node.rs

@@ -62,6 +62,7 @@ pub enum DepNode<D: Clone + Debug> {
     PluginRegistrar,
     StabilityIndex,
     CollectItem(D),
+    CollectItemSig(D),
     Coherence,
     EffectCheck,
     Liveness,
@@ -207,6 +208,7 @@ impl<D: Clone + Debug> DepNode<D> {
             HirBody(ref d) => op(d).map(HirBody),
             MetaData(ref d) => op(d).map(MetaData),
             CollectItem(ref d) => op(d).map(CollectItem),
+            CollectItemSig(ref d) => op(d).map(CollectItemSig),
             CoherenceCheckImpl(ref d) => op(d).map(CoherenceCheckImpl),
             CoherenceOverlapCheck(ref d) => op(d).map(CoherenceOverlapCheck),
             CoherenceOverlapCheckSpecial(ref d) => op(d).map(CoherenceOverlapCheckSpecial),

src/librustc_typeck/collect.rs

@@ -128,13 +128,62 @@ struct CollectItemTypesVisitor<'a, 'tcx: 'a> {
     ccx: &'a CrateCtxt<'a, 'tcx>
 }
 
+impl<'a, 'tcx> CollectItemTypesVisitor<'a, 'tcx> {
+    /// Collect item types is structured into two tasks. The outer
+    /// task, `CollectItem`, walks the entire content of an item-like
+    /// thing, including its body. It also spawns an inner task,
+    /// `CollectItemSig`, which walks only the signature. This inner
+    /// task is the one that writes the item-type into the various
+    /// maps.  This setup ensures that the item body is never
+    /// accessible to the task that computes its signature, so that
+    /// changes to the body don't affect the signature.
+    ///
+    /// Consider an example function `foo` that also has a closure in its body:
+    ///
+    /// ```
+    /// fn foo(<sig>) {
+    ///     ...
+    ///     let bar = || ...; // we'll label this closure as "bar" below
+    /// }
+    /// ```
+    ///
+    /// This results in a dep-graph like so. I've labeled the edges to
+    /// document where they arise.
+    ///
+    /// ```
+    /// [HirBody(foo)] -2--> [CollectItem(foo)] -4-> [ItemSignature(bar)]
+    ///                       ^           ^
+    ///                       1           3
+    /// [Hir(foo)] -----------+-6-> [CollectItemSig(foo)] -5-> [ItemSignature(foo)]
+    /// ```
+    ///
+    /// 1. This is added by the `visit_all_item_likes_in_krate`.
+    /// 2. This is added when we fetch the item body.
+    /// 3. This is added because `CollectItem` launches `CollectItemSig`.
+    ///    - it is arguably false; if we refactor the `with_task` system;
+    ///      we could get probably rid of it, but it is also harmless enough.
+    /// 4. This is added by the code in `visit_expr` when we write to `item_types`.
+    /// 5. This is added by the code in `convert_item` when we write to `item_types`;
+    ///    note that this write occurs inside the `CollectItemSig` task.
+    /// 6. Added by explicit `read` below
+    fn with_collect_item_sig<OP>(&self, id: ast::NodeId, op: OP)
+        where OP: FnOnce()
+    {
+        let def_id = self.ccx.tcx.map.local_def_id(id);
+        self.ccx.tcx.dep_graph.with_task(DepNode::CollectItemSig(def_id), || {
+            self.ccx.tcx.map.read(id);
+            op();
+        });
+    }
+}
+
 impl<'a, 'tcx> Visitor<'tcx> for CollectItemTypesVisitor<'a, 'tcx> {
     fn nested_visit_map(&mut self) -> Option<(&hir::map::Map<'tcx>, NestedVisitMode)> {
         Some((&self.ccx.tcx.map, NestedVisitMode::OnlyBodies))
     }
 
     fn visit_item(&mut self, item: &'tcx hir::Item) {
-        convert_item(self.ccx, item);
+        self.with_collect_item_sig(item.id, || convert_item(self.ccx, item));
         intravisit::walk_item(self, item);
     }
 
@@ -154,7 +203,9 @@ impl<'a, 'tcx> Visitor<'tcx> for CollectItemTypesVisitor<'a, 'tcx> {
     }
 
     fn visit_impl_item(&mut self, impl_item: &'tcx hir::ImplItem) {
-        convert_impl_item(self.ccx, impl_item);
+        self.with_collect_item_sig(impl_item.id, || {
+            convert_impl_item(self.ccx, impl_item)
+        });
         intravisit::walk_impl_item(self, impl_item);
     }
 }

src/test/incremental/change_private_impl_method_cc/struct_point.rs

@@ -23,18 +23,16 @@
 #![rustc_partition_reused(module="struct_point-fn_write_field", cfg="rpass2")]
 #![rustc_partition_reused(module="struct_point-fn_make_struct", cfg="rpass2")]
 
-// FIXME(#37720) these two should be reused, but data gets entangled across crates
-#![rustc_partition_translated(module="struct_point-fn_calls_methods_in_same_impl", cfg="rpass2")]
-#![rustc_partition_translated(module="struct_point-fn_calls_methods_in_another_impl", cfg="rpass2")]
+#![rustc_partition_reused(module="struct_point-fn_calls_methods_in_same_impl", cfg="rpass2")]
+#![rustc_partition_reused(module="struct_point-fn_calls_methods_in_another_impl", cfg="rpass2")]
 
 extern crate point;
 
 /// A fn item that calls (public) methods on `Point` from the same impl which changed
 mod fn_calls_methods_in_same_impl {
     use point::Point;
 
-    // FIXME(#37720) data gets entangled across crates
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn check() {
         let x = Point { x: 2.0, y: 2.0 };
         x.distance_from_origin();
@@ -45,8 +43,7 @@ mod fn_calls_methods_in_same_impl {
 mod fn_calls_methods_in_another_impl {
     use point::Point;
 
-    // FIXME(#37720) data gets entangled across crates
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn dirty() {
         let mut x = Point { x: 2.0, y: 2.0 };
         x.translate(3.0, 3.0);

src/test/incremental/change_pub_inherent_method_body/struct_point.rs

@@ -19,9 +19,7 @@
 
 #![rustc_partition_translated(module="struct_point-point", cfg="rpass2")]
 
-// FIXME(#35078) -- this gets recompiled because we don't separate sig from body
-#![rustc_partition_translated(module="struct_point-fn_calls_changed_method", cfg="rpass2")]
-
+#![rustc_partition_reused(module="struct_point-fn_calls_changed_method", cfg="rpass2")]
 #![rustc_partition_reused(module="struct_point-fn_calls_another_method", cfg="rpass2")]
 #![rustc_partition_reused(module="struct_point-fn_make_struct", cfg="rpass2")]
 #![rustc_partition_reused(module="struct_point-fn_read_field", cfg="rpass2")]
@@ -52,8 +50,7 @@ mod point {
 mod fn_calls_changed_method {
     use point::Point;
 
-    // FIXME(#35078) -- this gets recompiled because we don't separate sig from body
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn check() {
         let p = Point { x: 2.0, y: 2.0 };
         p.distance_from_origin();

src/test/incremental/hello_world.rs

@@ -31,8 +31,7 @@ mod x {
 mod y {
     use x;
 
-    // FIXME: This should be clean
-    #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")]
+    #[rustc_clean(label="TypeckItemBody", cfg="rpass2")]
     pub fn yyyy() {
         x::xxxx();
     }