fix dfs iterator when same file imported multiple times into another

This commit is contained in:
Noah Santschi-Cooney 2022-04-24 22:16:45 +01:00
parent 27d1d7b34e
commit 3b568ea087
No known key found for this signature in database
GPG key ID: 3B22282472C8AE48
4 changed files with 64 additions and 82 deletions

View file

@ -63,25 +63,22 @@ impl<'a> Iterator for Dfs<'a> {
touch: 1,
});
let mut children: Vec<NodeIndex> = self.graph.child_node_indexes(child).collect();
let mut children: Vec<_> = self
.graph
.get_all_child_positions(child)
.collect();
children.reverse();
if !children.is_empty() {
// sort by line number in parent
children.sort_by(|x, y| {
let graph = &self.graph.graph;
let edge1 = graph.edge_weight(graph.find_edge(child, *x).unwrap()).unwrap();
let edge2 = graph.edge_weight(graph.find_edge(child, *y).unwrap()).unwrap();
edge2.line.cmp(&edge1.line)
});
match self.check_for_cycle(&children) {
let child_indexes: Vec<_> = children.iter().map(|c| c.0).collect();
match self.check_for_cycle(&child_indexes) {
Ok(_) => {}
Err(e) => return Some(Err(e)),
};
for child in children {
self.stack.push(child);
self.stack.push(child.0);
}
} else {
self.reset_path_to_branch();
@ -314,10 +311,10 @@ mod dfs_test {
// \ / \
// 6 - 7
assert!(is_cyclic_directed(&graph.graph));
let next = dfs.next().unwrap();
assert_that!(next, err());
assert!(is_cyclic_directed(&graph.graph));
}
{
let mut graph = CachedStableGraph::new();

View file

@ -59,9 +59,9 @@ impl CachedStableGraph {
PathBuf::from_str(&self.graph[node]).unwrap()
}
/// returns an iterator over all the `IncludePosition`'s between a parent and its child for all the positions
/// Returns an iterator over all the `IncludePosition`'s between a parent and its child for all the positions
/// that the child may be imported into the parent, in order of import.
pub fn get_edge_metas(&self, parent: NodeIndex, child: NodeIndex) -> impl Iterator<Item = IncludePosition> + '_ {
pub fn get_child_positions(&self, parent: NodeIndex, child: NodeIndex) -> impl Iterator<Item = IncludePosition> + '_ {
let mut edges = self
.graph
.edges(parent)
@ -77,6 +77,18 @@ impl CachedStableGraph {
edges.into_iter()
}
/// Returns an iterator over all the `(NodeIndex, IncludePosition)` tuples between a node and all its children, in order
/// of import.
pub fn get_all_child_positions(&self, node: NodeIndex) -> impl Iterator<Item = (NodeIndex, IncludePosition)> + '_ {
let mut edges = self.graph.edges(node).map(|edge| {
let child = self.graph.edge_endpoints(edge.id()).unwrap().1;
(child, self.graph[edge.id()])
})
.collect::<Vec<_>>();
edges.sort_by(|x, y| x.1.line.cmp(&y.1.line));
edges.into_iter()
}
pub fn add_node(&mut self, name: &Path) -> NodeIndex {
if let Some(idx) = self.cache.get(name) {
return *idx;
@ -99,14 +111,6 @@ impl CachedStableGraph {
.and_then(|edge| self.graph.remove_edge(edge));
}
pub fn child_node_metas(&self, node: NodeIndex) -> impl Iterator<Item = (PathBuf, IncludePosition)> + '_ {
self.graph.neighbors(node).map(move |n| {
let edge = self.graph.find_edge(node, n).unwrap();
let edge_meta = self.graph.edge_weight(edge).unwrap();
return (self.reverse_index.get(&n).unwrap().clone(), *edge_meta);
})
}
pub fn child_node_indexes(&self, node: NodeIndex) -> impl Iterator<Item = NodeIndex> + '_ {
self.graph.neighbors(node)
}
@ -236,9 +240,9 @@ mod graph_test {
// / \
// 1 1
assert_eq!(2, graph.get_edge_metas(idx0, idx1).count());
assert_eq!(2, graph.get_child_positions(idx0, idx1).count());
let mut edge_metas = graph.get_edge_metas(idx0, idx1);
let mut edge_metas = graph.get_child_positions(idx0, idx1);
assert_eq!(Some(IncludePosition { line: 2, start: 0, end: 0 }), edge_metas.next());
assert_eq!(Some(IncludePosition { line: 4, start: 0, end: 0 }), edge_metas.next());
}

View file

@ -251,7 +251,9 @@ impl MinecraftShaderLanguageServer {
Some(n) => n,
};
let prev_children: HashSet<_> = HashSet::from_iter(self.graph.borrow().child_node_metas(idx));
let prev_children: HashSet<_> = HashSet::from_iter(self.graph.borrow().get_all_child_positions(idx).map(|tup| {
(self.graph.borrow().get_node(tup.0), tup.1)
}));
let new_children: HashSet<_> = includes.iter().cloned().collect();
let to_be_added = new_children.difference(&prev_children);
@ -825,7 +827,7 @@ impl LanguageServerHandling for MinecraftShaderLanguageServer {
.child_node_indexes(node)
.filter_map::<Vec<DocumentLink>, _>(|child| {
let graph = self.graph.borrow();
graph.get_edge_metas(node, child).map(|value| {
graph.get_child_positions(node, child).map(|value| {
let path = graph.get_node(child);
let url = match Url::from_file_path(&path) {
Ok(url) => url,

View file

@ -8,6 +8,7 @@ use std::{
use core::slice::Iter;
use petgraph::stable_graph::NodeIndex;
use slog_scope::debug;
use crate::graph::CachedStableGraph;
use crate::source_mapper::SourceMapper;
@ -88,13 +89,7 @@ impl<'a> MergeViewBuilder<'a> {
// );
// last_offset_set.insert((first, None), version_char_offsets.1);
self.last_offset_set.insert(
FilialTuple {
child: first,
parent: None,
},
0,
);
self.set_last_offset_for_tuple(None, first, 0);
// stack to keep track of the depth first traversal
let mut stack = VecDeque::<NodeIndex>::new();
@ -102,13 +97,8 @@ impl<'a> MergeViewBuilder<'a> {
self.create_merge_views(&mut merge_list, &mut extra_lines, &mut stack);
// now we add a view of the remainder of the root file
let offset = *self
.last_offset_set
.get(&FilialTuple {
child: first,
parent: None,
})
.unwrap();
let offset = self.get_last_offset_for_tuple(None, first).unwrap();
let len = first_source.len();
merge_list.push_back(&first_source[min(offset, len)..]);
@ -135,8 +125,8 @@ impl<'a> MergeViewBuilder<'a> {
.parent_child_edge_iterator
.entry(*n)
.or_insert_with(|| {
let edge_metas = self.graph.get_edge_metas(parent, child);
Box::new(edge_metas)
let child_positions = self.graph.get_child_positions(parent, child);
Box::new(child_positions)
})
.next()
.unwrap();
@ -147,15 +137,16 @@ impl<'a> MergeViewBuilder<'a> {
let (char_for_line, char_following_line) = self.char_offset_for_line(edge.line, parent_source);
let offset = *self
.last_offset_set
.insert(
FilialTuple {
child: parent,
parent: stack.back().copied(),
},
char_following_line,
)
.set_last_offset_for_tuple(stack.back().copied(), parent, char_following_line)
.get_or_insert(0);
debug!("creating view to start child file";
"parent" => parent_path.to_str().unwrap(), "child" => child_path.to_str().unwrap(),
"grandparent" => stack.back().copied().map(|g| self.graph.get_node(g).to_str().unwrap().to_string()), // self.graph.get_node().to_str().unwrap(),
"last_parent_offset" => offset, "line" => edge.line, "char_for_line" => char_for_line,
"char_following_line" => char_following_line,
);
merge_list.push_back(&parent_source[offset..char_for_line]);
self.add_opening_line_directive(&child_path, child, merge_list, extra_lines);
@ -173,13 +164,7 @@ impl<'a> MergeViewBuilder<'a> {
}
};
merge_list.push_back(&child_source[..offset]);
self.last_offset_set.insert(
FilialTuple {
child,
parent: Some(parent),
},
0,
);
self.set_last_offset_for_tuple(Some(parent), child, 0);
// +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
self.add_closing_line_directive(edge.line + 2, &parent_path, parent, merge_list, extra_lines);
// if the next pair's parent is not the current pair's parent, we need to bubble up
@ -193,29 +178,17 @@ impl<'a> MergeViewBuilder<'a> {
self.create_merge_views(merge_list, extra_lines, stack);
stack.pop_back();
let offset = *self
.last_offset_set
.get(&FilialTuple {
child,
parent: Some(parent),
})
.unwrap();
let offset = self.get_last_offset_for_tuple(Some(parent), child).unwrap();
let child_source = self.sources.get(&child_path).unwrap();
// this evaluates to false once the file contents have been exhausted aka offset = child_source.len() + 1
let end_offset = match child_source.ends_with('\n') {
true => 1, /* child_source.len()-1 */
false => 0, /* child_source.len() */
true => 1,
false => 0,
};
if offset < child_source.len() - end_offset {
// if ends in \n\n, we want to exclude the last \n for some reason. Ask optilad
merge_list.push_back(&child_source[offset../* std::cmp::max( */child_source.len()-end_offset/* , offset) */]);
self.last_offset_set.insert(
FilialTuple {
child,
parent: Some(parent),
},
0,
);
merge_list.push_back(&child_source[offset..child_source.len() - end_offset]);
self.set_last_offset_for_tuple(Some(parent), child, 0);
}
// +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
@ -234,13 +207,7 @@ impl<'a> MergeViewBuilder<'a> {
false => child_source.len(),
};
merge_list.push_back(&child_source[..offset]);
self.last_offset_set.insert(
FilialTuple {
child,
parent: Some(parent),
},
0,
);
self.set_last_offset_for_tuple(Some(parent), child, 0);
// +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
self.add_closing_line_directive(edge.line + 2, &parent_path, parent, merge_list, extra_lines);
}
@ -248,6 +215,18 @@ impl<'a> MergeViewBuilder<'a> {
}
}
fn set_last_offset_for_tuple(&mut self, parent: Option<NodeIndex>, child: NodeIndex, offset: usize) -> Option<usize> {
debug!("inserting last offset";
"parent" => parent.map(|p| self.graph.get_node(p).to_str().unwrap().to_string()),
"child" => self.graph.get_node(child).to_str().unwrap().to_string(),
"offset" => offset);
self.last_offset_set.insert(FilialTuple { child, parent }, offset)
}
fn get_last_offset_for_tuple(&self, parent: Option<NodeIndex>, child: NodeIndex) -> Option<usize> {
self.last_offset_set.get(&FilialTuple { child, parent }).copied()
}
// returns the character offset + 1 of the end of line number `line` and the character
// offset + 1 for the end of the line after the previous one
fn char_offset_for_line(&self, line_num: usize, source: &str) -> (usize, usize) {