Move caching of HIR-inlining into CStore in order to avoid duplicating inlined HIR.

Author: michaelwoerister

src/librustc/hir/intravisit.rs

@@ -867,7 +867,7 @@ pub fn walk_vis<'v, V: Visitor<'v>>(visitor: &mut V, vis: &'v Visibility) {
     }
 }
 
-#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug)]
+#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, PartialEq, Eq)]
 pub struct IdRange {
     pub min: NodeId,
     pub max: NodeId,
@@ -893,6 +893,7 @@ impl IdRange {
         self.min = cmp::min(self.min, id);
         self.max = cmp::max(self.max, id + 1);
     }
+
 }
 
 

src/librustc/hir/map/mod.rs

@@ -32,6 +32,7 @@ use hir::print as pprust;
 
 use arena::TypedArena;
 use std::cell::RefCell;
+use std::cmp;
 use std::io;
 use std::mem;
 
@@ -127,7 +128,10 @@ impl<'ast> MapEntry<'ast> {
             EntryStructCtor(id, _) => id,
             EntryLifetime(id, _) => id,
             EntryTyParam(id, _) => id,
-            _ => return None
+
+            NotPresent |
+            RootCrate |
+            RootInlinedParent(_) => return None,
         })
     }
 
@@ -196,6 +200,10 @@ pub struct Map<'ast> {
     map: RefCell<Vec<MapEntry<'ast>>>,
 
     definitions: RefCell<Definitions>,
+
+    /// All NodeIds that are numerically greater or equal to this value come
+    /// from inlined items.
+    local_node_id_watermark: NodeId,
 }
 
 impl<'ast> Map<'ast> {
@@ -550,6 +558,13 @@ impl<'ast> Map<'ast> {
         }
     }
 
