Skip to content

Commit bd60982

Browse files
authored
Rollup merge of rust-lang#39582 - nikomatsakis:incr-comp-issue-39569, r=michaelwoerister
Handle the case where an intermediate node can't be recreated This solution grows the graph, but this is quite the corner case. r? @michaelwoerister
2 parents f00408e + 4f5fc4e commit bd60982

File tree

2 files changed

+122
-35
lines changed

2 files changed

+122
-35
lines changed

src/librustc_incremental/persist/load.rs

+84-35
Original file line numberDiff line numberDiff line change
@@ -176,46 +176,32 @@ pub fn decode_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
176176
// Recreate the edges in the graph that are still clean.
177177
let mut clean_work_products = FxHashSet();
178178
let mut dirty_work_products = FxHashSet(); // incomplete; just used to suppress debug output
179+
let mut extra_edges = vec![];
179180
for (source, targets) in &edge_map {
180181
for target in targets {
181-
// If the target is dirty, skip the edge. If this is an edge
182-
// that targets a work-product, we can print the blame
183-
// information now.
184-
if let Some(blame) = dirty_raw_nodes.get(target) {
185-
if let DepNode::WorkProduct(ref wp) = *target {
186-
if tcx.sess.opts.debugging_opts.incremental_info {
187-
if dirty_work_products.insert(wp.clone()) {
188-
// It'd be nice to pretty-print these paths better than just
189-
// using the `Debug` impls, but wev.
190-
println!("incremental: module {:?} is dirty because {:?} \
191-
changed or was removed",
192-
wp,
193-
blame.map_def(|&index| {
194-
Some(directory.def_path_string(tcx, index))
195-
}).unwrap());
196-
}
197-
}
198-
}
199-
continue;
200-
}
201-
202-
// If the source is dirty, the target will be dirty.
203-
assert!(!dirty_raw_nodes.contains_key(source));
204-
205-
// Retrace the source -> target edges to def-ids and then
206-
// create an edge in the graph. Retracing may yield none if
207-
// some of the data happens to have been removed; this ought
208-
// to be impossible unless it is dirty, so we can unwrap.
209-
let source_node = retraced.map(source).unwrap();
210-
let target_node = retraced.map(target).unwrap();
211-
let _task = tcx.dep_graph.in_task(target_node);
212-
tcx.dep_graph.read(source_node);
213-
if let DepNode::WorkProduct(ref wp) = *target {
214-
clean_work_products.insert(wp.clone());
215-
}
182+
process_edges(tcx, source, target, &edge_map, &directory, &retraced, &dirty_raw_nodes,
183+
&mut clean_work_products, &mut dirty_work_products, &mut extra_edges);
216184
}
217185
}
218186

187+
// Subtle. Sometimes we have intermediate nodes that we can't recreate in the new graph.
188+
// This is pretty unusual but it arises in a scenario like this:
189+
//
190+
// Hir(X) -> Foo(Y) -> Bar
191+
//
192+
// Note that the `Hir(Y)` is not an input to `Foo(Y)` -- this
193+
// almost never happens, but can happen in some obscure
194+
// scenarios. In that case, if `Y` is removed, then we can't
195+
// recreate `Foo(Y)` (the def-id `Y` no longer exists); what we do
196+
// then is to push the edge `Hir(X) -> Bar` onto `extra_edges`
197+
// (along with any other targets of `Foo(Y)`). We will then add
198+
// the edge from `Hir(X)` to `Bar` (or, if `Bar` itself cannot be
199+
// recreated, to the targets of `Bar`).
200+
while let Some((source, target)) = extra_edges.pop() {
201+
process_edges(tcx, source, target, &edge_map, &directory, &retraced, &dirty_raw_nodes,
202+
&mut clean_work_products, &mut dirty_work_products, &mut extra_edges);
203+
}
204+
219205
// Add in work-products that are still clean, and delete those that are
220206
// dirty.
221207
reconcile_work_products(tcx, work_products, &clean_work_products);
@@ -393,3 +379,66 @@ fn load_prev_metadata_hashes(tcx: TyCtxt,
393379
serialized_hashes.index_map.len());
394380
}
395381

