Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address ICEs running w/ incremental compilation and building glium #35166

Merged
merged 24 commits into from
Aug 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
9294f8e
hash foreign items too
nikomatsakis Jul 29, 2016
c56eb4b
remap Hir(InlinedDefId) to MetaData(OriginalDefId)
nikomatsakis Jul 29, 2016
b4929d1
watch out for krate numbers being reassigned
nikomatsakis Jul 29, 2016
2797b2a
remove register_reads
nikomatsakis Jul 30, 2016
2e7df80
make metadata hashes determinstic
nikomatsakis Aug 1, 2016
903142a
dump statistics about re-use w/ -Z time-passes
nikomatsakis Aug 1, 2016
94acff1
replace graph rewriting with detecting inlined ids
nikomatsakis Aug 2, 2016
b13d504
improve log when something no longer exists
nikomatsakis Aug 2, 2016
54595ec
use memoized pattern for SizedConstraint
nikomatsakis Aug 2, 2016
bfbfe63
skip assert-dep-graph unless unit testing
nikomatsakis Aug 3, 2016
a6a97a9
rustfmt save.rs
nikomatsakis Aug 5, 2016
88b2e9a
rename KrateInfo to CrateInfo
nikomatsakis Aug 8, 2016
8fdc72f
track MIR through the dep-graph
nikomatsakis Aug 8, 2016
82b6dc2
fixup tests for new def'n of InlinedItem
nikomatsakis Aug 9, 2016
0e97240
isolate predecessor computation
nikomatsakis Aug 5, 2016
a92b1a7
make DepNode PartialOrd
nikomatsakis Aug 6, 2016
571010b
replace Name with InternedString in DefPathData
nikomatsakis Aug 6, 2016
d4bd054
add a `-Z incremental-info` flag
nikomatsakis Aug 6, 2016
8150494
add a `deterministic_hash` method to `DefPath`
nikomatsakis Aug 6, 2016
9978cbc
generalize BitMatrix to be NxM and not just NxN
nikomatsakis Aug 6, 2016
02a4703
use preds to serialize just what we need
nikomatsakis Aug 6, 2016
ecbcf1b
address comments from mw
nikomatsakis Aug 9, 2016
76eecc7
pacify the mercilous tidy
nikomatsakis Aug 9, 2016
e0b82d5
fix license
nikomatsakis Aug 9, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/librustc/dep_graph/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ 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
Expand Down Expand Up @@ -387,3 +391,24 @@ RUST_DEP_GRAPH_FILTER='Hir&foo -> TypeckItemBody & bar'
This will dump out all the nodes that lead from `Hir(foo)` to
`TypeckItemBody(bar)`, from which you can (hopefully) see the source
of the erroneous edge.

### 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good explanation.


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. You can
also use `tcx.map.is_inlined_def_id()` and
`tcx.map.is_inlined_node_id()` to test.
16 changes: 8 additions & 8 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ macro_rules! try_opt {
)
}

