Auto merge of #51613 - nnethercote:ob-forest-cleanup, r=nikomatsakis

Obligation forest cleanup

While looking at this code I was scratching my head about whether a node could appear in both `parent` and `dependents`. Turns out it can, but it's not useful to do so, so this PR cleans things up so it's no longer possible.
This commit is contained in:
bors 2018-06-26 07:06:18 +00:00
commit 773ce53ce7

View File

@ -91,13 +91,14 @@ struct Node<O> {
obligation: O,
state: Cell<NodeState>,
/// The parent of a node - the original obligation of
/// which it is a subobligation. Except for error reporting,
/// it is just like any member of `dependents`.
parent: Option<NodeIndex>,
/// Obligations that depend on this obligation for their
/// completion. They must all be in a non-pending state.
dependents: Vec<NodeIndex>,
/// The parent of a node - the original obligation of
/// which it is a subobligation. Except for error reporting,
/// this is just another member of `dependents`.
parent: Option<NodeIndex>,
}
/// The state of one node in some tree within the forest. This
@ -193,15 +194,18 @@ impl<O: ForestObligation> ObligationForest<O> {
Entry::Occupied(o) => {
debug!("register_obligation_at({:?}, {:?}) - duplicate of {:?}!",
obligation, parent, o.get());
let node = &mut self.nodes[o.get().get()];
if let Some(parent) = parent {
if self.nodes[o.get().get()].dependents.contains(&parent) {
debug!("register_obligation_at({:?}, {:?}) - duplicate subobligation",
obligation, parent);
} else {
self.nodes[o.get().get()].dependents.push(parent);
// If the node is already in `waiting_cache`, it's already
// been marked with a parent. (It's possible that parent
// has been cleared by `apply_rewrites`, though.) So just
// dump `parent` into `node.dependents`... unless it's
// already in `node.dependents` or `node.parent`.
if !node.dependents.contains(&parent) && Some(parent) != node.parent {
node.dependents.push(parent);
}
}
if let NodeState::Error = self.nodes[o.get().get()].state.get() {
if let NodeState::Error = node.state.get() {
Err(())
} else {
Ok(())
@ -380,10 +384,7 @@ impl<O: ForestObligation> ObligationForest<O> {
NodeState::Success => {
node.state.set(NodeState::OnDfsStack);
stack.push(index);
if let Some(parent) = node.parent {
self.find_cycles_from_node(stack, processor, parent.get());
}
for dependent in &node.dependents {
for dependent in node.parent.iter().chain(node.dependents.iter()) {
self.find_cycles_from_node(stack, processor, dependent.get());
}
stack.pop();
@ -427,7 +428,7 @@ impl<O: ForestObligation> ObligationForest<O> {
}
error_stack.extend(
node.dependents.iter().cloned().chain(node.parent).map(|x| x.get())
node.parent.iter().chain(node.dependents.iter()).map(|x| x.get())
);
}
@ -437,11 +438,7 @@ impl<O: ForestObligation> ObligationForest<O> {
#[inline]
fn mark_neighbors_as_waiting_from(&self, node: &Node<O>) {
if let Some(parent) = node.parent {
self.mark_as_waiting_from(&self.nodes[parent.get()]);
}
for dependent in &node.dependents {
for dependent in node.parent.iter().chain(node.dependents.iter()) {
self.mark_as_waiting_from(&self.nodes[dependent.get()]);
}
}
@ -588,8 +585,8 @@ impl<O> Node<O> {
fn new(parent: Option<NodeIndex>, obligation: O) -> Node<O> {
Node {
obligation,
parent,
state: Cell::new(NodeState::Pending),
parent,
dependents: vec![],
}
}