Auto merge of #35166 - nikomatsakis:incr-comp-ice-34991-2, r=mw

Address ICEs running w/ incremental compilation and building glium

Fixes for various ICEs I encountered trying to build glium with incremental compilation enabled. Building glium now works. Of the 4 ICEs, I have test cases for 3 of them -- I didn't isolate a test for the last commit and kind of want to go do other things -- most notably, figuring out why incremental isn't saving much *effort*.

But if it seems worthwhile and I can come back and try to narrow down the problem.

r? @michaelwoerister

Fixes #34991
Fixes #32015

Author: bors

src/librustc/dep_graph/README.md

@@ -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
@@ -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.
+
+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.

src/librustc/dep_graph/dep_node.rs

@@ -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
@@ -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,
@@ -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),
@@ -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);
 

src/librustc/dep_graph/dep_tracking_map.rs

@@ -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)
@@ -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>> {

src/librustc/dep_graph/visit.rs

@@ -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);

src/librustc/hir/lowering.rs

@@ -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);

src/librustc/hir/map/def_collector.rs

@@ -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);
@@ -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 {
@@ -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));
                     }
                 }
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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);
         }
 
@@ -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()));
     }
 }
 
@@ -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);
@@ -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);
@@ -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()));
                     }
                 }
                 _ => {}
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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);
         }
 
@@ -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()));
     }
 }