From be1819f14bfa67e95b711f1e093816f4d29c94de Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 24 May 2016 15:50:24 -0400 Subject: [PATCH 1/6] add a series of tests for changes to structs These tests reveal that the edges are in some cases too strict. --- src/test/incremental/struct_add_field.rs | 48 ++++++++++++++++ .../incremental/struct_change_field_name.rs | 55 +++++++++++++++++++ .../incremental/struct_change_field_type.rs | 53 ++++++++++++++++++ .../auxiliary/a.rs | 29 ++++++++++ .../struct_change_field_type_cross_crate/b.rs | 36 ++++++++++++ src/test/incremental/struct_change_nothing.rs | 53 ++++++++++++++++++ src/test/incremental/struct_remove_field.rs | 52 ++++++++++++++++++ 7 files changed, 326 insertions(+) create mode 100644 src/test/incremental/struct_add_field.rs create mode 100644 src/test/incremental/struct_change_field_name.rs create mode 100644 src/test/incremental/struct_change_field_type.rs create mode 100644 src/test/incremental/struct_change_field_type_cross_crate/auxiliary/a.rs create mode 100644 src/test/incremental/struct_change_field_type_cross_crate/b.rs create mode 100644 src/test/incremental/struct_change_nothing.rs create mode 100644 src/test/incremental/struct_remove_field.rs diff --git a/src/test/incremental/struct_add_field.rs b/src/test/incremental/struct_add_field.rs new file mode 100644 index 0000000000000..425aa3e07a0ec --- /dev/null +++ b/src/test/incremental/struct_add_field.rs @@ -0,0 +1,48 @@ +// 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. + +// Test incremental compilation tracking where we change field names +// in between revisions (hashing should be stable). + +// revisions:rpass1 rpass2 + +#![feature(rustc_attrs)] + +pub struct X { + pub x: u32, + + #[cfg(rpass2)] + pub x2: u32, +} + +pub struct EmbedX { + x: X +} + +pub struct Y { + pub y: char +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +pub fn use_X(x: X) -> u32 { + x.x as u32 +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +pub fn use_EmbedX(embed: EmbedX) -> u32 { + embed.x.x as u32 +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] // FIXME(#33850) should be clean +pub fn use_Y() { + let x: Y = Y { y: 'c' }; +} + +pub fn main() { } diff --git a/src/test/incremental/struct_change_field_name.rs b/src/test/incremental/struct_change_field_name.rs new file mode 100644 index 0000000000000..cf6f89bd8a4c5 --- /dev/null +++ b/src/test/incremental/struct_change_field_name.rs @@ -0,0 +1,55 @@ +// 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. + +// Test incremental compilation tracking where we change field names +// in between revisions (hashing should be stable). + +// revisions:rpass1 cfail2 + +#![feature(rustc_attrs)] + +#[cfg(rpass1)] +pub struct X { + pub x: u32 +} + +#[cfg(cfail2)] +pub struct X { + pub y: u32 +} + +pub struct EmbedX { + x: X +} + +pub struct Y { + pub y: char +} + +#[rustc_dirty(label="TypeckItemBody", cfg="cfail2")] +pub fn use_X() -> u32 { + let x: X = X { x: 22 }; + //[cfail2]~^ ERROR structure `X` has no field named `x` + x.x as u32 + //[cfail2]~^ ERROR attempted access of field `x` +} + +#[rustc_dirty(label="TypeckItemBody", cfg="cfail2")] +pub fn use_EmbedX(embed: EmbedX) -> u32 { + embed.x.x as u32 + //[cfail2]~^ ERROR attempted access of field `x` +} + +#[rustc_dirty(label="TypeckItemBody", cfg="cfail2")] // FIXME(#33850) should be clean +pub fn use_Y() { + let x: Y = Y { y: 'c' }; +} + +pub fn main() { } diff --git a/src/test/incremental/struct_change_field_type.rs b/src/test/incremental/struct_change_field_type.rs new file mode 100644 index 0000000000000..2bd61a3f559d4 --- /dev/null +++ b/src/test/incremental/struct_change_field_type.rs @@ -0,0 +1,53 @@ +// 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. + +// Test incremental compilation tracking where we change nothing +// in between revisions (hashing should be stable). + +// revisions:rpass1 rpass2 + +#![feature(rustc_attrs)] + +#[cfg(rpass1)] +pub struct X { + pub x: u32 +} + +#[cfg(rpass2)] +pub struct X { + pub x: i32 +} + +pub struct EmbedX { + x: X +} + +pub struct Y { + pub y: char +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +pub fn use_X() -> u32 { + let x: X = X { x: 22 }; + x.x as u32 +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +pub fn use_EmbedX(x: EmbedX) -> u32 { + let x: X = X { x: 22 }; + x.x as u32 +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] // FIXME(#33850) should be clean +pub fn use_Y() { + let x: Y = Y { y: 'c' }; +} + +pub fn main() { } diff --git a/src/test/incremental/struct_change_field_type_cross_crate/auxiliary/a.rs b/src/test/incremental/struct_change_field_type_cross_crate/auxiliary/a.rs new file mode 100644 index 0000000000000..2ddcaf157210d --- /dev/null +++ b/src/test/incremental/struct_change_field_type_cross_crate/auxiliary/a.rs @@ -0,0 +1,29 @@ +// 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. + +#![crate_type="rlib"] + + #[cfg(rpass1)] +pub struct X { + pub x: u32 +} + +#[cfg(rpass2)] +pub struct X { + pub x: i32 +} + +pub struct EmbedX { + pub x: X +} + +pub struct Y { + pub y: char +} diff --git a/src/test/incremental/struct_change_field_type_cross_crate/b.rs b/src/test/incremental/struct_change_field_type_cross_crate/b.rs new file mode 100644 index 0000000000000..19f146ce176d7 --- /dev/null +++ b/src/test/incremental/struct_change_field_type_cross_crate/b.rs @@ -0,0 +1,36 @@ +// 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. + +// aux-build:a.rs +// revisions:rpass1 rpass2 + +#![feature(rustc_attrs)] + +extern crate a; + +use a::*; + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +pub fn use_X() -> u32 { + let x: X = X { x: 22 }; + x.x as u32 +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +pub fn use_EmbedX(embed: EmbedX) -> u32 { + embed.x.x as u32 +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] // FIXME(#33850) should be clean +pub fn use_Y() { + let x: Y = Y { y: 'c' }; +} + +pub fn main() { } diff --git a/src/test/incremental/struct_change_nothing.rs b/src/test/incremental/struct_change_nothing.rs new file mode 100644 index 0000000000000..8095e1ecd84a0 --- /dev/null +++ b/src/test/incremental/struct_change_nothing.rs @@ -0,0 +1,53 @@ +// 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. + +// Test incremental compilation tracking where we change nothing +// in between revisions (hashing should be stable). + +// revisions:rpass1 rpass2 + +#![feature(rustc_attrs)] + +#[cfg(rpass1)] +pub struct X { + pub x: u32 +} + +#[cfg(rpass2)] +pub struct X { + pub x: u32 +} + +pub struct EmbedX { + x: X +} + +pub struct Y { + pub y: char +} + +#[rustc_clean(label="TypeckItemBody", cfg="rpass2")] +pub fn use_X() -> u32 { + let x: X = X { x: 22 }; + x.x as u32 +} + +#[rustc_clean(label="TypeckItemBody", cfg="rpass2")] +pub fn use_EmbedX(x: EmbedX) -> u32 { + let x: X = X { x: 22 }; + x.x as u32 +} + +#[rustc_clean(label="TypeckItemBody", cfg="rpass2")] +pub fn use_Y() { + let x: Y = Y { y: 'c' }; +} + +pub fn main() { } diff --git a/src/test/incremental/struct_remove_field.rs b/src/test/incremental/struct_remove_field.rs new file mode 100644 index 0000000000000..db49a7516275d --- /dev/null +++ b/src/test/incremental/struct_remove_field.rs @@ -0,0 +1,52 @@ +// 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. + +// Test incremental compilation tracking where we change field names +// in between revisions (hashing should be stable). + +// revisions:rpass1 rpass2 + +#![feature(rustc_attrs)] + +#[cfg(rpass1)] +pub struct X { + pub x: u32, + pub x2: u32, +} + +#[cfg(rpass2)] +pub struct X { + pub x: u32, +} + +pub struct EmbedX { + x: X +} + +pub struct Y { + pub y: char +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +pub fn use_X(x: X) -> u32 { + x.x as u32 +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +pub fn use_EmbedX(embed: EmbedX) -> u32 { + embed.x.x as u32 +} + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] // FIXME(#33850) should be clean +pub fn use_Y() { + let x: Y = Y { y: 'c' }; +} + +pub fn main() { } From 5dc6a058b2ec05a18447308b224669c4a9726f3c Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 24 May 2016 15:59:07 -0400 Subject: [PATCH 2/6] extend type alias test to include a clean rev --- src/test/incremental/type_alias_cross_crate/auxiliary/a.rs | 4 ++++ src/test/incremental/type_alias_cross_crate/b.rs | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/incremental/type_alias_cross_crate/auxiliary/a.rs b/src/test/incremental/type_alias_cross_crate/auxiliary/a.rs index 446d97e5bc054..e1dba1317703d 100644 --- a/src/test/incremental/type_alias_cross_crate/auxiliary/a.rs +++ b/src/test/incremental/type_alias_cross_crate/auxiliary/a.rs @@ -16,4 +16,8 @@ pub type X = u32; #[cfg(rpass2)] pub type X = i32; +// this version doesn't actually change anything: +#[cfg(rpass3)] +pub type X = i32; + pub type Y = char; diff --git a/src/test/incremental/type_alias_cross_crate/b.rs b/src/test/incremental/type_alias_cross_crate/b.rs index b4e9b7601010a..c5421fcbf5cb2 100644 --- a/src/test/incremental/type_alias_cross_crate/b.rs +++ b/src/test/incremental/type_alias_cross_crate/b.rs @@ -9,19 +9,21 @@ // except according to those terms. // aux-build:a.rs -// revisions:rpass1 rpass2 +// revisions:rpass1 rpass2 rpass3 #![feature(rustc_attrs)] extern crate a; #[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +#[rustc_clean(label="TypeckItemBody", cfg="rpass3")] pub fn use_X() -> u32 { let x: a::X = 22; x as u32 } #[rustc_clean(label="TypeckItemBody", cfg="rpass2")] +#[rustc_clean(label="TypeckItemBody", cfg="rpass3")] pub fn use_Y() { let x: a::Y = 'c'; } From 63bb0847bdf58d7d021735184a9dfd48138ad8ab Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 26 May 2016 06:11:16 -0400 Subject: [PATCH 3/6] expand `DepNode::TraitSelect` to include type ids To handle the general case, we include a vector of def-ids, so that we can account for things like `(Foo, Bar)` which references both `Foo` and `Bar`. This means it is not Copy, so re-jigger some APIs to use borrowing more intelligently. --- src/librustc/dep_graph/dep_node.rs | 19 ++++- src/librustc/dep_graph/query.rs | 24 +++--- src/librustc/dep_graph/raii.rs | 4 +- src/librustc/dep_graph/visit.rs | 2 +- src/librustc/ty/mod.rs | 13 ++- src/librustc_incremental/assert_dep_graph.rs | 80 +++++++++---------- src/librustc_incremental/persist/directory.rs | 4 +- src/librustc_incremental/persist/hash.rs | 4 +- src/librustc_incremental/persist/load.rs | 12 +-- src/librustc_incremental/persist/save.rs | 8 +- 10 files changed, 91 insertions(+), 79 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index 84c84a7ed57a2..73b96651b05e2 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -10,7 +10,16 @@ use std::fmt::Debug; -#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)] +macro_rules! try_opt { + ($e:expr) => ( + match $e { + Some(r) => r, + None => return None, + } + ) +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)] pub enum DepNode { // The `D` type is "how definitions are identified". // During compilation, it is always `DefId`, but when serializing @@ -116,7 +125,7 @@ pub enum DepNode { // which would yield an overly conservative dep-graph. TraitItems(D), ReprHints(D), - TraitSelect(D), + TraitSelect(D, Vec), } impl DepNode { @@ -212,7 +221,11 @@ impl DepNode { TraitImpls(ref d) => op(d).map(TraitImpls), TraitItems(ref d) => op(d).map(TraitItems), ReprHints(ref d) => op(d).map(ReprHints), - TraitSelect(ref d) => op(d).map(TraitSelect), + TraitSelect(ref d, ref type_ds) => { + let d = try_opt!(op(d)); + let type_ds = try_opt!(type_ds.iter().map(|d| op(d)).collect()); + Some(TraitSelect(d, type_ds)) + } } } } diff --git a/src/librustc/dep_graph/query.rs b/src/librustc/dep_graph/query.rs index 93248edb197c6..7a780c1d4ae24 100644 --- a/src/librustc/dep_graph/query.rs +++ b/src/librustc/dep_graph/query.rs @@ -47,26 +47,26 @@ impl DepGraphQuery { self.indices.contains_key(&node) } - pub fn nodes(&self) -> Vec> { + pub fn nodes(&self) -> Vec<&DepNode> { self.graph.all_nodes() .iter() - .map(|n| n.data.clone()) + .map(|n| &n.data) .collect() } - pub fn edges(&self) -> Vec<(DepNode,DepNode)> { + pub fn edges(&self) -> Vec<(&DepNode,&DepNode)> { self.graph.all_edges() .iter() .map(|edge| (edge.source(), edge.target())) - .map(|(s, t)| (self.graph.node_data(s).clone(), - self.graph.node_data(t).clone())) + .map(|(s, t)| (self.graph.node_data(s), + self.graph.node_data(t))) .collect() } - fn reachable_nodes(&self, node: DepNode, direction: Direction) -> Vec> { - if let Some(&index) = self.indices.get(&node) { + fn reachable_nodes(&self, node: &DepNode, direction: Direction) -> Vec<&DepNode> { + if let Some(&index) = self.indices.get(node) { self.graph.depth_traverse(index, direction) - .map(|s| self.graph.node_data(s).clone()) + .map(|s| self.graph.node_data(s)) .collect() } else { vec![] @@ -75,20 +75,20 @@ impl DepGraphQuery { /// All nodes reachable from `node`. In other words, things that /// will have to be recomputed if `node` changes. - pub fn transitive_successors(&self, node: DepNode) -> Vec> { + pub fn transitive_successors(&self, node: &DepNode) -> Vec<&DepNode> { self.reachable_nodes(node, OUTGOING) } /// All nodes that can reach `node`. - pub fn transitive_predecessors(&self, node: DepNode) -> Vec> { + pub fn transitive_predecessors(&self, node: &DepNode) -> Vec<&DepNode> { self.reachable_nodes(node, INCOMING) } /// Just the outgoing edges from `node`. - pub fn immediate_successors(&self, node: DepNode) -> Vec> { + pub fn immediate_successors(&self, node: &DepNode) -> Vec<&DepNode> { if let Some(&index) = self.indices.get(&node) { self.graph.successor_nodes(index) - .map(|s| self.graph.node_data(s).clone()) + .map(|s| self.graph.node_data(s)) .collect() } else { vec![] diff --git a/src/librustc/dep_graph/raii.rs b/src/librustc/dep_graph/raii.rs index 13151d169fc3f..92cecabaa182f 100644 --- a/src/librustc/dep_graph/raii.rs +++ b/src/librustc/dep_graph/raii.rs @@ -20,14 +20,14 @@ pub struct DepTask<'graph> { impl<'graph> DepTask<'graph> { pub fn new(data: &'graph DepGraphThreadData, key: DepNode) -> DepTask<'graph> { - data.enqueue(DepMessage::PushTask(key)); + data.enqueue(DepMessage::PushTask(key.clone())); DepTask { data: data, key: key } } } impl<'graph> Drop for DepTask<'graph> { fn drop(&mut self) { - self.data.enqueue(DepMessage::PopTask(self.key)); + self.data.enqueue(DepMessage::PopTask(self.key.clone())); } } diff --git a/src/librustc/dep_graph/visit.rs b/src/librustc/dep_graph/visit.rs index 9133b4d22eeb2..5dd71db2f1832 100644 --- a/src/librustc/dep_graph/visit.rs +++ b/src/librustc/dep_graph/visit.rs @@ -39,7 +39,7 @@ pub fn visit_all_items_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>, fn visit_item(&mut self, i: &'tcx hir::Item) { let item_def_id = self.tcx.map.local_def_id(i.id); let task_id = (self.dep_node_fn)(item_def_id); - let _task = self.tcx.dep_graph.in_task(task_id); + let _task = self.tcx.dep_graph.in_task(task_id.clone()); debug!("Started task {:?}", task_id); self.tcx.dep_graph.read(DepNode::Hir(item_def_id)); self.visitor.visit_item(i); diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 24f0671ce6184..423d57cfb328d 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -946,7 +946,7 @@ impl<'tcx> TraitPredicate<'tcx> { /// Creates the dep-node for selecting/evaluating this trait reference. fn dep_node(&self) -> DepNode { - DepNode::TraitSelect(self.def_id()) + DepNode::TraitSelect(self.def_id(), vec![]) } pub fn input_types(&self) -> &[Ty<'tcx>] { @@ -1768,9 +1768,8 @@ impl<'a, 'tcx> AdtDefData<'tcx, 'tcx> { stack: &mut Vec>) { - let dep_node = DepNode::SizedConstraint(self.did); - - if self.sized_constraint.get(dep_node).is_some() { + let dep_node = || DepNode::SizedConstraint(self.did); + if self.sized_constraint.get(dep_node()).is_some() { return; } @@ -1780,7 +1779,7 @@ impl<'a, 'tcx> AdtDefData<'tcx, 'tcx> { // // Consider the type as Sized in the meanwhile to avoid // further errors. - self.sized_constraint.fulfill(dep_node, tcx.types.err); + self.sized_constraint.fulfill(dep_node(), tcx.types.err); return; } @@ -1803,14 +1802,14 @@ impl<'a, 'tcx> AdtDefData<'tcx, 'tcx> { _ => tcx.mk_tup(tys) }; - match self.sized_constraint.get(dep_node) { + match self.sized_constraint.get(dep_node()) { Some(old_ty) => { debug!("calculate_sized_constraint: {:?} recurred", self); assert_eq!(old_ty, tcx.types.err) } None => { debug!("calculate_sized_constraint: {:?} => {:?}", self, ty); - self.sized_constraint.fulfill(dep_node, ty) + self.sized_constraint.fulfill(dep_node(), ty) } } } diff --git a/src/librustc_incremental/assert_dep_graph.rs b/src/librustc_incremental/assert_dep_graph.rs index 9dc50a6306406..1c0274cdcca9a 100644 --- a/src/librustc_incremental/assert_dep_graph.rs +++ b/src/librustc_incremental/assert_dep_graph.rs @@ -195,7 +195,7 @@ fn check_paths<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } }; - for &(_, source_def_id, source_dep_node) in sources { + for &(_, source_def_id, ref source_dep_node) in sources { let dependents = query.transitive_successors(source_dep_node); for &(target_span, ref target_pass, _, ref target_dep_node) in targets { if !dependents.contains(&target_dep_node) { @@ -239,7 +239,7 @@ fn dump_graph(tcx: TyCtxt) { { // dump a .txt file with just the edges: let txt_path = format!("{}.txt", path); let mut file = File::create(&txt_path).unwrap(); - for &(source, target) in &edges { + for &(ref source, ref target) in &edges { write!(file, "{:?} -> {:?}\n", source, target).unwrap(); } } @@ -252,34 +252,34 @@ fn dump_graph(tcx: TyCtxt) { } } -pub struct GraphvizDepGraph(FnvHashSet>, - Vec<(DepNode, DepNode)>); +pub struct GraphvizDepGraph<'q>(FnvHashSet<&'q DepNode>, + Vec<(&'q DepNode, &'q DepNode)>); -impl<'a, 'tcx> dot::GraphWalk<'a> for GraphvizDepGraph { - type Node = DepNode; - type Edge = (DepNode, DepNode); - fn nodes(&self) -> dot::Nodes> { +impl<'a, 'tcx, 'q> dot::GraphWalk<'a> for GraphvizDepGraph<'q> { + type Node = &'q DepNode; + type Edge = (&'q DepNode, &'q DepNode); + fn nodes(&self) -> dot::Nodes<&'q DepNode> { let nodes: Vec<_> = self.0.iter().cloned().collect(); nodes.into_cow() } - fn edges(&self) -> dot::Edges<(DepNode, DepNode)> { + fn edges(&self) -> dot::Edges<(&'q DepNode, &'q DepNode)> { self.1[..].into_cow() } - fn source(&self, edge: &(DepNode, DepNode)) -> DepNode { + fn source(&self, edge: &(&'q DepNode, &'q DepNode)) -> &'q DepNode { edge.0 } - fn target(&self, edge: &(DepNode, DepNode)) -> DepNode { + fn target(&self, edge: &(&'q DepNode, &'q DepNode)) -> &'q DepNode { edge.1 } } -impl<'a, 'tcx> dot::Labeller<'a> for GraphvizDepGraph { - type Node = DepNode; - type Edge = (DepNode, DepNode); +impl<'a, 'tcx, 'q> dot::Labeller<'a> for GraphvizDepGraph<'q> { + type Node = &'q DepNode; + type Edge = (&'q DepNode, &'q DepNode); fn graph_id(&self) -> dot::Id { dot::Id::new("DependencyGraph").unwrap() } - fn node_id(&self, n: &DepNode) -> dot::Id { + fn node_id(&self, n: &&'q DepNode) -> dot::Id { let s: String = format!("{:?}", n).chars() .map(|c| if c == '_' || c.is_alphanumeric() { c } else { '_' }) @@ -287,7 +287,7 @@ impl<'a, 'tcx> dot::Labeller<'a> for GraphvizDepGraph { debug!("n={:?} s={:?}", n, s); dot::Id::new(s).unwrap() } - fn node_label(&self, n: &DepNode) -> dot::LabelText { + fn node_label(&self, n: &&'q DepNode) -> dot::LabelText { dot::LabelText::label(format!("{:?}", n)) } } @@ -295,8 +295,8 @@ impl<'a, 'tcx> dot::Labeller<'a> for GraphvizDepGraph { // Given an optional filter like `"x,y,z"`, returns either `None` (no // filter) or the set of nodes whose labels contain all of those // substrings. -fn node_set(query: &DepGraphQuery, filter: &DepNodeFilter) - -> Option>> +fn node_set<'q>(query: &'q DepGraphQuery, filter: &DepNodeFilter) + -> Option>> { debug!("node_set(filter={:?})", filter); @@ -307,10 +307,10 @@ fn node_set(query: &DepGraphQuery, filter: &DepNodeFilter) Some(query.nodes().into_iter().filter(|n| filter.test(n)).collect()) } -fn filter_nodes(query: &DepGraphQuery, - sources: &Option>>, - targets: &Option>>) - -> FnvHashSet> +fn filter_nodes<'q>(query: &'q DepGraphQuery, + sources: &Option>>, + targets: &Option>>) + -> FnvHashSet<&'q DepNode> { if let &Some(ref sources) = sources { if let &Some(ref targets) = targets { @@ -325,21 +325,21 @@ fn filter_nodes(query: &DepGraphQuery, } } -fn walk_nodes(query: &DepGraphQuery, - starts: &FnvHashSet>, - direction: Direction) - -> FnvHashSet> +fn walk_nodes<'q>(query: &'q DepGraphQuery, + starts: &FnvHashSet<&'q DepNode>, + direction: Direction) + -> FnvHashSet<&'q DepNode> { let mut set = FnvHashSet(); - for start in starts { + for &start in starts { debug!("walk_nodes: start={:?} outgoing?={:?}", start, direction == OUTGOING); - if set.insert(*start) { + if set.insert(start) { let mut stack = vec![query.indices[start]]; while let Some(index) = stack.pop() { for (_, edge) in query.graph.adjacent_edges(index, direction) { let neighbor_index = edge.source_or_target(direction); let neighbor = query.graph.node_data(neighbor_index); - if set.insert(*neighbor) { + if set.insert(neighbor) { stack.push(neighbor_index); } } @@ -349,10 +349,10 @@ fn walk_nodes(query: &DepGraphQuery, set } -fn walk_between(query: &DepGraphQuery, - sources: &FnvHashSet>, - targets: &FnvHashSet>) - -> FnvHashSet> +fn walk_between<'q>(query: &'q DepGraphQuery, + sources: &FnvHashSet<&'q DepNode>, + targets: &FnvHashSet<&'q DepNode>) + -> FnvHashSet<&'q DepNode> { // This is a bit tricky. We want to include a node only if it is: // (a) reachable from a source and (b) will reach a target. And we @@ -365,16 +365,16 @@ fn walk_between(query: &DepGraphQuery, let mut node_states = vec![State::Undecided; query.graph.len_nodes()]; for &target in targets { - node_states[query.indices[&target].0] = State::Included; + node_states[query.indices[target].0] = State::Included; } - for source in sources.iter().map(|n| query.indices[n]) { + for source in sources.iter().map(|&n| query.indices[n]) { recurse(query, &mut node_states, source); } return query.nodes() .into_iter() - .filter(|n| { + .filter(|&n| { let index = query.indices[n]; node_states[index.0] == State::Included }) @@ -417,12 +417,12 @@ fn walk_between(query: &DepGraphQuery, } } -fn filter_edges(query: &DepGraphQuery, - nodes: &FnvHashSet>) - -> Vec<(DepNode, DepNode)> +fn filter_edges<'q>(query: &'q DepGraphQuery, + nodes: &FnvHashSet<&'q DepNode>) + -> Vec<(&'q DepNode, &'q DepNode)> { query.edges() .into_iter() - .filter(|&(source, target)| nodes.contains(&source) && nodes.contains(&target)) + .filter(|&(source, target)| nodes.contains(source) && nodes.contains(target)) .collect() } diff --git a/src/librustc_incremental/persist/directory.rs b/src/librustc_incremental/persist/directory.rs index f9e90f393219d..2fd6973909a8e 100644 --- a/src/librustc_incremental/persist/directory.rs +++ b/src/librustc_incremental/persist/directory.rs @@ -57,7 +57,7 @@ impl RetracedDefIdDirectory { self.ids[index.index as usize] } - pub fn map(&self, node: DepNode) -> Option> { + pub fn map(&self, node: &DepNode) -> Option> { node.map_def(|&index| self.def_id(index)) } } @@ -91,7 +91,7 @@ impl<'a,'tcx> DefIdDirectoryBuilder<'a,'tcx> { .clone() } - pub fn map(&mut self, node: DepNode) -> DepNode { + pub fn map(&mut self, node: &DepNode) -> DepNode { node.map_def(|&def_id| Some(self.add(def_id))).unwrap() } diff --git a/src/librustc_incremental/persist/hash.rs b/src/librustc_incremental/persist/hash.rs index b729f25b873d4..99119dd184c8b 100644 --- a/src/librustc_incremental/persist/hash.rs +++ b/src/librustc_incremental/persist/hash.rs @@ -39,8 +39,8 @@ impl<'a, 'tcx> HashContext<'a, 'tcx> { } } - pub fn hash(&mut self, dep_node: DepNode) -> Option { - match dep_node { + 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()); diff --git a/src/librustc_incremental/persist/load.rs b/src/librustc_incremental/persist/load.rs index e3fd290443c11..0ac1018462ee7 100644 --- a/src/librustc_incremental/persist/load.rs +++ b/src/librustc_incremental/persist/load.rs @@ -114,15 +114,15 @@ pub fn decode_dep_graph<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let clean_nodes = serialized_dep_graph.nodes .iter() - .filter_map(|&node| retraced.map(node)) + .filter_map(|node| retraced.map(node)) .filter(|node| !dirty_nodes.contains(node)) - .map(|node| (node, node)); + .map(|node| (node.clone(), node)); // Add nodes and edges that are not dirty into our main graph. let dep_graph = tcx.dep_graph.clone(); for (source, target) in clean_edges.into_iter().chain(clean_nodes) { - let _task = dep_graph.in_task(target); - dep_graph.read(source); + let _task = dep_graph.in_task(target.clone()); + dep_graph.read(source.clone()); debug!("decode_dep_graph: clean edge: {:?} -> {:?}", source, target); } @@ -140,7 +140,7 @@ fn initial_dirty_nodes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, for hash in hashes { match hash.node.map_def(|&i| retraced.def_id(i)) { Some(dep_node) => { - let current_hash = hcx.hash(dep_node).unwrap(); + let current_hash = hcx.hash(&dep_node).unwrap(); debug!("initial_dirty_nodes: hash of {:?} is {:?}, was {:?}", dep_node, current_hash, hash.hash); if current_hash != hash.hash { @@ -171,7 +171,7 @@ fn compute_clean_edges(serialized_edges: &[(SerializedEdge)], // target) if neither node has been removed. If the source has // been removed, add target to the list of dirty nodes. let mut clean_edges = Vec::with_capacity(serialized_edges.len()); - for &(serialized_source, serialized_target) in serialized_edges { + for &(ref serialized_source, ref serialized_target) in serialized_edges { if let Some(target) = retraced.map(serialized_target) { if let Some(source) = retraced.map(serialized_source) { clean_edges.push((source, target)) diff --git a/src/librustc_incremental/persist/save.rs b/src/librustc_incremental/persist/save.rs index 7deb1ca36dbde..99f4d4f307298 100644 --- a/src/librustc_incremental/persist/save.rs +++ b/src/librustc_incremental/persist/save.rs @@ -99,7 +99,7 @@ pub fn encode_dep_graph<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>, query.nodes() .into_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 } @@ -147,7 +147,7 @@ pub fn encode_metadata_hashes<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>, let meta_data_def_ids = query.nodes() .into_iter() - .filter_map(|dep_node| match dep_node { + .filter_map(|dep_node| match *dep_node { DepNode::MetaData(def_id) if def_id.is_local() => Some(def_id), _ => None, }); @@ -165,8 +165,8 @@ pub fn encode_metadata_hashes<'a, 'tcx>(hcx: &mut HashContext<'a, 'tcx>, let dep_node = DepNode::MetaData(def_id); let mut state = SipHasher::new(); debug!("save: computing metadata hash for {:?}", dep_node); - for node in query.transitive_predecessors(dep_node) { - if let Some(hash) = hcx.hash(node) { + for node in query.transitive_predecessors(&dep_node) { + if let Some(hash) = hcx.hash(&node) { debug!("save: predecessor {:?} has hash {}", node, hash); state.write_u64(hash.to_le()); } else { From 4038b5b3eee3d26e07ec49b2e697975307488d67 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 27 May 2016 21:19:11 -0400 Subject: [PATCH 4/6] add def-ids from nominal types into TraitSelect This way we distinguish, in particular, `Foo: Sized` and `Bar: Sized`, which fixes #33850. --- src/librustc/ty/mod.rs | 23 ++++++++++++++++++- src/test/incremental/struct_add_field.rs | 2 +- .../incremental/struct_change_field_name.rs | 2 +- .../incremental/struct_change_field_type.rs | 2 +- .../struct_change_field_type_cross_crate/b.rs | 2 +- src/test/incremental/struct_remove_field.rs | 2 +- 6 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 423d57cfb328d..2a22e13d31ec4 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -946,7 +946,28 @@ impl<'tcx> TraitPredicate<'tcx> { /// Creates the dep-node for selecting/evaluating this trait reference. fn dep_node(&self) -> DepNode { - DepNode::TraitSelect(self.def_id(), vec![]) + // Ideally, the dep-node would just have all the input types + // in it. But they are limited to including def-ids. So as an + // approximation we include the def-ids for all nominal types + // found somewhere. This means that we will e.g. conflate the + // dep-nodes for `u32: SomeTrait` and `u64: SomeTrait`, but we + // would have distinct dep-nodes for `Vec: SomeTrait`, + // `Rc: SomeTrait`, and `(Vec, Rc): SomeTrait`. + // Note that it's always sound to conflate dep-nodes, it jus + // leads to more recompilation. + let def_ids: Vec<_> = + self.input_types() + .iter() + .flat_map(|t| t.walk()) + .filter_map(|t| match t.sty { + ty::TyStruct(adt_def, _) | + ty::TyEnum(adt_def, _) => + Some(adt_def.did), + _ => + None + }) + .collect(); + DepNode::TraitSelect(self.def_id(), def_ids) } pub fn input_types(&self) -> &[Ty<'tcx>] { diff --git a/src/test/incremental/struct_add_field.rs b/src/test/incremental/struct_add_field.rs index 425aa3e07a0ec..cc8ef8aedd77b 100644 --- a/src/test/incremental/struct_add_field.rs +++ b/src/test/incremental/struct_add_field.rs @@ -40,7 +40,7 @@ pub fn use_EmbedX(embed: EmbedX) -> u32 { embed.x.x as u32 } -#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] // FIXME(#33850) should be clean +#[rustc_clean(label="TypeckItemBody", cfg="rpass2")] pub fn use_Y() { let x: Y = Y { y: 'c' }; } diff --git a/src/test/incremental/struct_change_field_name.rs b/src/test/incremental/struct_change_field_name.rs index cf6f89bd8a4c5..fe29ad66b5fd8 100644 --- a/src/test/incremental/struct_change_field_name.rs +++ b/src/test/incremental/struct_change_field_name.rs @@ -47,7 +47,7 @@ pub fn use_EmbedX(embed: EmbedX) -> u32 { //[cfail2]~^ ERROR attempted access of field `x` } -#[rustc_dirty(label="TypeckItemBody", cfg="cfail2")] // FIXME(#33850) should be clean +#[rustc_clean(label="TypeckItemBody", cfg="cfail2")] pub fn use_Y() { let x: Y = Y { y: 'c' }; } diff --git a/src/test/incremental/struct_change_field_type.rs b/src/test/incremental/struct_change_field_type.rs index 2bd61a3f559d4..1a50d515db6d0 100644 --- a/src/test/incremental/struct_change_field_type.rs +++ b/src/test/incremental/struct_change_field_type.rs @@ -45,7 +45,7 @@ pub fn use_EmbedX(x: EmbedX) -> u32 { x.x as u32 } -#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] // FIXME(#33850) should be clean +#[rustc_clean(label="TypeckItemBody", cfg="rpass2")] pub fn use_Y() { let x: Y = Y { y: 'c' }; } diff --git a/src/test/incremental/struct_change_field_type_cross_crate/b.rs b/src/test/incremental/struct_change_field_type_cross_crate/b.rs index 19f146ce176d7..7a4900d1d9a90 100644 --- a/src/test/incremental/struct_change_field_type_cross_crate/b.rs +++ b/src/test/incremental/struct_change_field_type_cross_crate/b.rs @@ -28,7 +28,7 @@ pub fn use_EmbedX(embed: EmbedX) -> u32 { embed.x.x as u32 } -#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] // FIXME(#33850) should be clean +#[rustc_clean(label="TypeckItemBody", cfg="rpass2")] pub fn use_Y() { let x: Y = Y { y: 'c' }; } diff --git a/src/test/incremental/struct_remove_field.rs b/src/test/incremental/struct_remove_field.rs index db49a7516275d..ae6399463b81b 100644 --- a/src/test/incremental/struct_remove_field.rs +++ b/src/test/incremental/struct_remove_field.rs @@ -44,7 +44,7 @@ pub fn use_EmbedX(embed: EmbedX) -> u32 { embed.x.x as u32 } -#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] // FIXME(#33850) should be clean +#[rustc_clean(label="TypeckItemBody", cfg="rpass2")] pub fn use_Y() { let x: Y = Y { y: 'c' }; } From af4a0e8e0569ecf83278df437f504b63678e8710 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 3 Jun 2016 20:39:43 -0400 Subject: [PATCH 5/6] avoid extra clone --- src/librustc/dep_graph/raii.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc/dep_graph/raii.rs b/src/librustc/dep_graph/raii.rs index 92cecabaa182f..c43d493d17675 100644 --- a/src/librustc/dep_graph/raii.rs +++ b/src/librustc/dep_graph/raii.rs @@ -14,20 +14,20 @@ use super::thread::{DepGraphThreadData, DepMessage}; pub struct DepTask<'graph> { data: &'graph DepGraphThreadData, - key: DepNode, + key: Option>, } impl<'graph> DepTask<'graph> { pub fn new(data: &'graph DepGraphThreadData, key: DepNode) -> DepTask<'graph> { data.enqueue(DepMessage::PushTask(key.clone())); - DepTask { data: data, key: key } + DepTask { data: data, key: Some(key) } } } impl<'graph> Drop for DepTask<'graph> { fn drop(&mut self) { - self.data.enqueue(DepMessage::PopTask(self.key.clone())); + self.data.enqueue(DepMessage::PopTask(self.key.take().unwrap())); } } From e548c46c7067010f203db0a9c69e5c6fae058545 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 3 Jun 2016 20:40:44 -0400 Subject: [PATCH 6/6] correct misspelled word --- src/librustc/ty/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 2a22e13d31ec4..454f524e4da37 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -953,7 +953,7 @@ impl<'tcx> TraitPredicate<'tcx> { // dep-nodes for `u32: SomeTrait` and `u64: SomeTrait`, but we // would have distinct dep-nodes for `Vec: SomeTrait`, // `Rc: SomeTrait`, and `(Vec, Rc): SomeTrait`. - // Note that it's always sound to conflate dep-nodes, it jus + // Note that it's always sound to conflate dep-nodes, it just // leads to more recompilation. let def_ids: Vec<_> = self.input_types()