#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
pub enum DepNode<D: Clone + Debug> {
// The `D` type is "how definitions are identified".
// During compilation, it is always `DefId`, but when serializing
Expand Down Expand Up @@ -82,9 +82,11 @@ pub enum DepNode<D: Clone + Debug> {
Privacy,
IntrinsicCheck(D),
MatchCheck(D),
MirMapConstruction(D),
MirPass(D),
MirTypeck(D),

// Represents the MIR for a fn; also used as the task node for
// things read/modify that MIR.
Mir(D),

BorrowCheck(D),
RvalueCheck(D),
Reachability,
Expand Down Expand Up @@ -214,9 +216,7 @@ impl<D: Clone + Debug> DepNode<D> {
CheckConst(ref d) => op(d).map(CheckConst),
IntrinsicCheck(ref d) => op(d).map(IntrinsicCheck),
MatchCheck(ref d) => op(d).map(MatchCheck),
MirMapConstruction(ref d) => op(d).map(MirMapConstruction),
MirPass(ref d) => op(d).map(MirPass),
MirTypeck(ref d) => op(d).map(MirTypeck),
Mir(ref d) => op(d).map(Mir),
BorrowCheck(ref d) => op(d).map(BorrowCheck),
RvalueCheck(ref d) => op(d).map(RvalueCheck),
TransCrateItem(ref d) => op(d).map(TransCrateItem),
Expand Down Expand Up @@ -245,6 +245,6 @@ impl<D: Clone + Debug> DepNode<D> {
/// some independent path or string that persists between runs without
/// the need to be mapped or unmapped. (This ensures we can serialize
/// them even in the absence of a tcx.)
#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
pub struct WorkProductId(pub String);

10 changes: 10 additions & 0 deletions src/librustc/dep_graph/dep_tracking_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ impl<M: DepTrackingMapConfig> DepTrackingMap<M> {
self.map.get(k)
}

pub fn get_mut(&mut self, k: &M::Key) -> Option<&mut M::Value> {
self.read(k);
self.write(k);
self.map.get_mut(k)
}

pub fn insert(&mut self, k: M::Key, v: M::Value) -> Option<M::Value> {
self.write(&k);
self.map.insert(k, v)
Expand All @@ -70,6 +76,10 @@ impl<M: DepTrackingMapConfig> DepTrackingMap<M> {
self.read(k);
self.map.contains_key(k)
}

pub fn keys(&self) -> Vec<M::Key> {
self.map.keys().cloned().collect()
}
}

impl<M: DepTrackingMapConfig> MemoizationMap for RefCell<DepTrackingMap<M>> {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/dep_graph/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub fn visit_all_items_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let task_id = (self.dep_node_fn)(item_def_id);
let _task = self.tcx.dep_graph.in_task(task_id.clone());
debug!("Started task {:?}", task_id);
assert!(!self.tcx.map.is_inlined_def_id(item_def_id));
self.tcx.dep_graph.read(DepNode::Hir(item_def_id));
self.visitor.visit_item(i);
debug!("Ended task {:?}", task_id);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1854,7 +1854,7 @@ impl<'a> LoweringContext<'a> {

let parent_def = self.parent_def;
let def = self.resolver.definitions().map(|defs| {
let def_path_data = DefPathData::Binding(name);
let def_path_data = DefPathData::Binding(name.as_str());
let def_index = defs.create_def_with_parent(parent_def, pat.id, def_path_data);
Def::Local(DefId::local(def_index), pat.id)
}).unwrap_or(Def::Err);
Expand Down
68 changes: 35 additions & 33 deletions src/librustc/hir/map/def_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,11 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {
DefPathData::Impl,
ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Trait(..) |
ItemKind::ExternCrate(..) | ItemKind::ForeignMod(..) | ItemKind::Ty(..) =>
DefPathData::TypeNs(i.ident.name),
ItemKind::Mod(..) => DefPathData::Module(i.ident.name),
DefPathData::TypeNs(i.ident.name.as_str()),
ItemKind::Mod(..) => DefPathData::Module(i.ident.name.as_str()),
ItemKind::Static(..) | ItemKind::Const(..) | ItemKind::Fn(..) =>
DefPathData::ValueNs(i.ident.name),
ItemKind::Mac(..) => DefPathData::MacroDef(i.ident.name),
DefPathData::ValueNs(i.ident.name.as_str()),
ItemKind::Mac(..) => DefPathData::MacroDef(i.ident.name.as_str()),
ItemKind::Use(..) => DefPathData::Misc,
};
let def = self.create_def(i.id, def_data);
Expand All @@ -150,12 +150,12 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {
for v in &enum_definition.variants {
let variant_def_index =
this.create_def(v.node.data.id(),
DefPathData::EnumVariant(v.node.name.name));
DefPathData::EnumVariant(v.node.name.name.as_str()));
this.with_parent(variant_def_index, |this| {
for (index, field) in v.node.data.fields().iter().enumerate() {
let name = field.ident.map(|ident| ident.name)
.unwrap_or_else(|| token::intern(&index.to_string()));
this.create_def(field.id, DefPathData::Field(name));
this.create_def(field.id, DefPathData::Field(name.as_str()));
}

if let Some(ref expr) = v.node.disr_expr {
Expand All @@ -172,8 +172,8 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {
}

for (index, field) in struct_def.fields().iter().enumerate() {
let name = field.ident.map(|ident| ident.name)
.unwrap_or(token::intern(&index.to_string()));
let name = field.ident.map(|ident| ident.name.as_str())
.unwrap_or(token::intern(&index.to_string()).as_str());
this.create_def(field.id, DefPathData::Field(name));
}
}
Expand All @@ -184,7 +184,8 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {
}

fn visit_foreign_item(&mut self, foreign_item: &ForeignItem) {
let def = self.create_def(foreign_item.id, DefPathData::ValueNs(foreign_item.ident.name));
let def = self.create_def(foreign_item.id,
DefPathData::ValueNs(foreign_item.ident.name.as_str()));

self.with_parent(def, |this| {
visit::walk_foreign_item(this, foreign_item);
Expand All @@ -193,7 +194,7 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {

fn visit_generics(&mut self, generics: &Generics) {
for ty_param in generics.ty_params.iter() {
self.create_def(ty_param.id, DefPathData::TypeParam(ty_param.ident.name));
self.create_def(ty_param.id, DefPathData::TypeParam(ty_param.ident.name.as_str()));
}

visit::walk_generics(self, generics);
Expand All @@ -202,9 +203,9 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {
fn visit_trait_item(&mut self, ti: &TraitItem) {
let def_data = match ti.node {
TraitItemKind::Method(..) | TraitItemKind::Const(..) =>
DefPathData::ValueNs(ti.ident.name),
TraitItemKind::Type(..) => DefPathData::TypeNs(ti.ident.name),
TraitItemKind::Macro(..) => DefPathData::MacroDef(ti.ident.name),
DefPathData::ValueNs(ti.ident.name.as_str()),
TraitItemKind::Type(..) => DefPathData::TypeNs(ti.ident.name.as_str()),
TraitItemKind::Macro(..) => DefPathData::MacroDef(ti.ident.name.as_str()),
};

let def = self.create_def(ti.id, def_data);
Expand All @@ -220,9 +221,9 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {
fn visit_impl_item(&mut self, ii: &ImplItem) {
let def_data = match ii.node {
ImplItemKind::Method(..) | ImplItemKind::Const(..) =>
DefPathData::ValueNs(ii.ident.name),
ImplItemKind::Type(..) => DefPathData::TypeNs(ii.ident.name),
ImplItemKind::Macro(..) => DefPathData::MacroDef(ii.ident.name),
DefPathData::ValueNs(ii.ident.name.as_str()),
ImplItemKind::Type(..) => DefPathData::TypeNs(ii.ident.name.as_str()),
ImplItemKind::Macro(..) => DefPathData::MacroDef(ii.ident.name.as_str()),
};

let def = self.create_def(ii.id, def_data);
Expand All @@ -239,7 +240,7 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {
let parent_def = self.parent_def;

if let PatKind::Ident(_, id, _) = pat.node {
let def = self.create_def(pat.id, DefPathData::Binding(id.node.name));
let def = self.create_def(pat.id, DefPathData::Binding(id.node.name.as_str()));
self.parent_def = Some(def);
}

Expand Down Expand Up @@ -271,11 +272,11 @@ impl<'ast> visit::Visitor for DefCollector<'ast> {
}

fn visit_lifetime_def(&mut self, def: &LifetimeDef) {
self.create_def(def.lifetime.id, DefPathData::LifetimeDef(def.lifetime.name));
self.create_def(def.lifetime.id, DefPathData::LifetimeDef(def.lifetime.name.as_str()));
}

fn visit_macro_def(&mut self, macro_def: &MacroDef) {
self.create_def(macro_def.id, DefPathData::MacroDef(macro_def.ident.name));
self.create_def(macro_def.id, DefPathData::MacroDef(macro_def.ident.name.as_str()));
}
}

Expand All @@ -301,9 +302,9 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
hir::ItemEnum(..) | hir::ItemStruct(..) | hir::ItemTrait(..) |
hir::ItemExternCrate(..) | hir::ItemMod(..) | hir::ItemForeignMod(..) |
hir::ItemTy(..) =>
DefPathData::TypeNs(i.name),
DefPathData::TypeNs(i.name.as_str()),
hir::ItemStatic(..) | hir::ItemConst(..) | hir::ItemFn(..) =>
DefPathData::ValueNs(i.name),
DefPathData::ValueNs(i.name.as_str()),
hir::ItemUse(..) => DefPathData::Misc,
};
let def = self.create_def(i.id, def_data);
Expand All @@ -314,12 +315,12 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
for v in &enum_definition.variants {
let variant_def_index =
this.create_def(v.node.data.id(),
DefPathData::EnumVariant(v.node.name));
DefPathData::EnumVariant(v.node.name.as_str()));

this.with_parent(variant_def_index, |this| {
for field in v.node.data.fields() {
this.create_def(field.id,
DefPathData::Field(field.name));
DefPathData::Field(field.name.as_str()));
}
if let Some(ref expr) = v.node.disr_expr {
this.visit_hir_const_integer(expr);
Expand All @@ -335,7 +336,7 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
}

for field in struct_def.fields() {
this.create_def(field.id, DefPathData::Field(field.name));
this.create_def(field.id, DefPathData::Field(field.name.as_str()));
}
}
_ => {}
Expand All @@ -345,7 +346,8 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
}

fn visit_foreign_item(&mut self, foreign_item: &'ast hir::ForeignItem) {
let def = self.create_def(foreign_item.id, DefPathData::ValueNs(foreign_item.name));
let def = self.create_def(foreign_item.id,
DefPathData::ValueNs(foreign_item.name.as_str()));

self.with_parent(def, |this| {
intravisit::walk_foreign_item(this, foreign_item);
Expand All @@ -354,7 +356,7 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {

fn visit_generics(&mut self, generics: &'ast hir::Generics) {
for ty_param in generics.ty_params.iter() {
self.create_def(ty_param.id, DefPathData::TypeParam(ty_param.name));
self.create_def(ty_param.id, DefPathData::TypeParam(ty_param.name.as_str()));
}

intravisit::walk_generics(self, generics);
Expand All @@ -363,8 +365,8 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
fn visit_trait_item(&mut self, ti: &'ast hir::TraitItem) {
let def_data = match ti.node {
hir::MethodTraitItem(..) | hir::ConstTraitItem(..) =>
DefPathData::ValueNs(ti.name),
hir::TypeTraitItem(..) => DefPathData::TypeNs(ti.name),
DefPathData::ValueNs(ti.name.as_str()),
hir::TypeTraitItem(..) => DefPathData::TypeNs(ti.name.as_str()),
};

let def = self.create_def(ti.id, def_data);
Expand All @@ -380,8 +382,8 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
fn visit_impl_item(&mut self, ii: &'ast hir::ImplItem) {
let def_data = match ii.node {
hir::ImplItemKind::Method(..) | hir::ImplItemKind::Const(..) =>
DefPathData::ValueNs(ii.name),
hir::ImplItemKind::Type(..) => DefPathData::TypeNs(ii.name),
DefPathData::ValueNs(ii.name.as_str()),
hir::ImplItemKind::Type(..) => DefPathData::TypeNs(ii.name.as_str()),
};

let def = self.create_def(ii.id, def_data);
Expand All @@ -398,7 +400,7 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
let parent_def = self.parent_def;

if let hir::PatKind::Binding(_, name, _) = pat.node {
let def = self.create_def(pat.id, DefPathData::Binding(name.node));
let def = self.create_def(pat.id, DefPathData::Binding(name.node.as_str()));
self.parent_def = Some(def);
}

Expand Down Expand Up @@ -430,10 +432,10 @@ impl<'ast> intravisit::Visitor<'ast> for DefCollector<'ast> {
}

fn visit_lifetime_def(&mut self, def: &'ast hir::LifetimeDef) {
self.create_def(def.lifetime.id, DefPathData::LifetimeDef(def.lifetime.name));
self.create_def(def.lifetime.id, DefPathData::LifetimeDef(def.lifetime.name.as_str()));
}

fn visit_macro_def(&mut self, macro_def: &'ast hir::MacroDef) {
self.create_def(macro_def.id, DefPathData::MacroDef(macro_def.name));
self.create_def(macro_def.id, DefPathData::MacroDef(macro_def.name.as_str()));
}
}
Loading