382+
fn process_edges<'a, 'tcx, 'edges>(
383+
tcx: TyCtxt<'a, 'tcx, 'tcx>,
384+
source: &'edges DepNode<DefPathIndex>,
385+
target: &'edges DepNode<DefPathIndex>,
386+
edges: &'edges FxHashMap<DepNode<DefPathIndex>, Vec<DepNode<DefPathIndex>>>,
387+
directory: &DefIdDirectory,
388+
retraced: &RetracedDefIdDirectory,
389+
dirty_raw_nodes: &DirtyNodes,
390+
clean_work_products: &mut FxHashSet<Arc<WorkProductId>>,
391+
dirty_work_products: &mut FxHashSet<Arc<WorkProductId>>,
392+
extra_edges: &mut Vec<(&'edges DepNode<DefPathIndex>, &'edges DepNode<DefPathIndex>)>)
393+
{
394+
// If the target is dirty, skip the edge. If this is an edge
395+
// that targets a work-product, we can print the blame
396+
// information now.
397+
if let Some(blame) = dirty_raw_nodes.get(target) {
398+
if let DepNode::WorkProduct(ref wp) = *target {
399+
if tcx.sess.opts.debugging_opts.incremental_info {
400+
if dirty_work_products.insert(wp.clone()) {
401+
// It'd be nice to pretty-print these paths better than just
402+
// using the `Debug` impls, but wev.
403+
println!("incremental: module {:?} is dirty because {:?} \
404+
changed or was removed",
405+
wp,
406+
blame.map_def(|&index| {
407+
Some(directory.def_path_string(tcx, index))
408+
}).unwrap());
409+
}
410+
}
411+
}
412+
return;
413+
}
414+
415+
// If the source is dirty, the target will be dirty.
416+
assert!(!dirty_raw_nodes.contains_key(source));
417+
418+
// Retrace the source -> target edges to def-ids and then create
419+
// an edge in the graph. Retracing may yield none if some of the
420+
// data happens to have been removed.
421+
if let Some(source_node) = retraced.map(source) {
422+
if let Some(target_node) = retraced.map(target) {
423+
let _task = tcx.dep_graph.in_task(target_node);
424+
tcx.dep_graph.read(source_node);
425+
if let DepNode::WorkProduct(ref wp) = *target {
426+
clean_work_products.insert(wp.clone());
427+
}
428+
} else {
429+
// As discussed in `decode_dep_graph` above, sometimes the
430+
// target cannot be recreated again, in which case we add
431+
// edges to go from `source` to the targets of `target`.
432+
extra_edges.extend(
433+
edges[target].iter().map(|t| (source, t)));
434+
}
435+
} else {
436+
// It's also possible that the source can't be created! But we
437+
// can ignore such cases, because (a) if `source` is a HIR
438+
// node, it would be considered dirty; and (b) in other cases,
439+
// there must be some input to this node that is clean, and so
440+
// we'll re-create the edges over in the case where target is
441+
// undefined.
442+
}
443+
}
444+

src/test/incremental/issue-39569.rs

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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 a weird corner case in our dep-graph reduction
12+
// code. When we solve `CoerceUnsized<Foo>`, we find no impls, so we
13+
// don't end up with an edge to any HIR nodes, but it still gets
14+
// preserved in the dep graph.
15+
16+
// revisions:rpass1 rpass2
17+
// compile-flags: -Z query-dep-graph
18+
19+
use std::sync::Arc;
20+
21+
#[cfg(rpass1)]
22+
struct Foo { x: usize }
23+
24+
#[cfg(rpass1)]
25+
fn main() {
26+
let x: Arc<Foo> = Arc::new(Foo { x: 22 });
27+
let y: Arc<Foo> = x;
28+
}
29+
30+
#[cfg(rpass2)]
31+
struct FooX { x: usize }
32+
33+
#[cfg(rpass2)]
34+
fn main() {
35+
let x: Arc<FooX> = Arc::new(FooX { x: 22 });
36+
let y: Arc<FooX> = x;
37+
}
38+

0 commit comments

Comments
 (0)