replace (NodeIndex, Option<NodeIndex>) with FilialTuple globally

Author: Strum355

server/src/commands/merged_includes.rs

@@ -14,6 +14,7 @@ use anyhow::{format_err, Result};
 use std::fs;
 
 use crate::dfs;
+use crate::merge_views::FilialTuple;
 use crate::{graph::CachedStableGraph, merge_views, url_norm::FromJson};
 
 use super::Invokeable;
@@ -36,15 +37,15 @@ impl VirtualMergedDocument {
         Ok(Some(roots))
     }
 
-    pub fn get_dfs_for_node(&self, root: NodeIndex) -> Result<Vec<(NodeIndex, Option<NodeIndex>)>, dfs::error::CycleError> {
+    pub fn get_dfs_for_node(&self, root: NodeIndex) -> Result<Vec<FilialTuple>, dfs::error::CycleError> {
         let graph_ref = self.graph.borrow();
 
         let dfs = dfs::Dfs::new(&graph_ref, root);
 
         dfs.collect::<Result<Vec<_>, _>>()
     }
 
-    pub fn load_sources(&self, nodes: &[(NodeIndex, Option<NodeIndex>)]) -> Result<HashMap<PathBuf, String>> {
+    pub fn load_sources(&self, nodes: &[FilialTuple]) -> Result<HashMap<PathBuf, String>> {
         let mut sources = HashMap::new();
 
         for node in nodes {

server/src/dfs.rs

@@ -1,6 +1,6 @@
 use petgraph::stable_graph::NodeIndex;
 
-use crate::graph::CachedStableGraph;
+use crate::{graph::CachedStableGraph, merge_views::FilialTuple};
 
 use anyhow::Result;
 
@@ -51,9 +51,9 @@ impl<'a> Dfs<'a> {
 }
 
 impl<'a> Iterator for Dfs<'a> {
-    type Item = Result<(NodeIndex, Option<NodeIndex>), error::CycleError>;
+    type Item = Result<FilialTuple, error::CycleError>;
 
-    fn next(&mut self) -> Option<Result<(NodeIndex, Option<NodeIndex>), error::CycleError>> {
+    fn next(&mut self) -> Option<Result<FilialTuple, error::CycleError>> {
         let parent = self.cycle.last().map(|p| p.node);
 
         if let Some(node) = self.stack.pop() {

server/src/main.rs

@@ -1,3 +1,4 @@
+use merge_views::FilialTuple;
 use rust_lsp::jsonrpc::{method_types::*, *};
 use rust_lsp::lsp::*;
 use rust_lsp::lsp_types::{notification::*, *};
@@ -47,8 +48,8 @@ mod graph;
 mod logging;
 mod lsp_ext;
 mod merge_views;
-mod opengl;
 mod navigation;
+mod opengl;
 mod url_norm;
 
 #[cfg(test)]
@@ -419,15 +420,15 @@ impl MinecraftShaderLanguageServer {
         Ok(diagnostics)
     }
 
-    pub fn get_dfs_for_node(&self, root: NodeIndex) -> Result<Vec<(NodeIndex, Option<NodeIndex>)>, dfs::error::CycleError> {
+    pub fn get_dfs_for_node(&self, root: NodeIndex) -> Result<Vec<FilialTuple>, dfs::error::CycleError> {
         let graph_ref = self.graph.borrow();
 
         let dfs = dfs::Dfs::new(&graph_ref, root);
 
         dfs.collect::<Result<_, _>>()
     }
 
-    pub fn load_sources(&self, nodes: &[(NodeIndex, Option<NodeIndex>)]) -> Result<HashMap<PathBuf, String>> {
+    pub fn load_sources(&self, nodes: &[FilialTuple]) -> Result<HashMap<PathBuf, String>> {
         let mut sources = HashMap::new();
 
         for node in nodes {
@@ -675,13 +676,15 @@ impl LanguageServerHandling for MinecraftShaderLanguageServer {
             let parser = &mut self.tree_sitter.borrow_mut();
             let parser_ctx = match navigation::ParserContext::new(parser, params.text_document.uri.clone()) {
                 Ok(ctx) => ctx,
-                Err(e) => return completable.complete(Err(MethodError{
-                    code: 42069,
-                    message: format!("error building parser context: {}", e),
-                    data: (),
-                })),
+                Err(e) => {
+                    return completable.complete(Err(MethodError {
+                        code: 42069,
+                        message: format!("error building parser context: {}", e),
+                        data: (),
+                    }))
+                }
             };
-    
+
             match parser_ctx.find_definitions(params.text_document.uri, params.position) {
                 Ok(locations) => completable.complete(Ok(locations)),
                 Err(e) => completable.complete(Err(MethodError {

server/src/merge_views.rs

@@ -11,28 +11,22 @@ use petgraph::stable_graph::NodeIndex;
 
 use crate::graph::CachedStableGraph;
 
-/// FilialTuple represents a tuple with a parent at index 0
-/// and a child at index 1. Parent can be nullable in the case of
+/// FilialTuple represents a tuple with a child at index 0
+/// and a parent at index 1. Parent can be nullable in the case of
 /// the child being a top level node in the tree.
-#[derive(PartialEq, Eq, Hash)]
-struct FilialTuple(Option<NodeIndex>, NodeIndex);
+pub type FilialTuple = (NodeIndex, Option<NodeIndex>);
 
-impl From<(Option<&NodeIndex>, NodeIndex)> for FilialTuple {
-    fn from(tuple: (Option<&NodeIndex>, NodeIndex)) -> Self {
-        FilialTuple(tuple.0.copied(), tuple.1)
-    }
-}
-
-pub fn generate_merge_list<'a>(
-    nodes: &'a [(NodeIndex, Option<NodeIndex>)], sources: &'a HashMap<PathBuf, String>, graph: &'a CachedStableGraph,
-) -> String {
-    let mut line_directives: Vec<String> = Vec::new();
+pub fn generate_merge_list<'a>(nodes: &'a [FilialTuple], sources: &'a HashMap<PathBuf, String>, graph: &'a CachedStableGraph) -> String {
+    // contains additionally inserted lines such as #line and other directives, preamble defines etc
+    let mut extra_lines: Vec<String> = Vec::new();
+    extra_lines.reserve((nodes.len() * 2) + 2);
 
     // list of source code views onto the below sources
     let mut merge_list: LinkedList<&'a str> = LinkedList::new();
 
-    line_directives.reserve(nodes.len() * 2);
-
+    // holds the offset into the child which has been added to the merge list for a parent.
+    // A child can have multiple parents for a given tree, hence we have to track it for
+    // a (child, parent) tuple instead of just the child.
     let mut last_offset_set: HashMap<FilialTuple, usize> = HashMap::new();
 
     let mut nodes_iter = nodes.iter().peekable();
@@ -41,7 +35,7 @@ pub fn generate_merge_list<'a>(
     let first = nodes_iter.next().unwrap().0;
     let first_path = graph.get_node(first);
 
-    last_offset_set.insert(FilialTuple(None, first), 0);
+    last_offset_set.insert((first, None), 0);
 
     // stack to keep track of the depth first traversal
     let mut stack = VecDeque::<NodeIndex>::new();
@@ -52,30 +46,27 @@ pub fn generate_merge_list<'a>(
         &mut last_offset_set,
         graph,
         sources,
-        &mut line_directives,
+        &mut extra_lines,
         &mut stack,
     );
 
     // now we add a view of the remainder of the root file
-    let offset = *last_offset_set.get(&FilialTuple(None, first)).unwrap();
+    let offset = *last_offset_set.get(&(first, None)).unwrap();
 
     let len = sources.get(&first_path).unwrap().len();
     merge_list.push_back(&sources.get(&first_path).unwrap()[min(offset, len)..]);
 
     let total_len = merge_list.iter().fold(0, |a, b| a + b.len());
 
     let mut merged = String::with_capacity(total_len);
-    for slice in merge_list {
-        merged.push_str(slice);
-    }
+    merged.extend(merge_list);
 
     merged
 }
 
 fn create_merge_views<'a>(
-    nodes: &mut Peekable<Iter<(NodeIndex, Option<NodeIndex>)>>, merge_list: &mut LinkedList<&'a str>,
-    last_offset_set: &mut HashMap<FilialTuple, usize>, graph: &'a CachedStableGraph, sources: &'a HashMap<PathBuf, String>,
-    line_directives: &mut Vec<String>, stack: &mut VecDeque<NodeIndex>,
+    nodes: &mut Peekable<Iter<FilialTuple>>, merge_list: &mut LinkedList<&'a str>, last_offset_set: &mut HashMap<FilialTuple, usize>,
+    graph: &'a CachedStableGraph, sources: &'a HashMap<PathBuf, String>, extra_lines: &mut Vec<String>, stack: &mut VecDeque<NodeIndex>,
 ) {
     loop {
         let n = match nodes.next() {
@@ -94,10 +85,10 @@ fn create_merge_views<'a>(
         let (char_for_line, char_following_line) = char_offset_for_line(edge.line, parent_source);
 
         let offset = *last_offset_set
-            .insert((stack.back(), parent).into(), char_following_line)
+            .insert((parent, stack.back().copied()), char_following_line)
             .get_or_insert(0);
         merge_list.push_back(&parent_source[offset..char_for_line]);
-        add_opening_line_directive(&child_path, merge_list, line_directives);
+        add_opening_line_directive(&child_path, merge_list, extra_lines);
 
         match nodes.peek() {
             Some(next) => {
@@ -113,9 +104,9 @@ fn create_merge_views<'a>(
                         }
                     };
                     merge_list.push_back(&child_source[..offset]);
-                    last_offset_set.insert(FilialTuple(Some(parent), child), 0);
+                    last_offset_set.insert((child, Some(parent)), 0);
                     // +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
-                    add_closing_line_directive(edge.line + 2, &parent_path, merge_list, line_directives);
+                    add_closing_line_directive(edge.line + 2, &parent_path, merge_list, extra_lines);
                     // if the next pair's parent is not the current pair's parent, we need to bubble up
                     if stack.contains(&next.1.unwrap()) {
                         return;
@@ -124,26 +115,24 @@ fn create_merge_views<'a>(
                 }
 
                 stack.push_back(parent);
-                create_merge_views(nodes, merge_list, last_offset_set, graph, sources, line_directives, stack);
+                create_merge_views(nodes, merge_list, last_offset_set, graph, sources, extra_lines, stack);
                 stack.pop_back();
 
-                let offset = *last_offset_set.get(&FilialTuple(Some(parent), child)).unwrap();
+                let offset = *last_offset_set.get(&(child, Some(parent))).unwrap();
                 let child_source = 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() */
-                    }
+                let end_offset = match child_source.ends_with('\n') {
+                    true => 1,  /* child_source.len()-1 */
+                    false => 0, /* child_source.len() */
                 };
                 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) */]);
-                    last_offset_set.insert(FilialTuple(Some(parent), child), 0);
+                    last_offset_set.insert((child, Some(parent)), 0);
                 }
 
                 // +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
-                add_closing_line_directive(edge.line + 2, &parent_path, merge_list, line_directives);
+                add_closing_line_directive(edge.line + 2, &parent_path, merge_list, extra_lines);
 
                 // we need to check the next item at the point of original return further down the callstack
                 if nodes.peek().is_some() && stack.contains(&nodes.peek().unwrap().1.unwrap()) {
@@ -153,16 +142,14 @@ fn create_merge_views<'a>(
             None => {
                 let child_source = sources.get(&child_path).unwrap();
                 // if ends in \n\n, we want to exclude the last \n for some reason. Ask optilad
-                let offset = {
-                    match child_source.ends_with('\n') {
-                        true => child_source.len() - 1,
-                        false => child_source.len(),
-                    }
+                let offset = match child_source.ends_with('\n') {
+                    true => child_source.len() - 1,
+                    false => child_source.len(),
                 };
                 merge_list.push_back(&child_source[..offset]);
-                last_offset_set.insert(FilialTuple(Some(parent), child), 0);
+                last_offset_set.insert((child, Some(parent)), 0);
                 // +2 because edge.line is 0 indexed but #line is 1 indexed and references the *following* line
-                add_closing_line_directive(edge.line + 2, &parent_path, merge_list, line_directives);
+                add_closing_line_directive(edge.line + 2, &parent_path, merge_list, extra_lines);
             }
         }
     }
@@ -184,13 +171,13 @@ fn char_offset_for_line(line_num: usize, source: &str) -> (usize, usize) {
     (char_for_line, char_following_line)
 }
 
-fn add_opening_line_directive(path: &Path, merge_list: &mut LinkedList<&str>, line_directives: &mut Vec<String>) {
+fn add_opening_line_directive(path: &Path, merge_list: &mut LinkedList<&str>, extra_lines: &mut Vec<String>) {
     let line_directive = format!("#line 1 \"{}\"\n", path.to_str().unwrap().replace('\\', "\\\\"));
-    line_directives.push(line_directive);
-    unsafe_get_and_insert(merge_list, line_directives);
+    extra_lines.push(line_directive);
+    unsafe_get_and_insert(merge_list, extra_lines);
 }
 
-fn add_closing_line_directive(line: usize, path: &Path, merge_list: &mut LinkedList<&str>, line_directives: &mut Vec<String>) {
+fn add_closing_line_directive(line: usize, path: &Path, merge_list: &mut LinkedList<&str>, extra_lines: &mut Vec<String>) {
     // Optifine doesn't seem to add a leading newline if the previous line was a #line directive
     let line_directive = if let Some(l) = merge_list.back() {
         if l.trim().starts_with("#line") {
@@ -202,14 +189,14 @@ fn add_closing_line_directive(line: usize, path: &Path, merge_list: &mut LinkedL
         format!("\n#line {} \"{}\"\n", line, path.to_str().unwrap().replace('\\', "\\\\"))
     };
 
-    line_directives.push(line_directive);
-    unsafe_get_and_insert(merge_list, line_directives);
+    extra_lines.push(line_directive);
+    unsafe_get_and_insert(merge_list, extra_lines);
 }
 
-fn unsafe_get_and_insert(merge_list: &mut LinkedList<&str>, line_directives: &[String]) {
+fn unsafe_get_and_insert(merge_list: &mut LinkedList<&str>, extra_lines: &[String]) {
     // :^)
     unsafe {
-        let vec_ptr_offset = line_directives.as_ptr().add(line_directives.len() - 1);
+        let vec_ptr_offset = extra_lines.as_ptr().add(extra_lines.len() - 1);
         merge_list.push_back(&vec_ptr_offset.as_ref().unwrap()[..]);
     }
 }