From 688946d671fa4b3c0fae5c676574fba1a0499108 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 21 Nov 2016 17:12:35 -0500 Subject: [PATCH] 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 #36078 Fixes #37720 --- src/librustc/dep_graph/dep_node.rs | 2 + src/librustc_typeck/collect.rs | 55 ++++++++++++++++++- .../struct_point.rs | 11 ++-- .../struct_point.rs | 7 +-- src/test/incremental/hello_world.rs | 3 +- 5 files changed, 62 insertions(+), 16 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 0fc8bf9e17d..d19bef6babe 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -62,6 +62,7 @@ pub enum DepNode { PluginRegistrar, StabilityIndex, CollectItem(D), + CollectItemSig(D), Coherence, EffectCheck, Liveness, @@ -206,6 +207,7 @@ impl DepNode { 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), diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index bdf0f988117..09e54cb9c53 100644 --- a/src/librustc_typeck/collect.rs +++ b/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() { + /// ... + /// 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(&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); } @@ -156,7 +205,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); } } diff --git a/src/test/incremental/change_private_impl_method_cc/struct_point.rs b/src/test/incremental/change_private_impl_method_cc/struct_point.rs index bb7f7025c59..4d9ca77969b 100644 --- a/src/test/incremental/change_private_impl_method_cc/struct_point.rs +++ b/src/test/incremental/change_private_impl_method_cc/struct_point.rs @@ -23,9 +23,8 @@ #![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; @@ -33,8 +32,7 @@ extern crate point; 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); diff --git a/src/test/incremental/change_pub_inherent_method_body/struct_point.rs b/src/test/incremental/change_pub_inherent_method_body/struct_point.rs index 665eafb4f4e..e0047e5ec64 100644 --- a/src/test/incremental/change_pub_inherent_method_body/struct_point.rs +++ b/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(); diff --git a/src/test/incremental/hello_world.rs b/src/test/incremental/hello_world.rs index 14e4cfcd988..b7f90c09b56 100644 --- a/src/test/incremental/hello_world.rs +++ b/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(); }