From 3f99118871371fae708f23aaf7632d26ad9251a5 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 8 Jun 2017 13:22:57 -0400 Subject: [PATCH] kill various tasks we no longer need and remove outdated README text In the case of `TransCrateItem`, I had to tweak the tests a bit, but it's a concept that doesn't work well under new system. --- src/librustc/dep_graph/README.md | 137 +----------------- src/librustc/dep_graph/dep_node.rs | 5 - src/librustc/lint/context.rs | 3 - src/librustc_incremental/persist/preds/mod.rs | 3 +- src/librustc_trans/trans_item.rs | 18 --- src/librustc_typeck/coherence/overlap.rs | 5 - .../dep-graph-assoc-type-trans.rs | 1 - .../compile-fail/dep-graph-caller-callee.rs | 2 - src/test/compile-fail/dep-graph-trait-impl.rs | 5 - src/test/incremental/dirty_clean.rs | 4 - src/test/incremental/krate-inlined.rs | 14 +- .../incremental/remapped_paths_cc/main.rs | 4 - src/test/incremental/string_constant.rs | 3 - 13 files changed, 13 insertions(+), 191 deletions(-) diff --git a/src/librustc/dep_graph/README.md b/src/librustc/dep_graph/README.md index 72715cf6bc7..c747c443b3a 100644 --- a/src/librustc/dep_graph/README.md +++ b/src/librustc/dep_graph/README.md @@ -18,7 +18,7 @@ one of three things: 1. HIR nodes (like `Hir(DefId)`) represent the HIR input itself. 2. Data nodes (like `ItemSignature(DefId)`) represent some computed information about a particular item. -3. Procedure notes (like `CoherenceCheckImpl(DefId)`) represent some +3. Procedure nodes (like `CoherenceCheckTrait(DefId)`) represent some procedure that is executing. Usually this procedure is performing some kind of check for errors. You can think of them as computed values where the value being computed is `()` (and the @@ -57,139 +57,10 @@ recompile that item for sure. But we need the dep tracking map to tell us what *else* we have to recompile. Shared state is anything that is used to communicate results from one item to another. -### Identifying the current task +### Identifying the current task, tracking reads/writes, etc -The dep graph always tracks a current task: this is basically the -`DepNode` that the compiler is computing right now. Typically it would -be a procedure node, but it can also be a data node (as noted above, -the two are kind of equivalent). - -You set the current task by calling `dep_graph.in_task(node)`. For example: - -```rust -let _task = tcx.dep_graph.in_task(DepNode::Privacy); -``` - -Now all the code until `_task` goes out of scope will be considered -part of the "privacy task". - -The tasks are maintained in a stack, so it is perfectly fine to nest -one task within another. Because pushing a task is considered to be -computing a value, when you nest a task `N2` inside of a task `N1`, we -automatically add an edge `N2 -> N1` (since `N1` presumably needed the -result of `N2` to complete): - -```rust -let _n1 = tcx.dep_graph.in_task(DepNode::N1); -let _n2 = tcx.dep_graph.in_task(DepNode::N2); -// this will result in an edge N1 -> n2 -``` - -### Ignore tasks - -Although it is rarely needed, you can also push a special "ignore" -task: - -```rust -let _ignore = tc.dep_graph.in_ignore(); -``` - -This will cause all read/write edges to be ignored until it goes out -of scope or until something else is pushed. For example, we could -suppress the edge between nested tasks like so: - -```rust -let _n1 = tcx.dep_graph.in_task(DepNode::N1); -let _ignore = tcx.dep_graph.in_ignore(); -let _n2 = tcx.dep_graph.in_task(DepNode::N2); -// now no edge is added -``` - -### Tracking reads and writes - -We need to identify what shared state is read/written by the current -task as it executes. The most fundamental way of doing that is to invoke -the `read` and `write` methods on `DepGraph`: - -```rust -// Adds an edge from DepNode::Hir(some_def_id) to the current task -tcx.dep_graph.read(DepNode::Hir(some_def_id)) - -// Adds an edge from the current task to DepNode::ItemSignature(some_def_id) -tcx.dep_graph.write(DepNode::ItemSignature(some_def_id)) -``` - -However, you should rarely need to invoke those methods directly. -Instead, the idea is to *encapsulate* shared state into some API that -will invoke `read` and `write` automatically. The most common way to -do this is to use a `DepTrackingMap`, described in the next section, -but any sort of abstraction barrier will do. In general, the strategy -is that getting access to information implicitly adds an appropriate -`read`. So, for example, when you use the -`dep_graph::visit_all_items_in_krate` helper method, it will visit -each item `X`, start a task `Foo(X)` for that item, and automatically -add an edge `Hir(X) -> Foo(X)`. This edge is added because the code is -being given access to the HIR node for `X`, and hence it is expected -to read from it. Similarly, reading from the `tcache` map for item `X` -(which is a `DepTrackingMap`, described below) automatically invokes -`dep_graph.read(ItemSignature(X))`. - -**Note:** adding `Hir` nodes requires a bit of caution due to the -"inlining" that old trans and constant evaluation still use. See the -section on inlining below. - -To make this strategy work, a certain amount of indirection is -required. For example, modules in the HIR do not have direct pointers -to the items that they contain. Rather, they contain node-ids -- one -can then ask the HIR map for the item with a given node-id. This gives -us an opportunity to add an appropriate read edge. - -#### Explicit calls to read and write when starting a new subtask - -One time when you *may* need to call `read` and `write` directly is -when you push a new task onto the stack, either by calling `in_task` -as shown above or indirectly, such as with the `memoize` pattern -described below. In that case, any data that the task has access to -from the surrounding environment must be explicitly "read". For -example, in `librustc_typeck`, the collection code visits all items -and, among other things, starts a subtask producing its signature -(what follows is simplified pseudocode, of course): - -```rust -fn visit_item(item: &hir::Item) { - // Here, current subtask is "Collect(X)", and an edge Hir(X) -> Collect(X) - // has automatically been added by `visit_all_items_in_krate`. - let sig = signature_of_item(item); -} - -fn signature_of_item(item: &hir::Item) { - let def_id = tcx.map.local_def_id(item.id); - let task = tcx.dep_graph.in_task(DepNode::ItemSignature(def_id)); - tcx.dep_graph.read(DepNode::Hir(def_id)); // <-- the interesting line - ... -} -``` - -Here you can see that, in `signature_of_item`, we started a subtask -corresponding to producing the `ItemSignature`. This subtask will read from -`item` -- but it gained access to `item` implicitly. This means that if it just -reads from `item`, there would be missing edges in the graph: - - Hir(X) --+ // added by the explicit call to `read` - | | - | +---> ItemSignature(X) -> Collect(X) - | ^ - | | - +---------------------------------+ // added by `visit_all_items_in_krate` - -In particular, the edge from `Hir(X)` to `ItemSignature(X)` is only -present because we called `read` ourselves when entering the `ItemSignature(X)` -task. - -So, the rule of thumb: when entering a new task yourself, register -reads on any shared state that you inherit. (This actually comes up -fairly infrequently though: the main place you need caution is around -memoization.) +FIXME(#42293). This text needs to be rewritten for the new red-green +system, which doesn't fully exist yet. #### Dependency tracking map diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 3b6a7f87c13..d5b57874766 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -315,9 +315,6 @@ define_dep_nodes!( Coherence, Resolve, CoherenceCheckTrait(DefId), - CoherenceCheckImpl(DefId), - CoherenceOverlapCheck(DefId), - CoherenceOverlapCheckSpecial(DefId), Variance, PrivacyAccessLevels(CrateNum), @@ -332,8 +329,6 @@ define_dep_nodes!( RvalueCheck(DefId), Reachability, MirKeys, - LateLintCheck, - TransCrateItem(DefId), TransWriteMetadata, CrateVariances, diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index aa7428e5910..a9e0ef51102 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -25,7 +25,6 @@ //! for all lint attributes. use self::TargetLint::*; -use dep_graph::{DepNode, DepKind}; use middle::privacy::AccessLevels; use traits::Reveal; use ty::{self, TyCtxt}; @@ -1341,8 +1340,6 @@ fn check_lint_name_cmdline(sess: &Session, lint_cx: &LintStore, /// /// Consumes the `lint_store` field of the `Session`. pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - let _task = tcx.dep_graph.in_task(DepNode::new_no_params(DepKind::LateLintCheck)); - let access_levels = &tcx.privacy_access_levels(LOCAL_CRATE); let krate = tcx.hir.krate(); diff --git a/src/librustc_incremental/persist/preds/mod.rs b/src/librustc_incremental/persist/preds/mod.rs index 0a259ad2685..f7b6b7376d1 100644 --- a/src/librustc_incremental/persist/preds/mod.rs +++ b/src/librustc_incremental/persist/preds/mod.rs @@ -57,8 +57,7 @@ impl<'q> Predecessors<'q> { } // if -Z query-dep-graph is passed, save more extended data // to enable better unit testing - DepKind::TypeckTables | - DepKind::TransCrateItem => tcx.sess.opts.debugging_opts.query_dep_graph, + DepKind::TypeckTables => tcx.sess.opts.debugging_opts.query_dep_graph, _ => false, } diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 2a36ef9358e..0dc2bc85e30 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -23,7 +23,6 @@ use common; use declare; use llvm; use monomorphize::Instance; -use rustc::dep_graph::DepKind; use rustc::hir; use rustc::hir::def_id::DefId; use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; @@ -63,22 +62,9 @@ impl<'a, 'tcx> TransItem<'tcx> { self.to_raw_string(), ccx.codegen_unit().name()); - // (*) This code executes in the context of a dep-node for the - // entire CGU. In some cases, we introduce dep-nodes for - // particular items that we are translating (these nodes will - // have read edges coming into the CGU node). These smaller - // nodes are not needed for correctness -- we always - // invalidate an entire CGU at a time -- but they enable - // finer-grained testing, since you can write tests that check - // that the incoming edges to a particular fn are from a - // particular set. - match *self { TransItem::Static(node_id) => { let tcx = ccx.tcx(); - let def_id = tcx.hir.local_def_id(node_id); - let dep_node = def_id.to_dep_node(tcx, DepKind::TransCrateItem); - let _task = ccx.tcx().dep_graph.in_task(dep_node); // (*) let item = tcx.hir.expect_item(node_id); if let hir::ItemStatic(_, m, _) = item.node { match consts::trans_static(&ccx, m, item.id, &item.attrs) { @@ -100,10 +86,6 @@ impl<'a, 'tcx> TransItem<'tcx> { } } TransItem::Fn(instance) => { - let _task = ccx.tcx().dep_graph.in_task( - instance.def_id() - .to_dep_node(ccx.tcx(), DepKind::TransCrateItem)); // (*) - base::trans_instance(&ccx, instance); } } diff --git a/src/librustc_typeck/coherence/overlap.rs b/src/librustc_typeck/coherence/overlap.rs index 781e323dea3..59ebae16d08 100644 --- a/src/librustc_typeck/coherence/overlap.rs +++ b/src/librustc_typeck/coherence/overlap.rs @@ -15,7 +15,6 @@ use rustc::traits; use rustc::ty::{self, TyCtxt, TypeFoldable}; use syntax::ast; -use rustc::dep_graph::DepKind; use rustc::hir; use rustc::hir::itemlikevisit::ItemLikeVisitor; @@ -38,10 +37,6 @@ pub fn check_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, node_id: ast::NodeId) { return } - let _task = - tcx.dep_graph.in_task(trait_def_id.to_dep_node(tcx, - DepKind::CoherenceOverlapCheck)); - // Trigger building the specialization graph for the trait of this impl. // This will detect any overlap errors. tcx.specialization_graph_of(trait_def_id); diff --git a/src/test/compile-fail/dep-graph-assoc-type-trans.rs b/src/test/compile-fail/dep-graph-assoc-type-trans.rs index fe76a4d439f..007a80008a8 100644 --- a/src/test/compile-fail/dep-graph-assoc-type-trans.rs +++ b/src/test/compile-fail/dep-graph-assoc-type-trans.rs @@ -36,7 +36,6 @@ mod y { use Foo; #[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK pub fn use_char_assoc() { // Careful here: in the representation, ::T gets // normalized away, so at a certain point we had no edge to diff --git a/src/test/compile-fail/dep-graph-caller-callee.rs b/src/test/compile-fail/dep-graph-caller-callee.rs index 9cb87886809..222c1972005 100644 --- a/src/test/compile-fail/dep-graph-caller-callee.rs +++ b/src/test/compile-fail/dep-graph-caller-callee.rs @@ -28,7 +28,6 @@ mod y { // These dependencies SHOULD exist: #[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK pub fn y() { x::x(); } @@ -40,7 +39,6 @@ mod z { // These are expected to yield errors, because changes to `x` // affect the BODY of `y`, but not its signature. #[rustc_then_this_would_need(TypeckTables)] //~ ERROR no path - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR no path pub fn z() { y::y(); } diff --git a/src/test/compile-fail/dep-graph-trait-impl.rs b/src/test/compile-fail/dep-graph-trait-impl.rs index c0f9f054626..85b3e69065d 100644 --- a/src/test/compile-fail/dep-graph-trait-impl.rs +++ b/src/test/compile-fail/dep-graph-trait-impl.rs @@ -35,25 +35,21 @@ mod y { use Foo; #[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK pub fn with_char() { char::method('a'); } #[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK pub fn take_foo_with_char() { take_foo::('a'); } #[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK pub fn with_u32() { u32::method(22); } #[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK pub fn take_foo_with_u32() { take_foo::(22); } @@ -67,7 +63,6 @@ mod z { // These are expected to yield errors, because changes to `x` // affect the BODY of `y`, but not its signature. #[rustc_then_this_would_need(TypeckTables)] //~ ERROR no path - #[rustc_then_this_would_need(TransCrateItem)] //~ ERROR no path pub fn z() { y::with_char(); y::with_u32(); diff --git a/src/test/incremental/dirty_clean.rs b/src/test/incremental/dirty_clean.rs index ce9865103dc..b828cc9c70a 100644 --- a/src/test/incremental/dirty_clean.rs +++ b/src/test/incremental/dirty_clean.rs @@ -36,19 +36,15 @@ mod y { use x; #[rustc_clean(label="TypeckTables", cfg="cfail2")] - #[rustc_clean(label="TransCrateItem", cfg="cfail2")] pub fn y() { //[cfail2]~^ ERROR `TypeckTables(y::y)` not found in dep graph, but should be clean - //[cfail2]~| ERROR `TransCrateItem(y::y)` not found in dep graph, but should be clean x::x(); } } mod z { #[rustc_dirty(label="TypeckTables", cfg="cfail2")] - #[rustc_dirty(label="TransCrateItem", cfg="cfail2")] pub fn z() { //[cfail2]~^ ERROR `TypeckTables(z::z)` found in dep graph, but should be dirty - //[cfail2]~| ERROR `TransCrateItem(z::z)` found in dep graph, but should be dirty } } diff --git a/src/test/incremental/krate-inlined.rs b/src/test/incremental/krate-inlined.rs index ba32b41983f..043cb761da0 100644 --- a/src/test/incremental/krate-inlined.rs +++ b/src/test/incremental/krate-inlined.rs @@ -9,20 +9,22 @@ // except according to those terms. // Regr. test that using HIR inlined from another krate does *not* add -// a dependency from the local Krate node. +// a dependency from the local Krate node. We can't easily test that +// directly anymore, so now we test that we get reuse. -// revisions: cfail1 +// revisions: rpass1 rpass2 // compile-flags: -Z query-dep-graph #![allow(warnings)] #![feature(rustc_attrs)] +#![rustc_partition_reused(module="krate_inlined-x", cfg="rpass2")] -#![rustc_if_this_changed(Krate)] - -fn main() { } +fn main() { + #[cfg(rpass2)] + () +} mod x { - #[rustc_then_this_would_need(TransCrateItem)] //[cfail1]~ ERROR no path fn method() { // use some methods that require inlining HIR from another crate: let mut v = vec![]; diff --git a/src/test/incremental/remapped_paths_cc/main.rs b/src/test/incremental/remapped_paths_cc/main.rs index be4764c7d99..701c5fec49b 100644 --- a/src/test/incremental/remapped_paths_cc/main.rs +++ b/src/test/incremental/remapped_paths_cc/main.rs @@ -25,8 +25,6 @@ extern crate extern_crate; -#[rustc_clean(label="TransCrateItem", cfg="rpass2")] -#[rustc_clean(label="TransCrateItem", cfg="rpass3")] fn main() { some_mod::some_fn(); } @@ -34,8 +32,6 @@ fn main() { mod some_mod { use extern_crate; - #[rustc_clean(label="TransCrateItem", cfg="rpass2")] - #[rustc_dirty(label="TransCrateItem", cfg="rpass3")] pub fn some_fn() { extern_crate::inline_fn(); } diff --git a/src/test/incremental/string_constant.rs b/src/test/incremental/string_constant.rs index 669d001cc63..36a26cf1755 100644 --- a/src/test/incremental/string_constant.rs +++ b/src/test/incremental/string_constant.rs @@ -28,7 +28,6 @@ mod x { #[cfg(rpass2)] #[rustc_dirty(label="TypeckTables", cfg="rpass2")] - #[rustc_dirty(label="TransCrateItem", cfg="rpass2")] pub fn x() { println!("{}", "2"); } @@ -38,7 +37,6 @@ mod y { use x; #[rustc_clean(label="TypeckTables", cfg="rpass2")] - #[rustc_clean(label="TransCrateItem", cfg="rpass2")] pub fn y() { x::x(); } @@ -48,7 +46,6 @@ mod z { use y; #[rustc_clean(label="TypeckTables", cfg="rpass2")] - #[rustc_clean(label="TransCrateItem", cfg="rpass2")] pub fn z() { y::y(); }