Skip to content

Commit

Permalink
Auto merge of #39000 - nikomatsakis:incr_comp_crosscontaminate_impl_i…
Browse files Browse the repository at this point in the history
…tem, r=michaelwoerister

process trait/impl items directly from the visitor callback

The current setup processes impl/trait items while visiting
the impl/trait. This means we basically have this setup:

    <Lots> -> TypeckItemBody(Impl) -> Tables(ImplItem{0,1,2,3})

But this was largely an artifact of the older code. By moving the
processing of items into method dedicated for their use, we produce this
setup:

    <Little> -> TypeckItemBody(ImplItem0) -> Tables(ImplItem0)
    ...
    <Little> -> TypeckItemBody(ImplItem3) -> Tables(ImplItem3)

r? @michaelwoerister

Also, we might consider removing the `TypeckItemBody` node altogether and just using `Tables` as the task. `Tables` is its primary output, I imagine? That would reduce size of dep-graph somewhat.

cc @eddyb -- perhaps this pattern applies elsewhere?
  • Loading branch information
bors committed Jan 26, 2017
2 parents 6991938 + 282f7a3 commit 2f0463a
Show file tree
Hide file tree
Showing 52 changed files with 255 additions and 259 deletions.
2 changes: 1 addition & 1 deletion src/librustc/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use hir::{self, PatKind};

struct CFGBuilder<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
tables: &'a ty::Tables<'tcx>,
tables: &'a ty::TypeckTables<'tcx>,
graph: CFGGraph,
fn_exit: CFGIndex,
loop_scopes: Vec<LoopScope>,
Expand Down
43 changes: 12 additions & 31 deletions src/librustc/dep_graph/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,15 +326,15 @@ The idea is that you can annotate a test like:
#[rustc_if_this_changed]
fn foo() { }

#[rustc_then_this_would_need(TypeckItemBody)] //~ ERROR OK
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR OK
fn bar() { foo(); }

#[rustc_then_this_would_need(TypeckItemBody)] //~ ERROR no path
#[rustc_then_this_would_need(TypeckTables)] //~ ERROR no path
fn baz() { }
```

This will check whether there is a path in the dependency graph from
`Hir(foo)` to `TypeckItemBody(bar)`. An error is reported for each
`Hir(foo)` to `TypeckTables(bar)`. An error is reported for each
`#[rustc_then_this_would_need]` annotation that indicates whether a
path exists. `//~ ERROR` annotations can then be used to test if a
path is found (as demonstrated above).
Expand Down Expand Up @@ -371,27 +371,27 @@ A node is considered to match a filter if all of those strings appear in its
label. So, for example:

```
RUST_DEP_GRAPH_FILTER='-> TypeckItemBody'
RUST_DEP_GRAPH_FILTER='-> TypeckTables'
```

would select the predecessors of all `TypeckItemBody` nodes. Usually though you
want the `TypeckItemBody` node for some particular fn, so you might write:
would select the predecessors of all `TypeckTables` nodes. Usually though you
want the `TypeckTables` node for some particular fn, so you might write:

```
RUST_DEP_GRAPH_FILTER='-> TypeckItemBody & bar'
RUST_DEP_GRAPH_FILTER='-> TypeckTables & bar'
```

This will select only the `TypeckItemBody` nodes for fns with `bar` in their name.
This will select only the `TypeckTables` nodes for fns with `bar` in their name.

Perhaps you are finding that when you change `foo` you need to re-type-check `bar`,
but you don't think you should have to. In that case, you might do:

```
RUST_DEP_GRAPH_FILTER='Hir&foo -> TypeckItemBody & bar'
RUST_DEP_GRAPH_FILTER='Hir&foo -> TypeckTables & bar'
```

This will dump out all the nodes that lead from `Hir(foo)` to
`TypeckItemBody(bar)`, from which you can (hopefully) see the source
`TypeckTables(bar)`, from which you can (hopefully) see the source
of the erroneous edge.

#### Tracking down incorrect edges
Expand All @@ -417,27 +417,8 @@ dep-graph as described in the previous section and open `dep-graph.txt`
to see something like:

Hir(foo) -> Collect(bar)
Collect(bar) -> TypeckItemBody(bar)

Collect(bar) -> TypeckTables(bar)
That first edge looks suspicious to you. So you set
`RUST_FORBID_DEP_GRAPH_EDGE` to `Hir&foo -> Collect&bar`, re-run, and
then observe the backtrace. Voila, bug fixed!

### Inlining of HIR nodes

