Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 18 additions & 6 deletions compiler/rustc_data_structures/src/graph/linked_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub struct LinkedGraph<N, E> {

pub struct Node<N> {
first_edge: [EdgeIndex; 2], // see module comment
pub data: N,
pub data: Option<N>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -135,18 +135,30 @@ impl<N: Debug, E: Debug> LinkedGraph<N, E> {
NodeIndex(self.nodes.len())
}

fn ensure_node(&mut self, idx: NodeIndex) -> &mut Node<N> {
self.nodes.ensure_contains_elem(idx, || Node {
first_edge: [INVALID_EDGE_INDEX, INVALID_EDGE_INDEX],
data: None,
})
}

pub fn add_node_with_idx(&mut self, idx: NodeIndex, data: N) {
let old_data = self.ensure_node(idx).data.replace(data);
debug_assert!(old_data.is_none());
}

pub fn add_node(&mut self, data: N) -> NodeIndex {
let idx = self.next_node_index();
self.nodes.push(Node { first_edge: [INVALID_EDGE_INDEX, INVALID_EDGE_INDEX], data });
self.add_node_with_idx(idx, data);
idx
}

pub fn mut_node_data(&mut self, idx: NodeIndex) -> &mut N {
&mut self.nodes[idx].data
self.nodes[idx].data.as_mut().unwrap()
}

pub fn node_data(&self, idx: NodeIndex) -> &N {
&self.nodes[idx].data
self.nodes[idx].data.as_ref().unwrap()
}

pub fn node(&self, idx: NodeIndex) -> &Node<N> {
Expand All @@ -165,8 +177,8 @@ impl<N: Debug, E: Debug> LinkedGraph<N, E> {
let idx = self.next_edge_index();

// read current first of the list of edges from each node
let source_first = self.nodes[source].first_edge[OUTGOING.repr];
let target_first = self.nodes[target].first_edge[INCOMING.repr];
let source_first = self.ensure_node(source).first_edge[OUTGOING.repr];
let target_first = self.ensure_node(target).first_edge[INCOMING.repr];

// create the new edge, with the previous firsts from each node
// as the next pointers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ fn each_node() {
let expected = ["A", "B", "C", "D", "E", "F"];
graph.each_node(|idx, node| {
assert_eq!(&expected[idx.0], graph.node_data(idx));
assert_eq!(expected[idx.0], node.data);
assert_eq!(expected[idx.0], node.data.unwrap());
true
});
}
Expand Down
17 changes: 5 additions & 12 deletions compiler/rustc_middle/src/dep_graph/retained.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::graph::linked_graph::{Direction, INCOMING, LinkedGraph, NodeIndex};
use rustc_index::IndexVec;

use super::{DepNode, DepNodeIndex};

Expand All @@ -13,7 +12,6 @@ use super::{DepNode, DepNodeIndex};
pub struct RetainedDepGraph {
pub inner: LinkedGraph<DepNode, ()>,
pub indices: FxHashMap<DepNode, NodeIndex>,
pub dep_index_to_index: IndexVec<DepNodeIndex, Option<NodeIndex>>,
}

impl RetainedDepGraph {
Expand All @@ -23,27 +21,22 @@ impl RetainedDepGraph {

let inner = LinkedGraph::with_capacity(node_count, edge_count);
let indices = FxHashMap::default();
let dep_index_to_index = IndexVec::new();

Self { inner, indices, dep_index_to_index }
Self { inner, indices }
}

pub fn push(&mut self, index: DepNodeIndex, node: DepNode, edges: &[DepNodeIndex]) {
let source = self.inner.add_node(node);
self.dep_index_to_index.insert(index, source);
let source = NodeIndex(index.as_usize());
self.inner.add_node_with_idx(source, node);
self.indices.insert(node, source);

for &target in edges.iter() {
// We may miss the edges that are pushed while the `DepGraphQuery` is being accessed.
// Skip them to issues.
if let Some(&Some(target)) = self.dep_index_to_index.get(target) {
self.inner.add_edge(source, target, ());
}
self.inner.add_edge(source, NodeIndex(target.as_usize()), ());
}
}

pub fn nodes(&self) -> Vec<&DepNode> {
self.inner.all_nodes().iter().map(|n| &n.data).collect()
self.inner.all_nodes().iter().map(|n| n.data.as_ref().unwrap()).collect()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks a bit suspicious.

The more obvious implementation would be to skip data-less nodes. Is there a reason to think that nodes will always be densely initialized whenever this is called?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The assumption is that the graph is used only when it's fully built (e.g. in assert_dep_graph/save_dep_graph).
The holes in data are supposed to occur only during the (parallel) graph building process.
If this ever changes, then it's better to fail with some noise than to silently lose nodes (like we are losing edges now).

}

pub fn edges(&self) -> Vec<(&DepNode, &DepNode)> {
Expand Down
Loading