Skip to content

Commit c56eb4b

Browse files
committed
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.
1 parent 9294f8e commit c56eb4b

File tree

9 files changed

+124
-32
lines changed

9 files changed

+124
-32
lines changed

src/librustc/hir/map/mod.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,9 +204,21 @@ pub struct Map<'ast> {
204204
/// All NodeIds that are numerically greater or equal to this value come
205205
/// from inlined items.
206206
local_node_id_watermark: NodeId,
207+
208+
/// All def-indices that are numerically greater or equal to this value come
209+
/// from inlined items.
210+
local_def_id_watermark: usize,
207211
}
208212

209213
impl<'ast> Map<'ast> {
214+
pub fn is_inlined_def_id(&self, id: DefId) -> bool {
215+
id.is_local() && id.index.as_usize() >= self.local_def_id_watermark
216+
}
217+
218+
pub fn is_inlined_node_id(&self, id: NodeId) -> bool {
219+
id >= self.local_node_id_watermark
220+
}
221+
210222
/// Registers a read in the dependency graph of the AST node with
211223
/// the given `id`. This needs to be called each time a public
212224
/// function returns the HIR for a node -- in other words, when it
@@ -664,10 +676,6 @@ impl<'ast> Map<'ast> {
664676
pub fn node_to_user_string(&self, id: NodeId) -> String {
665677
node_id_to_string(self, id, false)
666678
}
667-
668-
pub fn is_inlined(&self, id: NodeId) -> bool {
669-
id >= self.local_node_id_watermark
670-
}
671679
}
672680

673681
pub struct NodesMatchingSuffix<'a, 'ast:'a> {
@@ -846,13 +854,15 @@ pub fn map_crate<'ast>(forest: &'ast mut Forest,
846854
}
847855

848856
let local_node_id_watermark = map.len() as NodeId;
857+
let local_def_id_watermark = definitions.len();
849858

850859
Map {
851860
forest: forest,
852861
dep_graph: forest.dep_graph.clone(),
853862
map: RefCell::new(map),
854863
definitions: RefCell::new(definitions),
855-
local_node_id_watermark: local_node_id_watermark
864+
local_node_id_watermark: local_node_id_watermark,
865+
local_def_id_watermark: local_def_id_watermark,
856866
}
857867
}
858868

src/librustc/ty/context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
525525
}
526526

