Skip to content

Commit 9b5b514

Browse files
committed
Auto merge of #42576 - nikomatsakis:incr-comp-less-tasks, r=michaelwoerister
prune some tasks and depnode variants Pick some low-hanging fruit towards the goal of removing the older tasks. r? @michaelwoerister
2 parents 9adf969 + 36973f7 commit 9b5b514

File tree

16 files changed

+61
-217
lines changed

16 files changed

+61
-217
lines changed

src/librustc/dep_graph/README.md

+4-133
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ one of three things:
1818
1. HIR nodes (like `Hir(DefId)`) represent the HIR input itself.
1919
2. Data nodes (like `ItemSignature(DefId)`) represent some computed
2020
information about a particular item.
21-
3. Procedure notes (like `CoherenceCheckImpl(DefId)`) represent some
21+
3. Procedure nodes (like `CoherenceCheckTrait(DefId)`) represent some
2222
procedure that is executing. Usually this procedure is
2323
performing some kind of check for errors. You can think of them as
2424
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
5757
us what *else* we have to recompile. Shared state is anything that is
5858
used to communicate results from one item to another.
5959

60-
### Identifying the current task
60+
### Identifying the current task, tracking reads/writes, etc
6161

62-
The dep graph always tracks a current task: this is basically the
63-
`DepNode` that the compiler is computing right now. Typically it would
64-
be a procedure node, but it can also be a data node (as noted above,
65-
the two are kind of equivalent).
66-
67-
You set the current task by calling `dep_graph.in_task(node)`. For example:
68-
69-
```rust
70-
let _task = tcx.dep_graph.in_task(DepNode::Privacy);
71-
```
72-
73-
Now all the code until `_task` goes out of scope will be considered
74-
part of the "privacy task".
75-
76-
The tasks are maintained in a stack, so it is perfectly fine to nest
77-
one task within another. Because pushing a task is considered to be
78-
computing a value, when you nest a task `N2` inside of a task `N1`, we
79-
automatically add an edge `N2 -> N1` (since `N1` presumably needed the
80-
result of `N2` to complete):
81-
82-
```rust
83-
let _n1 = tcx.dep_graph.in_task(DepNode::N1);
84-
let _n2 = tcx.dep_graph.in_task(DepNode::N2);
85-
// this will result in an edge N1 -> n2
86-
```
87-
88-
### Ignore tasks
89-
90-
Although it is rarely needed, you can also push a special "ignore"
91-
task:
92-
93-
```rust
94-
let _ignore = tc.dep_graph.in_ignore();
95-
```
96-
97-
This will cause all read/write edges to be ignored until it goes out
98-
of scope or until something else is pushed. For example, we could
99-
suppress the edge between nested tasks like so:
100-
101-
```rust
102-
let _n1 = tcx.dep_graph.in_task(DepNode::N1);
103-
let _ignore = tcx.dep_graph.in_ignore();
104-
let _n2 = tcx.dep_graph.in_task(DepNode::N2);
105-
// now no edge is added
106-
```
107-
108-
### Tracking reads and writes
109-
110-
We need to identify what shared state is read/written by the current
111-
task as it executes. The most fundamental way of doing that is to invoke
112-
the `read` and `write` methods on `DepGraph`:
113-
114-
```rust
115-
// Adds an edge from DepNode::Hir(some_def_id) to the current task
116-
tcx.dep_graph.read(DepNode::Hir(some_def_id))
117-
118-
// Adds an edge from the current task to DepNode::ItemSignature(some_def_id)
119-
tcx.dep_graph.write(DepNode::ItemSignature(some_def_id))
120-
```
121-
122-
However, you should rarely need to invoke those methods directly.
123-
Instead, the idea is to *encapsulate* shared state into some API that
124-
will invoke `read` and `write` automatically. The most common way to
125-
do this is to use a `DepTrackingMap`, described in the next section,
126-
but any sort of abstraction barrier will do. In general, the strategy
127-
is that getting access to information implicitly adds an appropriate
128-
`read`. So, for example, when you use the
129-
`dep_graph::visit_all_items_in_krate` helper method, it will visit
130-
each item `X`, start a task `Foo(X)` for that item, and automatically
131-
add an edge `Hir(X) -> Foo(X)`. This edge is added because the code is
132-
being given access to the HIR node for `X`, and hence it is expected
133-
to read from it. Similarly, reading from the `tcache` map for item `X`
134-
(which is a `DepTrackingMap`, described below) automatically invokes
135-
`dep_graph.read(ItemSignature(X))`.
136-
137-
**Note:** adding `Hir` nodes requires a bit of caution due to the
138-
"inlining" that old trans and constant evaluation still use. See the
139-
section on inlining below.
140-
141-
To make this strategy work, a certain amount of indirection is
142-
required. For example, modules in the HIR do not have direct pointers
143-
to the items that they contain. Rather, they contain node-ids -- one
144-
can then ask the HIR map for the item with a given node-id. This gives
145-
us an opportunity to add an appropriate read edge.
146-
147-
#### Explicit calls to read and write when starting a new subtask
148-
149-
One time when you *may* need to call `read` and `write` directly is
150-
when you push a new task onto the stack, either by calling `in_task`
151-
as shown above or indirectly, such as with the `memoize` pattern
152-
described below. In that case, any data that the task has access to
153-
from the surrounding environment must be explicitly "read". For
154-
example, in `librustc_typeck`, the collection code visits all items
155-
and, among other things, starts a subtask producing its signature
156-
(what follows is simplified pseudocode, of course):
157-
158-
```rust
159-
fn visit_item(item: &hir::Item) {
160-
// Here, current subtask is "Collect(X)", and an edge Hir(X) -> Collect(X)
161-
// has automatically been added by `visit_all_items_in_krate`.
162-
let sig = signature_of_item(item);
163-
}
164-
165-
fn signature_of_item(item: &hir::Item) {
166-
let def_id = tcx.map.local_def_id(item.id);
167-
let task = tcx.dep_graph.in_task(DepNode::ItemSignature(def_id));
168-
tcx.dep_graph.read(DepNode::Hir(def_id)); // <-- the interesting line
169-
...
170-
}
171-
```
172-
173-
Here you can see that, in `signature_of_item`, we started a subtask
174-
corresponding to producing the `ItemSignature`. This subtask will read from
175-
`item` -- but it gained access to `item` implicitly. This means that if it just
176-
reads from `item`, there would be missing edges in the graph:
177-
178-
Hir(X) --+ // added by the explicit call to `read`
179-
| |
180-
| +---> ItemSignature(X) -> Collect(X)
181-
| ^
182-
| |
183-
+---------------------------------+ // added by `visit_all_items_in_krate`
184-
185-
In particular, the edge from `Hir(X)` to `ItemSignature(X)` is only
186-
present because we called `read` ourselves when entering the `ItemSignature(X)`
187-
task.
188-
189-
So, the rule of thumb: when entering a new task yourself, register
190-
reads on any shared state that you inherit. (This actually comes up
191-
fairly infrequently though: the main place you need caution is around
192-
memoization.)
62+
FIXME(#42293). This text needs to be rewritten for the new red-green
63+
system, which doesn't fully exist yet.
19364

19465
#### Dependency tracking map
19566

src/librustc/dep_graph/dep_node.rs

-8
Original file line numberDiff line numberDiff line change
@@ -315,15 +315,10 @@ define_dep_nodes!(
315315
Coherence,
316316
Resolve,
317317
CoherenceCheckTrait(DefId),
318-
CoherenceCheckImpl(DefId),
319-
CoherenceOverlapCheck(DefId),
320-
CoherenceOverlapCheckSpecial(DefId),
321-
Variance,
322318
PrivacyAccessLevels(CrateNum),
323319

324320
// Represents the MIR for a fn; also used as the task node for
325321
// things read/modify that MIR.
326-
MirKrate,
327322
Mir(DefId),
328323
MirShim(DefIdList),
329324

@@ -332,8 +327,6 @@ define_dep_nodes!(
332327
RvalueCheck(DefId),
333328
Reachability,
334329
MirKeys,
335-
LateLintCheck,
336-
TransCrateItem(DefId),
337330
TransWriteMetadata,
338331
CrateVariances,
339332

@@ -355,7 +348,6 @@ define_dep_nodes!(
355348
InherentImpls(DefId),
356349
TypeckBodiesKrate,
357350
TypeckTables(DefId),
358-
UsedTraitImports(DefId),
359351
ConstEval(DefId),
360352
SymbolName(DefId),
361353
SpecializationGraph(DefId),

src/librustc/lint/context.rs

-3
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
//! for all lint attributes.
2626
use self::TargetLint::*;
2727

28-
use dep_graph::{DepNode, DepKind};
2928
use middle::privacy::AccessLevels;
3029
use traits::Reveal;
3130
use ty::{self, TyCtxt};
@@ -1341,8 +1340,6 @@ fn check_lint_name_cmdline(sess: &Session, lint_cx: &LintStore,
13411340
///
13421341
/// Consumes the `lint_store` field of the `Session`.
13431342
pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
1344-
let _task = tcx.dep_graph.in_task(DepNode::new_no_params(DepKind::LateLintCheck));
1345-
13461343
let access_levels = &tcx.privacy_access_levels(LOCAL_CRATE);
13471344

13481345
let krate = tcx.hir.krate();

src/librustc/ty/mod.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -944,7 +944,9 @@ impl<'tcx> TraitPredicate<'tcx> {
944944
self.input_types()
945945
.flat_map(|t| t.walk())
946946
.filter_map(|t| match t.sty {
947-
ty::TyAdt(adt_def, _) => Some(adt_def.did),
947+
ty::TyAdt(adt_def, ..) => Some(adt_def.did),
948+
ty::TyClosure(def_id, ..) => Some(def_id),
949+
ty::TyFnDef(def_id, ..) => Some(def_id),
948950
_ => None
949951
})
950952
.next()

src/librustc_incremental/persist/preds/mod.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,7 @@ impl<'q> Predecessors<'q> {
5757
}
5858
// if -Z query-dep-graph is passed, save more extended data
5959
// to enable better unit testing
60-
DepKind::TypeckTables |
61-
DepKind::TransCrateItem => tcx.sess.opts.debugging_opts.query_dep_graph,
60+
DepKind::TypeckTables => tcx.sess.opts.debugging_opts.query_dep_graph,
6261

6362
_ => false,
6463
}

src/librustc_trans/trans_item.rs

-18
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ use common;
2323
use declare;
2424
use llvm;
2525
use monomorphize::Instance;
26-
use rustc::dep_graph::DepKind;
2726
use rustc::hir;
2827
use rustc::hir::def_id::DefId;
2928
use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
@@ -63,22 +62,9 @@ impl<'a, 'tcx> TransItem<'tcx> {
6362
self.to_raw_string(),
6463
ccx.codegen_unit().name());
6564

66-
// (*) This code executes in the context of a dep-node for the
67-
// entire CGU. In some cases, we introduce dep-nodes for
68-
// particular items that we are translating (these nodes will
69-
// have read edges coming into the CGU node). These smaller
70-
// nodes are not needed for correctness -- we always
71-
// invalidate an entire CGU at a time -- but they enable
72-
// finer-grained testing, since you can write tests that check
73-
// that the incoming edges to a particular fn are from a
74-
// particular set.
75-
7665
match *self {
7766
TransItem::Static(node_id) => {
7867
let tcx = ccx.tcx();
79-
let def_id = tcx.hir.local_def_id(node_id);
80-
let dep_node = def_id.to_dep_node(tcx, DepKind::TransCrateItem);
81-
let _task = ccx.tcx().dep_graph.in_task(dep_node); // (*)
8268
let item = tcx.hir.expect_item(node_id);
8369
if let hir::ItemStatic(_, m, _) = item.node {
8470
match consts::trans_static(&ccx, m, item.id, &item.attrs) {
@@ -100,10 +86,6 @@ impl<'a, 'tcx> TransItem<'tcx> {
10086
}
10187
}
10288
TransItem::Fn(instance) => {
103-
let _task = ccx.tcx().dep_graph.in_task(
104-
instance.def_id()
105-
.to_dep_node(ccx.tcx(), DepKind::TransCrateItem)); // (*)
106-
10789
base::trans_instance(&ccx, instance);
10890
}
10991
}

src/librustc_typeck/coherence/overlap.rs

-5
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use rustc::traits;
1616
use rustc::ty::{self, TyCtxt, TypeFoldable};
1717
use syntax::ast;
18-
use rustc::dep_graph::DepKind;
1918
use rustc::hir;
2019
use rustc::hir::itemlikevisit::ItemLikeVisitor;
2120

@@ -38,10 +37,6 @@ pub fn check_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, node_id: ast::NodeId) {
3837
return
3938
}
4039

41-
let _task =
42-
tcx.dep_graph.in_task(trait_def_id.to_dep_node(tcx,
43-
DepKind::CoherenceOverlapCheck));
44-
4540
// Trigger building the specialization graph for the trait of this impl.
4641
// This will detect any overlap errors.
4742
tcx.specialization_graph_of(trait_def_id);

src/test/compile-fail/dep-graph-assoc-type-trans.rs

-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ mod y {
3636
use Foo;
3737

3838
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK
39-
#[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK
4039
pub fn use_char_assoc() {
4140
// Careful here: in the representation, <char as Foo>::T gets
4241
// normalized away, so at a certain point we had no edge to

src/test/compile-fail/dep-graph-caller-callee.rs

-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ mod y {
2828

2929
// These dependencies SHOULD exist:
3030
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK
31-
#[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK
3231
pub fn y() {
3332
x::x();
3433
}
@@ -40,7 +39,6 @@ mod z {
4039
// These are expected to yield errors, because changes to `x`
4140
// affect the BODY of `y`, but not its signature.
4241
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR no path
43-
#[rustc_then_this_would_need(TransCrateItem)] //~ ERROR no path
4442
pub fn z() {
4543
y::y();
4644
}

src/test/compile-fail/dep-graph-trait-impl.rs

-5
Original file line numberDiff line numberDiff line change
@@ -35,25 +35,21 @@ mod y {
3535
use Foo;
3636

3737
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK
38-
#[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK
3938
pub fn with_char() {
4039
char::method('a');
4140
}
4241

4342
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK
44-
#[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK
4543
pub fn take_foo_with_char() {
4644
take_foo::<char>('a');
4745
}
4846

4947
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK
50-
#[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK
5148
pub fn with_u32() {
5249
u32::method(22);
5350
}
5451

5552
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK
56-
#[rustc_then_this_would_need(TransCrateItem)] //~ ERROR OK
5753
pub fn take_foo_with_u32() {
5854
take_foo::<u32>(22);
5955
}
@@ -67,7 +63,6 @@ mod z {
6763
// These are expected to yield errors, because changes to `x`
6864
// affect the BODY of `y`, but not its signature.
6965
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR no path
70-
#[rustc_then_this_would_need(TransCrateItem)] //~ ERROR no path
7166
pub fn z() {
7267
y::with_char();
7368
y::with_u32();

src/test/compile-fail/dep-graph-unrelated.rs

-22
This file was deleted.

src/test/incremental/dirty_clean.rs

-4
Original file line numberDiff line numberDiff line change
@@ -36,19 +36,15 @@ mod y {
3636
use x;
3737

3838
#[rustc_clean(label="TypeckTables", cfg="cfail2")]
39-
#[rustc_clean(label="TransCrateItem", cfg="cfail2")]
4039
pub fn y() {
4140
//[cfail2]~^ ERROR `TypeckTables(y::y)` not found in dep graph, but should be clean
42-
//[cfail2]~| ERROR `TransCrateItem(y::y)` not found in dep graph, but should be clean
4341
x::x();
4442
}
4543
}
4644

4745
mod z {
4846
#[rustc_dirty(label="TypeckTables", cfg="cfail2")]
49-
#[rustc_dirty(label="TransCrateItem", cfg="cfail2")]
5047
pub fn z() {
5148
//[cfail2]~^ ERROR `TypeckTables(z::z)` found in dep graph, but should be dirty
52-
//[cfail2]~| ERROR `TransCrateItem(z::z)` found in dep graph, but should be dirty
5349
}
5450
}

0 commit comments

Comments
 (0)