From 6151bab8e1b94510eeb3929eb969ef8679a24294 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <nnethercote@mozilla.com>
Date: Thu, 7 Jun 2018 15:35:10 +1000
Subject: [PATCH 1/2] Improve pushing to `Node::dependents`.

This patch makes it impossible for a node to end up in both
`node.parent` and `node.dependents`.
---
 .../obligation_forest/mod.rs                      | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/librustc_data_structures/obligation_forest/mod.rs b/src/librustc_data_structures/obligation_forest/mod.rs
index 990dc66c66a..c66011d4118 100644
--- a/src/librustc_data_structures/obligation_forest/mod.rs
+++ b/src/librustc_data_structures/obligation_forest/mod.rs
@@ -193,15 +193,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(())

From 70d22fa0519c8970cecbae400e7f6ebf69704d92 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <nnethercote@mozilla.com>
Date: Thu, 7 Jun 2018 16:19:52 +1000
Subject: [PATCH 2/2] Improve `Node::{parent,dependents}` interplay.

This patch:

- Reorders things a bit so that `parent` is always handled before
  `dependents`.

- Uses iterator chaining to avoid some code duplication.
---
 .../obligation_forest/mod.rs                  | 24 +++++++------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/librustc_data_structures/obligation_forest/mod.rs b/src/librustc_data_structures/obligation_forest/mod.rs
index c66011d4118..df34891ff03 100644
--- a/src/librustc_data_structures/obligation_forest/mod.rs
+++ b/src/librustc_data_structures/obligation_forest/mod.rs
@@ -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
@@ -383,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();
@@ -430,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())
             );
         }
 
@@ -440,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()]);
         }
     }
@@ -591,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![],
         }
     }