527527
pub fn retrace_path(self, path: &DefPath) -> Option<DefId> {
528-
debug!("retrace_path(path={:?})", path);
528+
debug!("retrace_path(path={:?}, krate={:?})", path, self.crate_name(path.krate));
529529

530530
let root_key = DefKey {
531531
parent: None,

src/librustc_incremental/persist/hash.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ impl<'a, 'tcx> HashContext<'a, 'tcx> {
4343
match *dep_node {
4444
// HIR nodes (which always come from our crate) are an input:
4545
DepNode::Hir(def_id) => {
46-
assert!(def_id.is_local());
4746
Some(self.hir_hash(def_id))
4847
}
4948

@@ -66,7 +65,11 @@ impl<'a, 'tcx> HashContext<'a, 'tcx> {
6665
}
6766

6867
fn hir_hash(&mut self, def_id: DefId) -> u64 {
69-
assert!(def_id.is_local());
68+
assert!(def_id.is_local(),
69+
"cannot hash HIR for non-local def-id {:?} => {:?}",
70+
def_id,
71+
self.tcx.item_path_str(def_id));
72+
7073
// FIXME(#32753) -- should we use a distinct hash here
7174
self.tcx.calculate_item_hash(def_id)
7275
}

src/librustc_incremental/persist/load.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ fn load_data(sess: &Session, path: &Path) -> Option<Vec<u8>> {
9393
None
9494
}
9595
}
96-
9796
}
9897

9998
/// 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>,
108107
let directory = try!(DefIdDirectory::decode(&mut dep_graph_decoder));
109108
let serialized_dep_graph = try!(SerializedDepGraph::decode(&mut dep_graph_decoder));
110109

111-
debug!("decode_dep_graph: directory = {:#?}", directory);
112-
debug!("decode_dep_graph: serialized_dep_graph = {:#?}", serialized_dep_graph);
113-
114110
// Retrace the paths in the directory to find their current location (if any).
115111
let retraced = directory.retrace(tcx);
116112

117-
debug!("decode_dep_graph: retraced = {:#?}", retraced);
118-
119113
// Compute the set of Hir nodes whose data has changed.
120114
let mut dirty_nodes =
121115
initial_dirty_nodes(tcx, &serialized_dep_graph.hashes, &retraced);
@@ -169,6 +163,7 @@ fn initial_dirty_nodes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
169163
let mut items_removed = false;
170164
let mut dirty_nodes = FnvHashSet();
171165
for hash in hashes {
166+
debug!("initial_dirty_nodes: retracing {:?}", hash);
172167
match hash.node.map_def(|&i| retraced.def_id(i)) {
173168
Some(dep_node) => {
174169
let current_hash = hcx.hash(&dep_node).unwrap();

src/librustc_incremental/persist/save.rs

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
// except according to those terms.
1010

1111
use rbml::opaque::Encoder;
12-
use rustc::dep_graph::DepNode;
12+
use rustc::dep_graph::{DepGraphQuery, DepNode};
13+
use rustc::hir::def_id::DefId;
1314
use rustc::middle::cstore::LOCAL_CRATE;
1415
use rustc::session::Session;
1516
use rustc::ty::TyCtxt;
17+
use rustc_data_structures::fnv::FnvHashMap;
1618
use rustc_serialize::{Encodable as RustcEncodable};
1719
use std::hash::{Hasher, SipHasher};
1820
use std::io::{self, Cursor, Write};
@@ -99,15 +101,15 @@ pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
99101
{
100102
let tcx = hcx.tcx;
101103
let query = tcx.dep_graph.query();
104+
let (nodes, edges) = post_process_graph(hcx, query);
102105

103106
let mut builder = DefIdDirectoryBuilder::new(tcx);
104107

105108
// Create hashes for inputs.
106109
let hashes =
107-
query.nodes()
108-
.into_iter()
110+
nodes.iter()
109111
.filter_map(|dep_node| {
110-
hcx.hash(&dep_node)
112+
hcx.hash(dep_node)
111113
.map(|hash| {
112114
let node = builder.map(dep_node);
113115
SerializedHash { node: node, hash: hash }
@@ -117,16 +119,14 @@ pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
117119

118120
// Create the serialized dep-graph.
119121
let graph = SerializedDepGraph {
120-
nodes: query.nodes().into_iter()
121-
.map(|node| builder.map(node))
122-
.collect(),
123-
edges: query.edges().into_iter()
124-
.map(|(source_node, target_node)| {
125-
let source = builder.map(source_node);
126-
let target = builder.map(target_node);
127-
(source, target)
128-
})
129-
.collect(),
122+
nodes: nodes.iter().map(|node| builder.map(node)).collect(),
123+
edges: edges.iter()
124+
.map(|&(ref source_node, ref target_node)| {
125+
let source = builder.map(source_node);
126+
let target = builder.map(target_node);
127+
(source, target)
128+
})
129+
.collect(),
130130
hashes: hashes,
131131
};
132132

@@ -140,6 +140,57 @@ pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
140140
Ok(())
141141
}
142142

143+
pub fn post_process_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
144+
query: DepGraphQuery<DefId>)
145+
-> (Vec<DepNode<DefId>>, Vec<(DepNode<DefId>, DepNode<DefId>)>)
146+
{
147+
let tcx = hcx.tcx;
148+
let mut cache = FnvHashMap();
149+
150+
let mut uninline_def_id = |def_id: DefId| -> Option<DefId> {
151+
if tcx.map.is_inlined_def_id(def_id) {
152+
Some(
153+
cache.entry(def_id)
154+
.or_insert_with(|| {
155+
let def_path = tcx.def_path(def_id);
156+
debug!("post_process_graph: uninlining def-id {:?} to yield {:?}",
157+
def_id, def_path);
158+
let retraced_def_id = tcx.retrace_path(&def_path).unwrap();
159+
debug!("post_process_graph: retraced to {:?}", retraced_def_id);
160+
retraced_def_id
161+
})
162+
.clone())
163+
} else {
164+
None
165+
}
166+
};
167+
168+
let mut uninline_metadata = |node: &DepNode<DefId>| -> DepNode<DefId> {
169+
match *node {
170+
DepNode::Hir(def_id) => {
171+
match uninline_def_id(def_id) {
172+
Some(uninlined_def_id) => DepNode::MetaData(uninlined_def_id),
173+
None => DepNode::Hir(def_id)
174+
}
175+
}
176+
_ => node.clone()
177+
}
178+
};
179+
180+
let nodes = query.nodes()
181+
.into_iter()
182+
.map(|node| uninline_metadata(node))
183+
.collect();
184+
185+
let edges = query.edges()
186+
.into_iter()
187+
.map(|(from, to)| (uninline_metadata(from), uninline_metadata(to)))
188+
.collect();
189+
190+
(nodes, edges)
191+
}
192+
193+
143194
pub fn encode_metadata_hashes<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>,
144195
encoder: &mut Encoder)
145196
-> io::Result<()>

src/librustc_trans/common.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1244,7 +1244,7 @@ pub fn inlined_variant_def<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
12441244
}), ..}) => ty,
12451245
_ => ctor_ty
12461246
}.ty_adt_def().unwrap();
1247-
let variant_def_id = if ccx.tcx().map.is_inlined(inlined_vid) {
1247+
let variant_def_id = if ccx.tcx().map.is_inlined_node_id(inlined_vid) {
12481248
ccx.defid_for_inlined_node(inlined_vid).unwrap()
12491249
} else {
12501250
ccx.tcx().map.local_def_id(inlined_vid)

src/librustc_trans/consts.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,7 @@ pub fn get_static<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, def_id: DefId)
10261026
.get(TransItem::Static(id))
10271027
.expect("Local statics should always be in the SymbolMap");
10281028
// Make sure that this is never executed for something inlined.
1029-
assert!(!ccx.tcx().map.is_inlined(id));
1029+
assert!(!ccx.tcx().map.is_inlined_node_id(id));
10301030

10311031
let defined_in_current_codegen_unit = ccx.codegen_unit()
10321032
.items()

src/librustc_trans/debuginfo/metadata.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ impl<'tcx> TypeMap<'tcx> {
326326
// First, find out the 'real' def_id of the type. Items inlined from
327327
// other crates have to be mapped back to their source.
328328
let def_id = if let Some(node_id) = cx.tcx().map.as_local_node_id(def_id) {
329-
if cx.tcx().map.is_inlined(node_id) {
329+
if cx.tcx().map.is_inlined_node_id(node_id) {
330330
// The given def_id identifies the inlined copy of a
331331
// type definition, let's take the source of the copy.
332332
cx.defid_for_inlined_node(node_id).unwrap()
@@ -1845,7 +1845,7 @@ pub fn create_global_var_metadata(cx: &CrateContext,
18451845
// crate should already contain debuginfo for it. More importantly, the
18461846
// global might not even exist in un-inlined form anywhere which would lead
18471847
// to a linker errors.
1848-
if cx.tcx().map.is_inlined(node_id) {
1848+
if cx.tcx().map.is_inlined_node_id(node_id) {
18491849
return;
18501850
}
18511851

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Regression test for #34991: an ICE occurred here because we inline
12+
// some of the vector routines and give them a local def-id `X`. This
13+
// got hashed after trans (`Hir(X)`). When we load back up, we get an
14+
// error because the `X` is remapped to the original def-id (in
15+
// libstd), and we can't hash a HIR node from std.
16+
17+
// revisions:rpass1 rpass2
18+
19+
#![feature(rustc_attrs)]
20+
21+
use std::vec::Vec;
22+
23+
pub fn foo() -> Vec<i32> {
24+
vec![1, 2, 3]
25+
}
26+
27+
pub fn bar() {
28+
foo();
29+
}
30+
31+
pub fn main() {
32+
bar();
33+
}

0 commit comments

Comments
 (0)