Skip to content

Commit

Permalink
kill various tasks we no longer need and remove outdated README text
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nikomatsakis committed Jun 12, 2017
1 parent cfb5deb commit 3f99118
Show file tree
Hide file tree
Showing 13 changed files with 13 additions and 191 deletions.
137 changes: 4 additions & 133 deletions src/librustc/dep_graph/README.md
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
5 changes: 0 additions & 5 deletions src/librustc/dep_graph/dep_node.rs
Expand Up @@ -315,9 +315,6 @@ define_dep_nodes!(
Coherence,
Resolve,
CoherenceCheckTrait(DefId),
CoherenceCheckImpl(DefId),
CoherenceOverlapCheck(DefId),
CoherenceOverlapCheckSpecial(DefId),
Variance,
PrivacyAccessLevels(CrateNum),

Expand All @@ -332,8 +329,6 @@ define_dep_nodes!(
RvalueCheck(DefId),
Reachability,
MirKeys,
LateLintCheck,
TransCrateItem(DefId),
TransWriteMetadata,
CrateVariances,

Expand Down
3 changes: 0 additions & 3 deletions src/librustc/lint/context.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_incremental/persist/preds/mod.rs
Expand Up @@ -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,
}
Expand Down
18 changes: 0 additions & 18 deletions src/librustc_trans/trans_item.rs
Expand Up @@ -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};
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}
}
Expand Down
5 changes: 0 additions & 5 deletions src/librustc_typeck/coherence/overlap.rs
Expand Up @@ -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;

Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/test/compile-fail/dep-graph-assoc-type-trans.rs
Expand Up @@ -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, <char as Foo>::T gets
// normalized away, so at a certain point we had no edge to
Expand Down
2 changes: 0 additions & 2 deletions src/test/compile-fail/dep-graph-caller-callee.rs
Expand Up @@ -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();
}
Expand All @@ -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();
}
Expand Down
5 changes: 0 additions & 5 deletions src/test/compile-fail/dep-graph-trait-impl.rs
Expand Up @@ -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::<char>('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::<u32>(22);
}
Expand All @@ -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();
Expand Down
4 changes: 0 additions & 4 deletions src/test/incremental/dirty_clean.rs
Expand Up @@ -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
}
}
14 changes: 8 additions & 6 deletions src/test/incremental/krate-inlined.rs
Expand Up @@ -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![];
Expand Down
4 changes: 0 additions & 4 deletions src/test/incremental/remapped_paths_cc/main.rs
Expand Up @@ -25,17 +25,13 @@

extern crate extern_crate;

#[rustc_clean(label="TransCrateItem", cfg="rpass2")]
#[rustc_clean(label="TransCrateItem", cfg="rpass3")]
fn main() {
some_mod::some_fn();
}

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();
}
Expand Down

0 comments on commit 3f99118

Please sign in to comment.