From: Niko Matsakis Date: Fri, 29 Jul 2016 20:49:26 +0000 (-0400) Subject: remap Hir(InlinedDefId) to MetaData(OriginalDefId) X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=c56eb4b7f5206f157aa200e8424f697791990d27;p=rust.git remap Hir(InlinedDefId) to MetaData(OriginalDefId) The way we do HIR inlining introduces reads of the "Hir" into the graph, but this Hir in fact belongs to other crates, so when we try to load later, we ICE because the Hir nodes in question don't belond to the crate (and we haven't done inlining yet). This pass rewrites those HIR nodes to the metadata from which the inlined HIR was loaded. --- diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index aed3613f44e..eea8ad9926c 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -204,9 +204,21 @@ pub struct Map<'ast> { /// All NodeIds that are numerically greater or equal to this value come /// from inlined items. local_node_id_watermark: NodeId, + + /// All def-indices that are numerically greater or equal to this value come + /// from inlined items. + local_def_id_watermark: usize, } impl<'ast> Map<'ast> { + pub fn is_inlined_def_id(&self, id: DefId) -> bool { + id.is_local() && id.index.as_usize() >= self.local_def_id_watermark + } + + pub fn is_inlined_node_id(&self, id: NodeId) -> bool { + id >= self.local_node_id_watermark + } + /// Registers a read in the dependency graph of the AST node with /// the given `id`. This needs to be called each time a public /// function returns the HIR for a node -- in other words, when it @@ -664,10 +676,6 @@ pub fn node_to_string(&self, id: NodeId) -> String { 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> { @@ -846,13 +854,15 @@ pub fn map_crate<'ast>(forest: &'ast mut Forest, } let local_node_id_watermark = map.len() as NodeId; + let local_def_id_watermark = definitions.len(); 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 + local_node_id_watermark: local_node_id_watermark, + local_def_id_watermark: local_def_id_watermark, } } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 5444dd94761..77cc62060aa 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -525,7 +525,7 @@ pub fn def_index_for_def_key(self, krate: ast::CrateNum, key: DefKey) } pub fn retrace_path(self, path: &DefPath) -> Option { - debug!("retrace_path(path={:?})", path); + debug!("retrace_path(path={:?}, krate={:?})", path, self.crate_name(path.krate)); let root_key = DefKey { parent: None, diff --git a/src/librustc_incremental/persist/hash.rs b/src/librustc_incremental/persist/hash.rs index 99119dd184c..e16371dd0ce 100644 --- a/src/librustc_incremental/persist/hash.rs +++ b/src/librustc_incremental/persist/hash.rs @@ -43,7 +43,6 @@ pub fn hash(&mut self, dep_node: &DepNode) -> Option { match *dep_node { // HIR nodes (which always come from our crate) are an input: DepNode::Hir(def_id) => { - assert!(def_id.is_local()); Some(self.hir_hash(def_id)) } @@ -66,7 +65,11 @@ pub fn hash(&mut self, dep_node: &DepNode) -> Option { } fn hir_hash(&mut self, def_id: DefId) -> u64 { - assert!(def_id.is_local()); + assert!(def_id.is_local(), + "cannot hash HIR for non-local def-id {:?} => {:?}", + def_id, + self.tcx.item_path_str(def_id)); + // FIXME(#32753) -- should we use a distinct hash here self.tcx.calculate_item_hash(def_id) } diff --git a/src/librustc_incremental/persist/load.rs b/src/librustc_incremental/persist/load.rs index 36b6c79c40f..82b1da67a4d 100644 --- a/src/librustc_incremental/persist/load.rs +++ b/src/librustc_incremental/persist/load.rs @@ -93,7 +93,6 @@ fn load_data(sess: &Session, path: &Path) -> Option> { None } } - } /// Decode the dep graph and load the edges/nodes that are still clean @@ -108,14 +107,9 @@ pub fn decode_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let directory = try!(DefIdDirectory::decode(&mut dep_graph_decoder)); let serialized_dep_graph = try!(SerializedDepGraph::decode(&mut dep_graph_decoder)); - debug!("decode_dep_graph: directory = {:#?}", directory); - debug!("decode_dep_graph: serialized_dep_graph = {:#?}", serialized_dep_graph); - // Retrace the paths in the directory to find their current location (if any). let retraced = directory.retrace(tcx); - debug!("decode_dep_graph: retraced = {:#?}", retraced); - // Compute the set of Hir nodes whose data has changed. let mut dirty_nodes = initial_dirty_nodes(tcx, &serialized_dep_graph.hashes, &retraced); @@ -169,6 +163,7 @@ fn initial_dirty_nodes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let mut items_removed = false; let mut dirty_nodes = FnvHashSet(); for hash in hashes { + debug!("initial_dirty_nodes: retracing {:?}", hash); match hash.node.map_def(|&i| retraced.def_id(i)) { Some(dep_node) => { let current_hash = hcx.hash(&dep_node).unwrap(); diff --git a/src/librustc_incremental/persist/save.rs b/src/librustc_incremental/persist/save.rs index 305250d5962..8bc1e80fc7c 100644 --- a/src/librustc_incremental/persist/save.rs +++ b/src/librustc_incremental/persist/save.rs @@ -9,10 +9,12 @@ // except according to those terms. use rbml::opaque::Encoder; -use rustc::dep_graph::DepNode; +use rustc::dep_graph::{DepGraphQuery, DepNode}; +use rustc::hir::def_id::DefId; use rustc::middle::cstore::LOCAL_CRATE; use rustc::session::Session; use rustc::ty::TyCtxt; +use rustc_data_structures::fnv::FnvHashMap; use rustc_serialize::{Encodable as RustcEncodable}; use std::hash::{Hasher, SipHasher}; use std::io::{self, Cursor, Write}; @@ -99,15 +101,15 @@ pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>, { let tcx = hcx.tcx; let query = tcx.dep_graph.query(); + let (nodes, edges) = post_process_graph(hcx, query); let mut builder = DefIdDirectoryBuilder::new(tcx); // Create hashes for inputs. let hashes = - query.nodes() - .into_iter() + nodes.iter() .filter_map(|dep_node| { - hcx.hash(&dep_node) + hcx.hash(dep_node) .map(|hash| { let node = builder.map(dep_node); SerializedHash { node: node, hash: hash } @@ -117,16 +119,14 @@ pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>, // Create the serialized dep-graph. let graph = SerializedDepGraph { - nodes: query.nodes().into_iter() - .map(|node| builder.map(node)) - .collect(), - edges: query.edges().into_iter() - .map(|(source_node, target_node)| { - let source = builder.map(source_node); - let target = builder.map(target_node); - (source, target) - }) - .collect(), + nodes: nodes.iter().map(|node| builder.map(node)).collect(), + edges: edges.iter() + .map(|&(ref source_node, ref target_node)| { + let source = builder.map(source_node); + let target = builder.map(target_node); + (source, target) + }) + .collect(), hashes: hashes, }; @@ -140,6 +140,57 @@ pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>, Ok(()) } +pub fn post_process_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>, + query: DepGraphQuery) + -> (Vec>, Vec<(DepNode, DepNode)>) +{ + let tcx = hcx.tcx; + let mut cache = FnvHashMap(); + + let mut uninline_def_id = |def_id: DefId| -> Option { + if tcx.map.is_inlined_def_id(def_id) { + Some( + cache.entry(def_id) + .or_insert_with(|| { + let def_path = tcx.def_path(def_id); + debug!("post_process_graph: uninlining def-id {:?} to yield {:?}", + def_id, def_path); + let retraced_def_id = tcx.retrace_path(&def_path).unwrap(); + debug!("post_process_graph: retraced to {:?}", retraced_def_id); + retraced_def_id + }) + .clone()) + } else { + None + } + }; + + let mut uninline_metadata = |node: &DepNode| -> DepNode { + match *node { + DepNode::Hir(def_id) => { + match uninline_def_id(def_id) { + Some(uninlined_def_id) => DepNode::MetaData(uninlined_def_id), + None => DepNode::Hir(def_id) + } + } + _ => node.clone() + } + }; + + let nodes = query.nodes() + .into_iter() + .map(|node| uninline_metadata(node)) + .collect(); + + let edges = query.edges() + .into_iter() + .map(|(from, to)| (uninline_metadata(from), uninline_metadata(to))) + .collect(); + + (nodes, edges) +} + + pub fn encode_metadata_hashes<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>, encoder: &mut Encoder) -> io::Result<()> diff --git a/src/librustc_trans/common.rs b/src/librustc_trans/common.rs index 61d8a0837c1..6a1fc6bcffb 100644 --- a/src/librustc_trans/common.rs +++ b/src/librustc_trans/common.rs @@ -1244,7 +1244,7 @@ pub fn inlined_variant_def<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, }), ..}) => ty, _ => ctor_ty }.ty_adt_def().unwrap(); - let variant_def_id = if ccx.tcx().map.is_inlined(inlined_vid) { + let variant_def_id = if ccx.tcx().map.is_inlined_node_id(inlined_vid) { ccx.defid_for_inlined_node(inlined_vid).unwrap() } else { ccx.tcx().map.local_def_id(inlined_vid) diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs index 27048994254..84c3b41fd0a 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -1026,7 +1026,7 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId) .get(TransItem::Static(id)) .expect("Local statics should always be in the SymbolMap"); // Make sure that this is never executed for something inlined. - assert!(!ccx.tcx().map.is_inlined(id)); + assert!(!ccx.tcx().map.is_inlined_node_id(id)); let defined_in_current_codegen_unit = ccx.codegen_unit() .items() diff --git a/src/librustc_trans/debuginfo/metadata.rs b/src/librustc_trans/debuginfo/metadata.rs index 1d718d4b57a..e8051212552 100644 --- a/src/librustc_trans/debuginfo/metadata.rs +++ b/src/librustc_trans/debuginfo/metadata.rs @@ -326,7 +326,7 @@ fn from_def_id_and_substs<'a, 'tcx>(type_map: &mut TypeMap<'tcx>, // First, find out the 'real' def_id of the type. Items inlined from // other crates have to be mapped back to their source. let def_id = if let Some(node_id) = cx.tcx().map.as_local_node_id(def_id) { - if cx.tcx().map.is_inlined(node_id) { + if cx.tcx().map.is_inlined_node_id(node_id) { // The given def_id identifies the inlined copy of a // type definition, let's take the source of the copy. cx.defid_for_inlined_node(node_id).unwrap() @@ -1845,7 +1845,7 @@ pub fn create_global_var_metadata(cx: &CrateContext, // crate should already contain debuginfo for it. More importantly, the // global might not even exist in un-inlined form anywhere which would lead // to a linker errors. - if cx.tcx().map.is_inlined(node_id) { + if cx.tcx().map.is_inlined_node_id(node_id) { return; } diff --git a/src/test/incremental/inlined_hir_34991/main.rs b/src/test/incremental/inlined_hir_34991/main.rs new file mode 100644 index 00000000000..a150a8c4df7 --- /dev/null +++ b/src/test/incremental/inlined_hir_34991/main.rs @@ -0,0 +1,33 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Regression test for #34991: an ICE occurred here because we inline +// some of the vector routines and give them a local def-id `X`. This +// got hashed after trans (`Hir(X)`). When we load back up, we get an +// error because the `X` is remapped to the original def-id (in +// libstd), and we can't hash a HIR node from std. + +// revisions:rpass1 rpass2 + +#![feature(rustc_attrs)] + +use std::vec::Vec; + +pub fn foo() -> Vec { + vec![1, 2, 3] +} + +pub fn bar() { + foo(); +} + +pub fn main() { + bar(); +}