+    pub fn expect_inlined_item(&self, id: NodeId) -> &'ast InlinedItem {
+        match self.find_entry(id) {
+            Some(RootInlinedParent(inlined_item)) => inlined_item,
+            _ => bug!("expected inlined item, found {}", self.node_to_string(id)),
+        }
+    }
+
     /// Returns the name associated with the given NodeId's AST.
     pub fn name(&self, id: NodeId) -> Name {
         match self.get(id) {
@@ -649,6 +664,10 @@ impl<'ast> Map<'ast> {
     pub fn node_to_user_string(&self, id: NodeId) -> String {
         node_id_to_string(self, id, false)
     }
+
+    pub fn is_inlined(&self, id: NodeId) -> bool {
+        id >= self.local_node_id_watermark
+    }
 }
 
 pub struct NodesMatchingSuffix<'a, 'ast:'a> {
@@ -765,13 +784,37 @@ pub trait FoldOps {
 }
 
 /// A Folder that updates IDs and Span's according to fold_ops.
-struct IdAndSpanUpdater<F> {
-    fold_ops: F
+pub struct IdAndSpanUpdater<F> {
+    fold_ops: F,
+    min_id_assigned: NodeId,
+    max_id_assigned: NodeId,
+}
+
+impl<F: FoldOps> IdAndSpanUpdater<F> {
+    pub fn new(fold_ops: F) -> IdAndSpanUpdater<F> {
+        IdAndSpanUpdater {
+            fold_ops: fold_ops,
+            min_id_assigned: ::std::u32::MAX,
+            max_id_assigned: ::std::u32::MIN,
+        }
+    }
+
+    pub fn id_range(&self) -> intravisit::IdRange {
+        intravisit::IdRange {
+            min: self.min_id_assigned,
+            max: self.max_id_assigned + 1,
+        }
+    }
 }
 
 impl<F: FoldOps> Folder for IdAndSpanUpdater<F> {
     fn new_id(&mut self, id: NodeId) -> NodeId {
-        self.fold_ops.new_id(id)
+        let id = self.fold_ops.new_id(id);
+
+        self.min_id_assigned = cmp::min(self.min_id_assigned, id);
+        self.max_id_assigned = cmp::max(self.max_id_assigned, id);
+
+        id
     }
 
     fn new_span(&mut self, span: Span) -> Span {
@@ -802,11 +845,14 @@ pub fn map_crate<'ast>(forest: &'ast mut Forest,
               entries, vector_length, (entries as f64 / vector_length as f64) * 100.);
     }
 
+    let local_node_id_watermark = map.len() as NodeId;
+
     Map {
         forest: forest,
         dep_graph: forest.dep_graph.clone(),
         map: RefCell::new(map),
         definitions: RefCell::new(definitions),
+        local_node_id_watermark: local_node_id_watermark
     }
 }
 
@@ -818,7 +864,7 @@ pub fn map_decoded_item<'ast, F: FoldOps>(map: &Map<'ast>,
                                           ii: InlinedItem,
                                           fold_ops: F)
                                           -> &'ast InlinedItem {
-    let mut fld = IdAndSpanUpdater { fold_ops: fold_ops };
+    let mut fld = IdAndSpanUpdater::new(fold_ops);
     let ii = match ii {
         II::Item(i) => II::Item(i.map(|i| fld.fold_item(i))),
         II::TraitItem(d, ti) => {
@@ -835,6 +881,12 @@ pub fn map_decoded_item<'ast, F: FoldOps>(map: &Map<'ast>,
     let ii = map.forest.inlined_items.alloc(ii);
     let ii_parent_id = fld.new_id(DUMMY_NODE_ID);
 
+    // Assert that the ii_parent_id is the last NodeId in our reserved range
+    assert!(ii_parent_id == fld.max_id_assigned);
+    // Assert that we did not violate the invariant that all inlined HIR items
+    // have NodeIds greater than or equal to `local_node_id_watermark`
+    assert!(fld.min_id_assigned >= map.local_node_id_watermark);
+
     let defs = &mut *map.definitions.borrow_mut();
     let mut def_collector = DefCollector::extend(ii_parent_id,
                                                  parent_def_path.clone(),

src/librustc/middle/cstore.rs

@@ -120,12 +120,6 @@ pub struct ChildItem {
     pub vis: ty::Visibility,
 }
 
-pub enum FoundAst<'ast> {
-    Found(&'ast InlinedItem),
-    FoundParent(DefId, &'ast hir::Item),
-    NotFound,
-}
-
 #[derive(Copy, Clone, Debug)]
 pub struct ExternCrate {
     /// def_id of an `extern crate` in the current crate that caused
@@ -250,7 +244,10 @@ pub trait CrateStore<'tcx> {
 
     // misc. metadata
     fn maybe_get_item_ast<'a>(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>, def: DefId)
-                              -> FoundAst<'tcx>;
+                              -> Option<(&'tcx InlinedItem, ast::NodeId)>;
+    fn local_node_for_inlined_defid(&'tcx self, def_id: DefId) -> Option<ast::NodeId>;
+    fn defid_for_inlined_node(&'tcx self, node_id: ast::NodeId) -> Option<DefId>;
+
     fn maybe_get_item_mir<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, def: DefId)
                               -> Option<Mir<'tcx>>;
     fn is_item_mir_available(&self, def: DefId) -> bool;
@@ -447,7 +444,16 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
 
     // misc. metadata
     fn maybe_get_item_ast<'a>(&'tcx self, tcx: TyCtxt<'a, 'tcx, 'tcx>, def: DefId)
-                              -> FoundAst<'tcx> { bug!("maybe_get_item_ast") }
+                              -> Option<(&'tcx InlinedItem, ast::NodeId)> {
+        bug!("maybe_get_item_ast")
+    }
+    fn local_node_for_inlined_defid(&'tcx self, def_id: DefId) -> Option<ast::NodeId> {
+        bug!("local_node_for_inlined_defid")
+    }
+    fn defid_for_inlined_node(&'tcx self, node_id: ast::NodeId) -> Option<DefId> {
+        bug!("defid_for_inlined_node")
+    }
+
     fn maybe_get_item_mir<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, def: DefId)
                               -> Option<Mir<'tcx>> { bug!("maybe_get_item_mir") }
     fn is_item_mir_available(&self, def: DefId) -> bool {

src/librustc_const_eval/eval.rs

@@ -17,7 +17,7 @@ use self::EvalHint::*;
 
 use rustc::hir::map as ast_map;
 use rustc::hir::map::blocks::FnLikeNode;
-use rustc::middle::cstore::{self, InlinedItem};
+use rustc::middle::cstore::InlinedItem;
 use rustc::traits;
 use rustc::hir::def::{Def, PathResolution};
 use rustc::hir::def_id::DefId;
@@ -142,13 +142,13 @@ pub fn lookup_const_by_id<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         }
         let mut used_substs = false;
         let expr_ty = match tcx.sess.cstore.maybe_get_item_ast(tcx, def_id) {
-            cstore::FoundAst::Found(&InlinedItem::Item(ref item)) => match item.node {
+            Some((&InlinedItem::Item(ref item), _)) => match item.node {
                 hir::ItemConst(ref ty, ref const_expr) => {
                     Some((&**const_expr, tcx.ast_ty_to_prim_ty(ty)))
                 },
                 _ => None
             },
-            cstore::FoundAst::Found(&InlinedItem::TraitItem(trait_id, ref ti)) => match ti.node {
+            Some((&InlinedItem::TraitItem(trait_id, ref ti), _)) => match ti.node {
                 hir::ConstTraitItem(_, _) => {
                     used_substs = true;
                     if let Some(substs) = substs {
@@ -163,7 +163,7 @@ pub fn lookup_const_by_id<'a, 'tcx: 'a>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                 }
                 _ => None
             },
-            cstore::FoundAst::Found(&InlinedItem::ImplItem(_, ref ii)) => match ii.node {
+            Some((&InlinedItem::ImplItem(_, ref ii), _)) => match ii.node {
                 hir::ImplItemKind::Const(ref ty, ref expr) => {
                     Some((&**expr, tcx.ast_ty_to_prim_ty(ty)))
                 },
@@ -198,8 +198,8 @@ fn inline_const_fn_from_external_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     }
 
     let fn_id = match tcx.sess.cstore.maybe_get_item_ast(tcx, def_id) {
-        cstore::FoundAst::Found(&InlinedItem::Item(ref item)) => Some(item.id),
-        cstore::FoundAst::Found(&InlinedItem::ImplItem(_, ref item)) => Some(item.id),
+        Some((&InlinedItem::Item(ref item), _)) => Some(item.id),
+        Some((&InlinedItem::ImplItem(_, ref item), _)) => Some(item.id),
         _ => None
     };
     tcx.extern_const_fns.borrow_mut().insert(def_id,

src/librustc_metadata/astencode.rs

@@ -88,8 +88,9 @@ pub fn encode_inlined_item(ecx: &e::EncodeContext,
            rbml_w.writer.seek(SeekFrom::Current(0)));
 
     // Folding could be avoided with a smarter encoder.
-    let ii = simplify_ast(ii);
+    let (ii, expected_id_range) = simplify_ast(ii);
     let id_range = inlined_item_id_range(&ii);
+    assert_eq!(expected_id_range, id_range);
 
     rbml_w.start_tag(c::tag_ast as usize);
     id_range.encode(rbml_w);
@@ -186,6 +187,10 @@ impl<'a, 'b, 'tcx> DecodeContext<'a, 'b, 'tcx> {
     pub fn tr_id(&self, id: ast::NodeId) -> ast::NodeId {
         // from_id_range should be non-empty
         assert!(!self.from_id_range.empty());
+        // Make sure that translating the NodeId will actually yield a
+        // meaningful result
+        assert!(self.from_id_range.contains(id));
+
         // Use wrapping arithmetic because otherwise it introduces control flow.
         // Maybe we should just have the control flow? -- aatch
         (id.wrapping_sub(self.from_id_range.min).wrapping_add(self.to_id_range.min))
@@ -279,9 +284,23 @@ fn encode_ast(rbml_w: &mut Encoder, item: &InlinedItem) {
     rbml_w.end_tag();
 }
 
-struct NestedItemsDropper;
+struct NestedItemsDropper {
+    id_range: IdRange
+}
 
 impl Folder for NestedItemsDropper {
+
+    // The unit tests below run on HIR with NodeIds not properly assigned. That
+    // causes an integer overflow. So we just don't track the id_range when
+    // building the unit tests.
+    #[cfg(not(test))]
+    fn new_id(&mut self, id: ast::NodeId) -> ast::NodeId {
+        // Record the range of NodeIds we are visiting, so we can do a sanity
+        // check later
+        self.id_range.add(id);
+        id
+    }
+
     fn fold_block(&mut self, blk: P<hir::Block>) -> P<hir::Block> {
         blk.and_then(|hir::Block {id, stmts, expr, rules, span, ..}| {
             let stmts_sans_items = stmts.into_iter().filter_map(|stmt| {
@@ -322,10 +341,12 @@ impl Folder for NestedItemsDropper {
 // As it happens, trans relies on the fact that we do not export
 // nested items, as otherwise it would get confused when translating
 // inlined items.
-fn simplify_ast(ii: InlinedItemRef) -> InlinedItem {
-    let mut fld = NestedItemsDropper;
+fn simplify_ast(ii: InlinedItemRef) -> (InlinedItem, IdRange) {
+    let mut fld = NestedItemsDropper {
+        id_range: IdRange::max()
+    };
 
-    match ii {
+    let ii = match ii {
         // HACK we're not dropping items.
         InlinedItemRef::Item(i) => {
             InlinedItem::Item(P(fold::noop_fold_item(i.clone(), &mut fld)))
@@ -339,7 +360,9 @@ fn simplify_ast(ii: InlinedItemRef) -> InlinedItem {
         InlinedItemRef::Foreign(i) => {
             InlinedItem::Foreign(P(fold::noop_fold_foreign_item(i.clone(), &mut fld)))
         }
-    }
+    };
+
+    (ii, fld.id_range)
 }
 
 fn decode_ast(item_doc: rbml::Doc) -> InlinedItem {
@@ -361,8 +384,18 @@ impl tr for Def {
         match *self {
           Def::Fn(did) => Def::Fn(did.tr(dcx)),
           Def::Method(did) => Def::Method(did.tr(dcx)),
-          Def::SelfTy(opt_did, impl_id) => { Def::SelfTy(opt_did.map(|did| did.tr(dcx)),
-                                                         impl_id.map(|id| dcx.tr_id(id))) }
+          Def::SelfTy(opt_did, impl_id) => {
+              // Since the impl_id will never lie within the reserved range of
+              // imported NodeIds, it does not make sense to translate it.
+              // The result would not make any sense within the importing crate.
+              // We also don't allow for impl items to be inlined (just their
+              // members), so even if we had a DefId here, we wouldn't be able
+              // to do much with it.
+              // So, we set the id to DUMMY_NODE_ID. That way we make it
+              // explicit that this is no usable NodeId.
+              Def::SelfTy(opt_did.map(|did| did.tr(dcx)),
+                          impl_id.map(|_| ast::DUMMY_NODE_ID))
+          }
           Def::Mod(did) => { Def::Mod(did.tr(dcx)) }
           Def::ForeignMod(did) => { Def::ForeignMod(did.tr(dcx)) }
           Def::Static(did, m) => { Def::Static(did.tr(dcx), m) }
@@ -1361,7 +1394,7 @@ fn test_simplification() {
     with_testing_context(|lcx| {
         let hir_item = lcx.lower_item(&item);
         let item_in = InlinedItemRef::Item(&hir_item);
-        let item_out = simplify_ast(item_in);
+        let (item_out, _) = simplify_ast(item_in);
         let item_exp = InlinedItem::Item(P(lcx.lower_item(&quote_item!(&cx,
             fn new_int_alist<B>() -> alist<isize, B> {
                 return alist {eq_fn: eq_int, data: Vec::new()};