For the time being, at least, we still sometimes "inline" HIR nodes
from other crates into the current HIR map. This creates a weird
scenario where the same logical item (let's call it `X`) has two
def-ids: the original def-id `X` and a new, inlined one `X'`. `X'` is
in the current crate, but it's not like other HIR nodes: in
particular, when we restart compilation, it will not be available to
hash. Therefore, we do not want `Hir(X')` nodes appearing in our
graph. Instead, we want a "read" of `Hir(X')` to be represented as a
read of `MetaData(X)`, since the metadata for `X` is where the inlined
representation originated in the first place.

To achieve this, the HIR map will detect if the def-id originates in
an inlined node and add a dependency to a suitable `MetaData` node
instead. If you are reading a HIR node and are not sure if it may be
inlined or not, you can use `tcx.map.read(node_id)` and it will detect
whether the node is inlined or not and do the right thing.
9 changes: 3 additions & 6 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ pub enum DepNode<D: Clone + Debug> {
Variance,
WfCheck(D),
TypeckItemType(D),
TypeckItemBody(D),
Dropck,
DropckImpl(D),
UnusedTraitCheck,
Expand Down Expand Up @@ -113,7 +112,7 @@ pub enum DepNode<D: Clone + Debug> {
SizedConstraint(D),
AssociatedItemDefIds(D),
InherentImpls(D),
Tables(D),
TypeckTables(D),

// The set of impls for a given trait. Ultimately, it would be
// nice to get more fine-grained here (e.g., to include a
Expand Down Expand Up @@ -158,12 +157,11 @@ impl<D: Clone + Debug> DepNode<D> {
HirBody,
TransCrateItem,
TypeckItemType,
TypeckItemBody,
AssociatedItems,
ItemSignature,
AssociatedItemDefIds,
InherentImpls,
Tables,
TypeckTables,
TraitImpls,
ReprHints,
}
Expand Down Expand Up @@ -216,7 +214,6 @@ impl<D: Clone + Debug> DepNode<D> {
CoherenceOrphanCheck(ref d) => op(d).map(CoherenceOrphanCheck),
WfCheck(ref d) => op(d).map(WfCheck),
TypeckItemType(ref d) => op(d).map(TypeckItemType),
TypeckItemBody(ref d) => op(d).map(TypeckItemBody),
DropckImpl(ref d) => op(d).map(DropckImpl),
CheckConst(ref d) => op(d).map(CheckConst),
IntrinsicCheck(ref d) => op(d).map(IntrinsicCheck),
Expand All @@ -232,7 +229,7 @@ impl<D: Clone + Debug> DepNode<D> {
SizedConstraint(ref d) => op(d).map(SizedConstraint),
AssociatedItemDefIds(ref d) => op(d).map(AssociatedItemDefIds),
InherentImpls(ref d) => op(d).map(InherentImpls),
Tables(ref d) => op(d).map(Tables),
TypeckTables(ref d) => op(d).map(TypeckTables),
TraitImpls(ref d) => op(d).map(TraitImpls),
TraitItems(ref d) => op(d).map(TraitItems),
ReprHints(ref d) => op(d).map(ReprHints),
Expand Down
48 changes: 24 additions & 24 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,23 +76,23 @@ pub type Bound<T> = Option<T>;
pub type UnitResult<'tcx> = RelateResult<'tcx, ()>; // "unify result"
pub type FixupResult<T> = Result<T, FixupError>; // "fixup result"

/// A version of &ty::Tables which can be `Missing` (not needed),
/// A version of &ty::TypeckTables which can be `Missing` (not needed),
/// `InProgress` (during typeck) or `Interned` (result of typeck).
/// Only the `InProgress` version supports `borrow_mut`.
#[derive(Copy, Clone)]
pub enum InferTables<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
Interned(&'a ty::Tables<'gcx>),
InProgress(&'a RefCell<ty::Tables<'tcx>>),
Interned(&'a ty::TypeckTables<'gcx>),
InProgress(&'a RefCell<ty::TypeckTables<'tcx>>),
Missing
}

pub enum InferTablesRef<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
Interned(&'a ty::Tables<'gcx>),
InProgress(Ref<'a, ty::Tables<'tcx>>)
Interned(&'a ty::TypeckTables<'gcx>),
InProgress(Ref<'a, ty::TypeckTables<'tcx>>)
}

impl<'a, 'gcx, 'tcx> Deref for InferTablesRef<'a, 'gcx, 'tcx> {
type Target = ty::Tables<'tcx>;
type Target = ty::TypeckTables<'tcx>;
fn deref(&self) -> &Self::Target {
match *self {
InferTablesRef::Interned(tables) => tables,
Expand All @@ -112,7 +112,7 @@ impl<'a, 'gcx, 'tcx> InferTables<'a, 'gcx, 'tcx> {
}
}

pub fn expect_interned(self) -> &'a ty::Tables<'gcx> {
pub fn expect_interned(self) -> &'a ty::TypeckTables<'gcx> {
match self {
InferTables::Interned(tables) => tables,
InferTables::InProgress(_) => {
Expand All @@ -124,7 +124,7 @@ impl<'a, 'gcx, 'tcx> InferTables<'a, 'gcx, 'tcx> {
}
}

pub fn borrow_mut(self) -> RefMut<'a, ty::Tables<'tcx>> {
pub fn borrow_mut(self) -> RefMut<'a, ty::TypeckTables<'tcx>> {
match self {
InferTables::Interned(_) => {
bug!("InferTables: infcx.tables.borrow_mut() outside of type-checking");
Expand Down Expand Up @@ -407,51 +407,51 @@ impl fmt::Display for FixupError {

pub trait InferEnv<'a, 'tcx> {
fn to_parts(self, tcx: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::Tables<'tcx>>,
Option<ty::Tables<'tcx>>,
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>);
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for () {
fn to_parts(self, _: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::Tables<'tcx>>,
Option<ty::Tables<'tcx>>,
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
(None, None, None)
}
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for ty::ParameterEnvironment<'tcx> {
fn to_parts(self, _: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::Tables<'tcx>>,
Option<ty::Tables<'tcx>>,
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
(None, None, Some(self))
}
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for (&'a ty::Tables<'tcx>, ty::ParameterEnvironment<'tcx>) {
impl<'a, 'tcx> InferEnv<'a, 'tcx> for (&'a ty::TypeckTables<'tcx>, ty::ParameterEnvironment<'tcx>) {
fn to_parts(self, _: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::Tables<'tcx>>,
Option<ty::Tables<'tcx>>,
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
(Some(self.0), None, Some(self.1))
}
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for (ty::Tables<'tcx>, ty::ParameterEnvironment<'tcx>) {
impl<'a, 'tcx> InferEnv<'a, 'tcx> for (ty::TypeckTables<'tcx>, ty::ParameterEnvironment<'tcx>) {
fn to_parts(self, _: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::Tables<'tcx>>,
Option<ty::Tables<'tcx>>,
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
(None, Some(self.0), Some(self.1))
}
}

impl<'a, 'tcx> InferEnv<'a, 'tcx> for hir::BodyId {
fn to_parts(self, tcx: TyCtxt<'a, 'tcx, 'tcx>)
-> (Option<&'a ty::Tables<'tcx>>,
Option<ty::Tables<'tcx>>,
-> (Option<&'a ty::TypeckTables<'tcx>>,
Option<ty::TypeckTables<'tcx>>,
Option<ty::ParameterEnvironment<'tcx>>) {
let item_id = tcx.map.body_owner(self);
(Some(tcx.item_tables(tcx.map.local_def_id(item_id))),
Expand All @@ -466,8 +466,8 @@ impl<'a, 'tcx> InferEnv<'a, 'tcx> for hir::BodyId {
pub struct InferCtxtBuilder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
global_tcx: TyCtxt<'a, 'gcx, 'gcx>,
arena: DroplessArena,
fresh_tables: Option<RefCell<ty::Tables<'tcx>>>,
tables: Option<&'a ty::Tables<'gcx>>,
fresh_tables: Option<RefCell<ty::TypeckTables<'tcx>>>,
tables: Option<&'a ty::TypeckTables<'gcx>>,
param_env: Option<ty::ParameterEnvironment<'gcx>>,
projection_mode: Reveal,
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ pub struct LateContext<'a, 'tcx: 'a> {
pub tcx: TyCtxt<'a, 'tcx, 'tcx>,

/// Side-tables for the body we are in.
pub tables: &'a ty::Tables<'tcx>,
pub tables: &'a ty::TypeckTables<'tcx>,

/// The crate being checked.
pub krate: &'a hir::Crate,
Expand Down Expand Up @@ -1212,7 +1212,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let lint_store = mem::replace(&mut *tcx.sess.lint_store.borrow_mut(), LintStore::new());
let mut cx = LateContext {
tcx: tcx,
tables: &ty::Tables::empty(),
tables: &ty::TypeckTables::empty(),
krate: krate,
access_levels: access_levels,
lints: lint_store,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ fn should_explore<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
struct MarkSymbolVisitor<'a, 'tcx: 'a> {
worklist: Vec<ast::NodeId>,
tcx: TyCtxt<'a, 'tcx, 'tcx>,
tables: &'a ty::Tables<'tcx>,
tables: &'a ty::TypeckTables<'tcx>,
live_symbols: Box<FxHashSet<ast::NodeId>>,
struct_has_extern_repr: bool,
ignore_non_const_paths: bool,
Expand Down Expand Up @@ -392,7 +392,7 @@ fn find_live<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let mut symbol_visitor = MarkSymbolVisitor {
worklist: worklist,
tcx: tcx,
tables: &ty::Tables::empty(),
tables: &ty::TypeckTables::empty(),
live_symbols: box FxHashSet(),
struct_has_extern_repr: false,
ignore_non_const_paths: false,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn type_is_unsafe_function(ty: Ty) -> bool {

struct EffectCheckVisitor<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
tables: &'a ty::Tables<'tcx>,
tables: &'a ty::TypeckTables<'tcx>,

/// Whether we're in an unsafe context.
unsafe_context: UnsafeContext,
Expand Down Expand Up @@ -245,7 +245,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {

let mut visitor = EffectCheckVisitor {
tcx: tcx,
tables: &ty::Tables::empty(),
tables: &ty::TypeckTables::empty(),
unsafe_context: UnsafeContext::new(SafeContext),
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ const ACC_USE: u32 = 4;

struct Liveness<'a, 'tcx: 'a> {
ir: &'a mut IrMaps<'a, 'tcx>,
tables: &'a ty::Tables<'tcx>,
tables: &'a ty::TypeckTables<'tcx>,
s: Specials,
successors: Vec<LiveNode>,
users: Vec<Users>,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ fn method_might_be_inlined<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
struct ReachableContext<'a, 'tcx: 'a> {
// The type context.
tcx: TyCtxt<'a, 'tcx, 'tcx>,
tables: &'a ty::Tables<'tcx>,
tables: &'a ty::TypeckTables<'tcx>,
// The set of items which must be exported in the linkage sense.
reachable_symbols: NodeSet,
// A worklist of item IDs. Each item ID in this worklist will be inlined
Expand Down Expand Up @@ -370,7 +370,7 @@ pub fn find_reachable<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
});
let mut reachable_context = ReachableContext {
tcx: tcx,
tables: &ty::Tables::empty(),
tables: &ty::TypeckTables::empty(),
reachable_symbols: NodeSet(),
worklist: Vec::new(),
any_library: any_library,
Expand Down
14 changes: 7 additions & 7 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub struct GlobalArenas<'tcx> {
trait_def: TypedArena<ty::TraitDef>,
adt_def: TypedArena<ty::AdtDef>,
mir: TypedArena<RefCell<Mir<'tcx>>>,
tables: TypedArena<ty::Tables<'tcx>>,
tables: TypedArena<ty::TypeckTables<'tcx>>,
}

impl<'tcx> GlobalArenas<'tcx> {
Expand Down Expand Up @@ -192,7 +192,7 @@ pub struct CommonTypes<'tcx> {
}

#[derive(RustcEncodable, RustcDecodable)]
pub struct Tables<'tcx> {
pub struct TypeckTables<'tcx> {
/// Resolved definitions for `<T>::X` associated paths.
pub type_relative_path_defs: NodeMap<Def>,

Expand Down Expand Up @@ -234,9 +234,9 @@ pub struct Tables<'tcx> {
pub fru_field_types: NodeMap<Vec<Ty<'tcx>>>
}

impl<'tcx> Tables<'tcx> {
pub fn empty() -> Tables<'tcx> {
Tables {
impl<'tcx> TypeckTables<'tcx> {
pub fn empty() -> TypeckTables<'tcx> {
TypeckTables {
type_relative_path_defs: NodeMap(),
node_types: FxHashMap(),
item_substs: NodeMap(),
Expand Down Expand Up @@ -402,7 +402,7 @@ pub struct GlobalCtxt<'tcx> {
free_region_maps: RefCell<NodeMap<FreeRegionMap>>,
// FIXME: jroesch make this a refcell

pub tables: RefCell<DepTrackingMap<maps::Tables<'tcx>>>,
pub tables: RefCell<DepTrackingMap<maps::TypeckTables<'tcx>>>,

/// Maps from a trait item to the trait item "descriptor"
pub associated_items: RefCell<DepTrackingMap<maps::AssociatedItems<'tcx>>>,
Expand Down Expand Up @@ -654,7 +654,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
self.global_arenas.mir.alloc(RefCell::new(mir))
}

pub fn alloc_tables(self, tables: ty::Tables<'gcx>) -> &'gcx ty::Tables<'gcx> {
pub fn alloc_tables(self, tables: ty::TypeckTables<'gcx>) -> &'gcx ty::TypeckTables<'gcx> {
self.global_arenas.tables.alloc(tables)
}

Expand Down
Loading

0 comments on commit 2f0463a

Please sign in to comment.