Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incr. comp. dep-node for traits, tests #33998

Merged
merged 6 commits into from
Jun 4, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<D: Clone + Debug> {
// The `D` type is "how definitions are identified".
// During compilation, it is always `DefId`, but when serializing
Expand Down Expand Up @@ -116,7 +125,7 @@ pub enum DepNode<D: Clone + Debug> {
// which would yield an overly conservative dep-graph.
TraitItems(D),
ReprHints(D),
TraitSelect(D),
TraitSelect(D, Vec<D>),
}

impl<D: Clone + Debug> DepNode<D> {
Expand Down Expand Up @@ -212,7 +221,11 @@ impl<D: Clone + Debug> DepNode<D> {
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());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this works. Doesn't the argument to try_opt!() have to be an Option of something? But collect() will return a Vec?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can collect an iterator yielding Options into a single Option

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonas-schievink Oh, that's interesting. Thanks for enlightening me :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can collect an iterator yielding Options into a single Option

best feature ever

Some(TraitSelect(d, type_ds))
}
}
}
}
24 changes: 12 additions & 12 deletions src/librustc/dep_graph/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,26 +47,26 @@ impl<D: Clone + Debug + Hash + Eq> DepGraphQuery<D> {
self.indices.contains_key(&node)
}

pub fn nodes(&self) -> Vec<DepNode<D>> {
pub fn nodes(&self) -> Vec<&DepNode<D>> {
self.graph.all_nodes()
.iter()
.map(|n| n.data.clone())
.map(|n| &n.data)
.collect()
}

pub fn edges(&self) -> Vec<(DepNode<D>,DepNode<D>)> {
pub fn edges(&self) -> Vec<(&DepNode<D>,&DepNode<D>)> {
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<D>, direction: Direction) -> Vec<DepNode<D>> {
if let Some(&index) = self.indices.get(&node) {
fn reachable_nodes(&self, node: &DepNode<D>, direction: Direction) -> Vec<&DepNode<D>> {
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![]
Expand All @@ -75,20 +75,20 @@ impl<D: Clone + Debug + Hash + Eq> DepGraphQuery<D> {

/// 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<D>) -> Vec<DepNode<D>> {
pub fn transitive_successors(&self, node: &DepNode<D>) -> Vec<&DepNode<D>> {
self.reachable_nodes(node, OUTGOING)
}

/// All nodes that can reach `node`.
pub fn transitive_predecessors(&self, node: DepNode<D>) -> Vec<DepNode<D>> {
pub fn transitive_predecessors(&self, node: &DepNode<D>) -> Vec<&DepNode<D>> {
self.reachable_nodes(node, INCOMING)
}

/// Just the outgoing edges from `node`.
pub fn immediate_successors(&self, node: DepNode<D>) -> Vec<DepNode<D>> {
pub fn immediate_successors(&self, node: &DepNode<D>) -> Vec<&DepNode<D>> {
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![]
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/dep_graph/raii.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,20 @@ use super::thread::{DepGraphThreadData, DepMessage};

pub struct DepTask<'graph> {
data: &'graph DepGraphThreadData,
key: DepNode<DefId>,
key: Option<DepNode<DefId>>,
}

impl<'graph> DepTask<'graph> {
pub fn new(data: &'graph DepGraphThreadData, key: DepNode<DefId>)
-> DepTask<'graph> {
data.enqueue(DepMessage::PushTask(key));
DepTask { data: data, key: key }
data.enqueue(DepMessage::PushTask(key.clone()));
DepTask { data: data, key: Some(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.take().unwrap()));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/dep_graph/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
34 changes: 27 additions & 7 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,28 @@ impl<'tcx> TraitPredicate<'tcx> {

/// Creates the dep-node for selecting/evaluating this trait reference.
fn dep_node(&self) -> DepNode<DefId> {
DepNode::TraitSelect(self.def_id())
// 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<u32>: SomeTrait`,
// `Rc<u32>: SomeTrait`, and `(Vec<u32>, Rc<u32>): SomeTrait`.
// Note that it's always sound to conflate dep-nodes, it just
// 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>] {
Expand Down Expand Up @@ -1768,9 +1789,8 @@ impl<'a, 'tcx> AdtDefData<'tcx, 'tcx> {
stack: &mut Vec<AdtDefMaster<'tcx>>)
{

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;
}

Expand All @@ -1780,7 +1800,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;
}

Expand All @@ -1803,14 +1823,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)
}
}
}
Expand Down
80 changes: 40 additions & 40 deletions src/librustc_incremental/assert_dep_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
}
}
Expand All @@ -252,51 +252,51 @@ fn dump_graph(tcx: TyCtxt) {
}
}

pub struct GraphvizDepGraph(FnvHashSet<DepNode<DefId>>,
Vec<(DepNode<DefId>, DepNode<DefId>)>);
pub struct GraphvizDepGraph<'q>(FnvHashSet<&'q DepNode<DefId>>,
Vec<(&'q DepNode<DefId>, &'q DepNode<DefId>)>);

impl<'a, 'tcx> dot::GraphWalk<'a> for GraphvizDepGraph {
type Node = DepNode<DefId>;
type Edge = (DepNode<DefId>, DepNode<DefId>);
fn nodes(&self) -> dot::Nodes<DepNode<DefId>> {
impl<'a, 'tcx, 'q> dot::GraphWalk<'a> for GraphvizDepGraph<'q> {
type Node = &'q DepNode<DefId>;
type Edge = (&'q DepNode<DefId>, &'q DepNode<DefId>);
fn nodes(&self) -> dot::Nodes<&'q DepNode<DefId>> {
let nodes: Vec<_> = self.0.iter().cloned().collect();
nodes.into_cow()
}
fn edges(&self) -> dot::Edges<(DepNode<DefId>, DepNode<DefId>)> {
fn edges(&self) -> dot::Edges<(&'q DepNode<DefId>, &'q DepNode<DefId>)> {
self.1[..].into_cow()
}
fn source(&self, edge: &(DepNode<DefId>, DepNode<DefId>)) -> DepNode<DefId> {
fn source(&self, edge: &(&'q DepNode<DefId>, &'q DepNode<DefId>)) -> &'q DepNode<DefId> {
edge.0
}
fn target(&self, edge: &(DepNode<DefId>, DepNode<DefId>)) -> DepNode<DefId> {
fn target(&self, edge: &(&'q DepNode<DefId>, &'q DepNode<DefId>)) -> &'q DepNode<DefId> {
edge.1
}
}

impl<'a, 'tcx> dot::Labeller<'a> for GraphvizDepGraph {
type Node = DepNode<DefId>;
type Edge = (DepNode<DefId>, DepNode<DefId>);
impl<'a, 'tcx, 'q> dot::Labeller<'a> for GraphvizDepGraph<'q> {
type Node = &'q DepNode<DefId>;
type Edge = (&'q DepNode<DefId>, &'q DepNode<DefId>);
fn graph_id(&self) -> dot::Id {
dot::Id::new("DependencyGraph").unwrap()
}
fn node_id(&self, n: &DepNode<DefId>) -> dot::Id {
fn node_id(&self, n: &&'q DepNode<DefId>) -> dot::Id {
let s: String =
format!("{:?}", n).chars()
.map(|c| if c == '_' || c.is_alphanumeric() { c } else { '_' })
.collect();
debug!("n={:?} s={:?}", n, s);
dot::Id::new(s).unwrap()
}
fn node_label(&self, n: &DepNode<DefId>) -> dot::LabelText {
fn node_label(&self, n: &&'q DepNode<DefId>) -> dot::LabelText {
dot::LabelText::label(format!("{:?}", n))
}
}

// 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<DefId>, filter: &DepNodeFilter)
-> Option<FnvHashSet<DepNode<DefId>>>
fn node_set<'q>(query: &'q DepGraphQuery<DefId>, filter: &DepNodeFilter)
-> Option<FnvHashSet<&'q DepNode<DefId>>>
{
debug!("node_set(filter={:?})", filter);

Expand All @@ -307,10 +307,10 @@ fn node_set(query: &DepGraphQuery<DefId>, filter: &DepNodeFilter)
Some(query.nodes().into_iter().filter(|n| filter.test(n)).collect())
}

fn filter_nodes(query: &DepGraphQuery<DefId>,
sources: &Option<FnvHashSet<DepNode<DefId>>>,
targets: &Option<FnvHashSet<DepNode<DefId>>>)
-> FnvHashSet<DepNode<DefId>>
fn filter_nodes<'q>(query: &'q DepGraphQuery<DefId>,
sources: &Option<FnvHashSet<&'q DepNode<DefId>>>,
targets: &Option<FnvHashSet<&'q DepNode<DefId>>>)
-> FnvHashSet<&'q DepNode<DefId>>
{
if let &Some(ref sources) = sources {
if let &Some(ref targets) = targets {
Expand All @@ -325,21 +325,21 @@ fn filter_nodes(query: &DepGraphQuery<DefId>,
}
}

fn walk_nodes(query: &DepGraphQuery<DefId>,
starts: &FnvHashSet<DepNode<DefId>>,
direction: Direction)
-> FnvHashSet<DepNode<DefId>>
fn walk_nodes<'q>(query: &'q DepGraphQuery<DefId>,
starts: &FnvHashSet<&'q DepNode<DefId>>,
direction: Direction)
-> FnvHashSet<&'q DepNode<DefId>>
{
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);
}
}
Expand All @@ -349,10 +349,10 @@ fn walk_nodes(query: &DepGraphQuery<DefId>,
set
}

fn walk_between(query: &DepGraphQuery<DefId>,
sources: &FnvHashSet<DepNode<DefId>>,
targets: &FnvHashSet<DepNode<DefId>>)
-> FnvHashSet<DepNode<DefId>>
fn walk_between<'q>(query: &'q DepGraphQuery<DefId>,
sources: &FnvHashSet<&'q DepNode<DefId>>,
targets: &FnvHashSet<&'q DepNode<DefId>>)
-> FnvHashSet<&'q DepNode<DefId>>
{
// 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
Expand All @@ -365,16 +365,16 @@ fn walk_between(query: &DepGraphQuery<DefId>,
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
})
Expand Down Expand Up @@ -417,12 +417,12 @@ fn walk_between(query: &DepGraphQuery<DefId>,
}
}

fn filter_edges(query: &DepGraphQuery<DefId>,
nodes: &FnvHashSet<DepNode<DefId>>)
-> Vec<(DepNode<DefId>, DepNode<DefId>)>
fn filter_edges<'q>(query: &'q DepGraphQuery<DefId>,
nodes: &FnvHashSet<&'q DepNode<DefId>>)
-> Vec<(&'q DepNode<DefId>, &'q DepNode<DefId>)>
{
query.edges()
.into_iter()
.filter(|&(source, target)| nodes.contains(&source) && nodes.contains(&target))
.filter(|&(source, target)| nodes.contains(source) && nodes.contains(target))
.collect()
}
Loading