diff --git a/src/diff/changes.rs b/src/diff/changes.rs index dd60280799..e682d68bcd 100644 --- a/src/diff/changes.rs +++ b/src/diff/changes.rs @@ -2,38 +2,42 @@ use crate::{ hash::DftHashMap, - parse::syntax::{Syntax, SyntaxId}, + parse::syntax::{Syntax, SyntaxId, SyntaxIdMap}, }; #[derive(PartialEq, Eq, Clone, Copy)] -pub(crate) enum ChangeKind<'a> { - Unchanged(&'a Syntax<'a>), - ReplacedComment(&'a Syntax<'a>, &'a Syntax<'a>), - ReplacedString(&'a Syntax<'a>, &'a Syntax<'a>), +pub(crate) enum ChangeKind { + Unchanged(SyntaxId), + ReplacedComment(SyntaxId, SyntaxId), + ReplacedString(SyntaxId, SyntaxId), Novel, } #[derive(Debug, Default)] -pub(crate) struct ChangeMap<'a> { - changes: DftHashMap>, +pub(crate) struct ChangeMap { + changes: DftHashMap, } -impl<'a> ChangeMap<'a> { - pub(crate) fn insert(&mut self, node: &'a Syntax<'a>, ck: ChangeKind<'a>) { - self.changes.insert(node.id(), ck); +impl ChangeMap { + pub(crate) fn insert(&mut self, node: SyntaxId, ck: ChangeKind) { + self.changes.insert(node, ck); } - pub(crate) fn get(&self, node: &Syntax<'a>) -> Option> { - self.changes.get(&node.id()).copied() + pub(crate) fn get(&self, node: SyntaxId) -> Option { + self.changes.get(&node).copied() } } -pub(crate) fn insert_deep_unchanged<'a>( - node: &'a Syntax<'a>, - opposite_node: &'a Syntax<'a>, - change_map: &mut ChangeMap<'a>, +pub(crate) fn insert_deep_unchanged<'s>( + node_id: SyntaxId, + opposite_node_id: SyntaxId, + id_map: &SyntaxIdMap<'s>, + change_map: &mut ChangeMap, ) { - change_map.insert(node, ChangeKind::Unchanged(opposite_node)); + let node = id_map[&node_id]; + let opposite_node = id_map[&opposite_node_id]; + + change_map.insert(node_id, ChangeKind::Unchanged(opposite_node_id)); match (node, opposite_node) { ( @@ -47,7 +51,7 @@ pub(crate) fn insert_deep_unchanged<'a>( }, ) => { for (child, opposite_child) in node_children.iter().zip(opposite_children) { - insert_deep_unchanged(child, opposite_child, change_map); + insert_deep_unchanged(child.id(), opposite_child.id(), id_map, change_map); } } (Syntax::Atom { .. }, Syntax::Atom { .. }) => {} @@ -55,12 +59,17 @@ pub(crate) fn insert_deep_unchanged<'a>( } } -pub(crate) fn insert_deep_novel<'a>(node: &'a Syntax<'a>, change_map: &mut ChangeMap<'a>) { - change_map.insert(node, ChangeKind::Novel); +pub(crate) fn insert_deep_novel<'s>( + node_id: SyntaxId, + id_map: &SyntaxIdMap<'s>, + change_map: &mut ChangeMap, +) { + change_map.insert(node_id, ChangeKind::Novel); + let node = id_map[&node_id]; if let Syntax::List { children, .. } = node { for child in children.iter() { - insert_deep_novel(child, change_map); + insert_deep_novel(child.id(), id_map, change_map); } } } diff --git a/src/diff/dijkstra.rs b/src/diff/dijkstra.rs index 20efc41295..a0d97bb84d 100644 --- a/src/diff/dijkstra.rs +++ b/src/diff/dijkstra.rs @@ -193,7 +193,7 @@ fn tree_count(root: Option<&Syntax>) -> u32 { pub(crate) fn mark_syntax<'a>( lhs_syntax_id: Option, rhs_syntax_id: Option, - change_map: &mut ChangeMap<'a>, + change_map: &mut ChangeMap, graph_limit: usize, id_map: &DftHashMap>, ) -> Result<(), ExceededGraphLimit> { @@ -226,7 +226,7 @@ pub(crate) fn mark_syntax<'a>( // than graph_limit nodes. let size_hint = std::cmp::min(lhs_node_count * rhs_node_count, graph_limit); - let start = Vertex::new(lhs_syntax, rhs_syntax); + let start = Vertex::new(lhs_syntax.map(|n| n.id()), rhs_syntax.map(|n| n.id())); let vertex_arena = Bump::new(); let route = shortest_path(start, &vertex_arena, size_hint, graph_limit, id_map)?; @@ -245,9 +245,9 @@ pub(crate) fn mark_syntax<'a>( format!( "{:20} {:20} --- {:3} {:?}", v.lhs_syntax - .map_or_else(|| "None".into(), Syntax::dbg_content), + .map_or_else(|| "None".into(), |id| Syntax::dbg_content(id_map[&id])), v.rhs_syntax - .map_or_else(|| "None".into(), Syntax::dbg_content), + .map_or_else(|| "None".into(), |id| Syntax::dbg_content(id_map[&id])), edge.cost(), edge, ) @@ -293,7 +293,7 @@ mod tests { let id_map = build_id_map(&[lhs], &[rhs]); - let start = Vertex::new(Some(lhs), Some(rhs)); + let start = Vertex::new(Some(lhs.id()), Some(rhs.id())); let vertex_arena = Bump::new(); let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT, &id_map).unwrap(); @@ -337,7 +337,10 @@ mod tests { let id_map = build_id_map(&lhs, &rhs); - let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied()); + let start = Vertex::new( + lhs.get(0).copied().map(|n| n.id()), + rhs.get(0).copied().map(|n| n.id()), + ); let vertex_arena = Bump::new(); let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT, &id_map).unwrap(); @@ -381,7 +384,10 @@ mod tests { let id_map = build_id_map(&lhs, &rhs); - let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied()); + let start = Vertex::new( + lhs.get(0).copied().map(|n| n.id()), + rhs.get(0).copied().map(|n| n.id()), + ); let vertex_arena = Bump::new(); let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT, &id_map).unwrap(); @@ -429,7 +435,10 @@ mod tests { let id_map = build_id_map(&lhs, &rhs); - let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied()); + let start = Vertex::new( + lhs.get(0).copied().map(|n| n.id()), + rhs.get(0).copied().map(|n| n.id()), + ); let vertex_arena = Bump::new(); let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT, &id_map).unwrap(); @@ -472,7 +481,10 @@ mod tests { let id_map = build_id_map(&lhs, &rhs); - let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied()); + let start = Vertex::new( + lhs.get(0).copied().map(|n| n.id()), + rhs.get(0).copied().map(|n| n.id()), + ); let vertex_arena = Bump::new(); let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT, &id_map).unwrap(); @@ -506,7 +518,10 @@ mod tests { let id_map = build_id_map(&lhs, &rhs); - let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied()); + let start = Vertex::new( + lhs.get(0).copied().map(|n| n.id()), + rhs.get(0).copied().map(|n| n.id()), + ); let vertex_arena = Bump::new(); let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT, &id_map).unwrap(); @@ -548,7 +563,10 @@ mod tests { let id_map = build_id_map(&lhs, &rhs); - let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied()); + let start = Vertex::new( + lhs.get(0).copied().map(|n| n.id()), + rhs.get(0).copied().map(|n| n.id()), + ); let vertex_arena = Bump::new(); let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT, &id_map).unwrap(); @@ -583,8 +601,14 @@ mod tests { ) .unwrap(); - assert_eq!(change_map.get(lhs), Some(ChangeKind::Unchanged(rhs))); - assert_eq!(change_map.get(rhs), Some(ChangeKind::Unchanged(lhs))); + assert_eq!( + change_map.get(lhs.id()), + Some(ChangeKind::Unchanged(rhs.id())) + ); + assert_eq!( + change_map.get(rhs.id()), + Some(ChangeKind::Unchanged(lhs.id())) + ); } #[test] @@ -605,7 +629,7 @@ mod tests { &id_map, ) .unwrap(); - assert_eq!(change_map.get(lhs), Some(ChangeKind::Novel)); - assert_eq!(change_map.get(rhs), Some(ChangeKind::Novel)); + assert_eq!(change_map.get(lhs.id()), Some(ChangeKind::Novel)); + assert_eq!(change_map.get(rhs.id()), Some(ChangeKind::Novel)); } } diff --git a/src/diff/graph.rs b/src/diff/graph.rs index 7ea7ed38fc..9b73c6c43a 100644 --- a/src/diff/graph.rs +++ b/src/diff/graph.rs @@ -19,7 +19,7 @@ use crate::{ stack::Stack, }, hash::DftHashMap, - parse::syntax::{AtomKind, Syntax, SyntaxId}, + parse::syntax::{AtomKind, Syntax, SyntaxId, SyntaxIdMap}, }; /// A vertex in a directed acyclic graph that represents a diff. @@ -54,11 +54,9 @@ use crate::{ pub(crate) struct Vertex<'s, 'b> { pub(crate) neighbours: RefCell)]>>, pub(crate) predecessor: Cell)>>, - // TODO: experiment with storing SyntaxId only, and have a HashMap - // from SyntaxId to &Syntax. - pub(crate) lhs_syntax: Option<&'s Syntax<'s>>, - pub(crate) rhs_syntax: Option<&'s Syntax<'s>>, - parents: Stack<'b, EnteredDelimiter<'s, 'b>>, + pub(crate) lhs_syntax: Option, + pub(crate) rhs_syntax: Option, + parents: Stack<'b, EnteredDelimiter<'b>>, lhs_parent_id: Option, rhs_parent_id: Option, } @@ -82,12 +80,12 @@ impl<'s, 'b> PartialEq for Vertex<'s, 'b> { // more vertices to be distinct, exponentially increasing // the graph size relative to tree depth. let b0 = match (self.lhs_syntax, other.lhs_syntax) { - (Some(s0), Some(s1)) => s0.id() == s1.id(), + (Some(s0), Some(s1)) => s0 == s1, (None, None) => self.lhs_parent_id == other.lhs_parent_id, _ => false, }; let b1 = match (self.rhs_syntax, other.rhs_syntax) { - (Some(s0), Some(s1)) => s0.id() == s1.id(), + (Some(s0), Some(s1)) => s0 == s1, (None, None) => self.rhs_parent_id == other.rhs_parent_id, _ => false, }; @@ -105,8 +103,8 @@ impl<'s, 'b> Eq for Vertex<'s, 'b> {} impl<'s, 'b> Hash for Vertex<'s, 'b> { fn hash(&self, state: &mut H) { - self.lhs_syntax.map(|node| node.id()).hash(state); - self.rhs_syntax.map(|node| node.id()).hash(state); + self.lhs_syntax.hash(state); + self.rhs_syntax.hash(state); self.lhs_parent_id.hash(state); self.rhs_parent_id.hash(state); @@ -116,12 +114,12 @@ impl<'s, 'b> Hash for Vertex<'s, 'b> { /// Tracks entering syntax List nodes. #[derive(Clone, PartialEq)] -enum EnteredDelimiter<'s, 'b> { +enum EnteredDelimiter<'b> { /// If we've entered the LHS or RHS separately, we can pop either /// side independently. /// /// Assumes that at least one stack is non-empty. - PopEither((Stack<'b, &'s Syntax<'s>>, Stack<'b, &'s Syntax<'s>>)), + PopEither((Stack<'b, SyntaxId>, Stack<'b, SyntaxId>)), /// If we've entered the LHS and RHS together, we must pop both /// sides together too. Otherwise we'd consider the following case to have no changes. /// @@ -129,10 +127,10 @@ enum EnteredDelimiter<'s, 'b> { /// Old: (a b c) /// New: (a b) c /// ``` - PopBoth((&'s Syntax<'s>, &'s Syntax<'s>)), + PopBoth((SyntaxId, SyntaxId)), } -impl<'s, 'b> fmt::Debug for EnteredDelimiter<'s, 'b> { +impl<'b> fmt::Debug for EnteredDelimiter<'b> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let desc = match self { EnteredDelimiter::PopEither((lhs_delims, rhs_delims)) => { @@ -148,12 +146,12 @@ impl<'s, 'b> fmt::Debug for EnteredDelimiter<'s, 'b> { } } -fn push_both_delimiters<'s, 'b>( - entered: &Stack<'b, EnteredDelimiter<'s, 'b>>, - lhs_delim: &'s Syntax<'s>, - rhs_delim: &'s Syntax<'s>, +fn push_both_delimiters<'b>( + entered: &Stack<'b, EnteredDelimiter<'b>>, + lhs_delim: SyntaxId, + rhs_delim: SyntaxId, alloc: &'b Bump, -) -> Stack<'b, EnteredDelimiter<'s, 'b>> { +) -> Stack<'b, EnteredDelimiter<'b>> { entered.push(EnteredDelimiter::PopBoth((lhs_delim, rhs_delim)), alloc) } @@ -161,25 +159,21 @@ fn can_pop_either_parent(entered: &Stack) -> bool { matches!(entered.peek(), Some(EnteredDelimiter::PopEither(_))) } -fn try_pop_both<'s, 'b>( - entered: &Stack<'b, EnteredDelimiter<'s, 'b>>, -) -> Option<( - &'s Syntax<'s>, - &'s Syntax<'s>, - Stack<'b, EnteredDelimiter<'s, 'b>>, -)> { +fn try_pop_both<'b>( + entered: &Stack<'b, EnteredDelimiter<'b>>, +) -> Option<(SyntaxId, SyntaxId, Stack<'b, EnteredDelimiter<'b>>)> { match entered.peek() { Some(EnteredDelimiter::PopBoth((lhs_delim, rhs_delim))) => { - Some((lhs_delim, rhs_delim, entered.pop().unwrap())) + Some((*lhs_delim, *rhs_delim, entered.pop().unwrap())) } _ => None, } } -fn try_pop_lhs<'s, 'b>( - entered: &Stack<'b, EnteredDelimiter<'s, 'b>>, +fn try_pop_lhs<'b>( + entered: &Stack<'b, EnteredDelimiter<'b>>, alloc: &'b Bump, -) -> Option<(&'s Syntax<'s>, Stack<'b, EnteredDelimiter<'s, 'b>>)> { +) -> Option<(SyntaxId, Stack<'b, EnteredDelimiter<'b>>)> { match entered.peek() { Some(EnteredDelimiter::PopEither((lhs_delims, rhs_delims))) => match lhs_delims.peek() { Some(lhs_delim) => { @@ -193,7 +187,7 @@ fn try_pop_lhs<'s, 'b>( ); } - Some((lhs_delim, entered)) + Some((*lhs_delim, entered)) } None => None, }, @@ -201,10 +195,10 @@ fn try_pop_lhs<'s, 'b>( } } -fn try_pop_rhs<'s, 'b>( - entered: &Stack<'b, EnteredDelimiter<'s, 'b>>, +fn try_pop_rhs<'b>( + entered: &Stack<'b, EnteredDelimiter<'b>>, alloc: &'b Bump, -) -> Option<(&'s Syntax<'s>, Stack<'b, EnteredDelimiter<'s, 'b>>)> { +) -> Option<(SyntaxId, Stack<'b, EnteredDelimiter<'b>>)> { match entered.peek() { Some(EnteredDelimiter::PopEither((lhs_delims, rhs_delims))) => match rhs_delims.peek() { Some(rhs_delim) => { @@ -218,7 +212,7 @@ fn try_pop_rhs<'s, 'b>( ); } - Some((rhs_delim, entered)) + Some((*rhs_delim, entered)) } None => None, }, @@ -226,11 +220,11 @@ fn try_pop_rhs<'s, 'b>( } } -fn push_lhs_delimiter<'s, 'b>( - entered: &Stack<'b, EnteredDelimiter<'s, 'b>>, - delimiter: &'s Syntax<'s>, +fn push_lhs_delimiter<'b>( + entered: &Stack<'b, EnteredDelimiter<'b>>, + delimiter: SyntaxId, alloc: &'b Bump, -) -> Stack<'b, EnteredDelimiter<'s, 'b>> { +) -> Stack<'b, EnteredDelimiter<'b>> { match entered.peek() { Some(EnteredDelimiter::PopEither((lhs_delims, rhs_delims))) => entered.pop().unwrap().push( EnteredDelimiter::PopEither((lhs_delims.push(delimiter, alloc), rhs_delims.clone())), @@ -243,11 +237,11 @@ fn push_lhs_delimiter<'s, 'b>( } } -fn push_rhs_delimiter<'s, 'b>( - entered: &Stack<'b, EnteredDelimiter<'s, 'b>>, - delimiter: &'s Syntax<'s>, +fn push_rhs_delimiter<'b>( + entered: &Stack<'b, EnteredDelimiter<'b>>, + delimiter: SyntaxId, alloc: &'b Bump, -) -> Stack<'b, EnteredDelimiter<'s, 'b>> { +) -> Stack<'b, EnteredDelimiter<'b>> { match entered.peek() { Some(EnteredDelimiter::PopEither((lhs_delims, rhs_delims))) => entered.pop().unwrap().push( EnteredDelimiter::PopEither((lhs_delims.clone(), rhs_delims.push(delimiter, alloc))), @@ -265,10 +259,7 @@ impl<'s, 'b> Vertex<'s, 'b> { self.lhs_syntax.is_none() && self.rhs_syntax.is_none() && self.parents.is_empty() } - pub(crate) fn new( - lhs_syntax: Option<&'s Syntax<'s>>, - rhs_syntax: Option<&'s Syntax<'s>>, - ) -> Self { + pub(crate) fn new(lhs_syntax: Option, rhs_syntax: Option) -> Self { let parents = Stack::new(); Vertex { neighbours: RefCell::new(None), @@ -426,59 +417,64 @@ fn looks_like_punctuation(node: &Syntax) -> bool { /// Pop as many parents of `lhs_node` and `rhs_node` as /// possible. Return the new syntax nodes and parents. fn pop_all_parents<'s, 'b>( - lhs_node: Option<&'s Syntax<'s>>, - rhs_node: Option<&'s Syntax<'s>>, + lhs_node_id: Option, + rhs_node_id: Option, lhs_parent_id: Option, rhs_parent_id: Option, - parents: &Stack<'b, EnteredDelimiter<'s, 'b>>, + parents: &Stack<'b, EnteredDelimiter<'b>>, alloc: &'b Bump, - id_map: &DftHashMap>, + id_map: &SyntaxIdMap<'s>, ) -> ( - Option<&'s Syntax<'s>>, - Option<&'s Syntax<'s>>, Option, Option, - Stack<'b, EnteredDelimiter<'s, 'b>>, + Option, + Option, + Stack<'b, EnteredDelimiter<'b>>, ) { - let mut lhs_node = lhs_node; - let mut rhs_node = rhs_node; - let mut lhs_parent_id = lhs_parent_id; - let mut rhs_parent_id = rhs_parent_id; + let mut lhs_node_id: Option = lhs_node_id; + let mut rhs_node_id: Option = rhs_node_id; + let mut lhs_parent_id: Option = lhs_parent_id; + let mut rhs_parent_id: Option = rhs_parent_id; let mut parents = parents.clone(); loop { - if lhs_node.is_none() { - if let Some((lhs_parent, parents_next)) = try_pop_lhs(&parents, alloc) { + if lhs_node_id.is_none() { + if let Some((lhs_parent_id_, parents_next)) = try_pop_lhs(&parents, alloc) { + let lhs_parent = id_map[&lhs_parent_id_]; // Move to next after LHS parent. // Continue from sibling of parent. - lhs_node = lhs_parent.next_sibling(); - lhs_parent_id = lhs_parent.parent().map(Syntax::id); + lhs_node_id = lhs_parent.next_sibling().map(|n| n.id()); + lhs_parent_id = lhs_parent.parent().map(|n| n.id()); parents = parents_next; continue; } } - if rhs_node.is_none() { - if let Some((rhs_parent, parents_next)) = try_pop_rhs(&parents, alloc) { + if rhs_node_id.is_none() { + if let Some((rhs_parent_id_, parents_next)) = try_pop_rhs(&parents, alloc) { + let rhs_parent = id_map[&rhs_parent_id_]; // Move to next after RHS parent. // Continue from sibling of parent. - rhs_node = rhs_parent.next_sibling(); + rhs_node_id = rhs_parent.next_sibling().map(|n| n.id()); rhs_parent_id = rhs_parent.parent().map(Syntax::id); parents = parents_next; continue; } } - if lhs_node.is_none() && rhs_node.is_none() { + if lhs_node_id.is_none() && rhs_node_id.is_none() { // We have exhausted all the nodes on both lists, so we can // move up to the parent node. // Continue from sibling of parent. - if let Some((lhs_parent, rhs_parent, parents_next)) = try_pop_both(&parents) { - lhs_node = lhs_parent.next_sibling(); - rhs_node = rhs_parent.next_sibling(); + if let Some((lhs_parent_id_, rhs_parent_id_, parents_next)) = try_pop_both(&parents) { + let lhs_parent = id_map[&lhs_parent_id_]; + let rhs_parent = id_map[&rhs_parent_id_]; + + lhs_node_id = lhs_parent.next_sibling().map(|n| n.id()); + rhs_node_id = rhs_parent.next_sibling().map(|n| n.id()); lhs_parent_id = lhs_parent.parent().map(Syntax::id); rhs_parent_id = rhs_parent.parent().map(Syntax::id); parents = parents_next; @@ -489,7 +485,13 @@ fn pop_all_parents<'s, 'b>( break; } - (lhs_node, rhs_node, lhs_parent_id, rhs_parent_id, parents) + ( + lhs_node_id, + rhs_node_id, + lhs_parent_id, + rhs_parent_id, + parents, + ) } /// Compute the neighbours of `v` if we haven't previously done so, @@ -498,7 +500,7 @@ pub(crate) fn set_neighbours<'s, 'b>( v: &Vertex<'s, 'b>, alloc: &'b Bump, seen: &mut DftHashMap<&Vertex<'s, 'b>, SmallVec<[&'b Vertex<'s, 'b>; 2]>>, - id_map: &DftHashMap>, + id_map: &SyntaxIdMap<'s>, ) { if v.neighbours.borrow().is_some() { return; @@ -507,7 +509,10 @@ pub(crate) fn set_neighbours<'s, 'b>( // There are only seven pushes in this function, so that's sufficient. let mut neighbours: Vec<(Edge, &Vertex)> = Vec::with_capacity(7); - if let (Some(lhs_syntax), Some(rhs_syntax)) = (&v.lhs_syntax, &v.rhs_syntax) { + if let (Some(lhs_syntax_id), Some(rhs_syntax_id)) = (&v.lhs_syntax, &v.rhs_syntax) { + let lhs_syntax = id_map[lhs_syntax_id]; + let rhs_syntax = id_map[rhs_syntax_id]; + if lhs_syntax == rhs_syntax { let depth_difference = (lhs_syntax.num_ancestors() as i32 - rhs_syntax.num_ancestors() as i32) @@ -517,8 +522,8 @@ pub(crate) fn set_neighbours<'s, 'b>( // Both nodes are equal, the happy case. let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) = pop_all_parents( - lhs_syntax.next_sibling(), - rhs_syntax.next_sibling(), + lhs_syntax.next_sibling().map(|n| n.id()), + rhs_syntax.next_sibling().map(|n| n.id()), v.lhs_parent_id, v.rhs_parent_id, &v.parents, @@ -568,7 +573,8 @@ pub(crate) fn set_neighbours<'s, 'b>( let rhs_next = rhs_children.first().copied(); // TODO: be consistent between parents_next and next_parents. - let parents_next = push_both_delimiters(&v.parents, lhs_syntax, rhs_syntax, alloc); + let parents_next = + push_both_delimiters(&v.parents, lhs_syntax.id(), rhs_syntax.id(), alloc); let depth_difference = (lhs_syntax.num_ancestors() as i32 - rhs_syntax.num_ancestors() as i32) @@ -578,8 +584,8 @@ pub(crate) fn set_neighbours<'s, 'b>( // pop several levels if the list has no children. let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) = pop_all_parents( - lhs_next, - rhs_next, + lhs_next.map(|n| n.id()), + rhs_next.map(|n| n.id()), Some(lhs_syntax.id()), Some(rhs_syntax.id()), &parents_next, @@ -632,8 +638,8 @@ pub(crate) fn set_neighbours<'s, 'b>( let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) = pop_all_parents( - lhs_syntax.next_sibling(), - rhs_syntax.next_sibling(), + lhs_syntax.next_sibling().map(|n| n.id()), + rhs_syntax.next_sibling().map(|n| n.id()), v.lhs_parent_id, v.rhs_parent_id, &v.parents, @@ -660,13 +666,14 @@ pub(crate) fn set_neighbours<'s, 'b>( } } - if let Some(lhs_syntax) = &v.lhs_syntax { + if let Some(lhs_syntax_id) = &v.lhs_syntax { + let lhs_syntax = id_map[lhs_syntax_id]; match lhs_syntax { // Step over this novel atom. Syntax::Atom { .. } => { let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) = pop_all_parents( - lhs_syntax.next_sibling(), + lhs_syntax.next_sibling().map(|n| n.id()), v.rhs_syntax, v.lhs_parent_id, v.rhs_parent_id, @@ -696,11 +703,11 @@ pub(crate) fn set_neighbours<'s, 'b>( Syntax::List { children, .. } => { let lhs_next = children.first().copied(); - let parents_next = push_lhs_delimiter(&v.parents, lhs_syntax, alloc); + let parents_next = push_lhs_delimiter(&v.parents, lhs_syntax.id(), alloc); let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) = pop_all_parents( - lhs_next, + lhs_next.map(|n| n.id()), v.rhs_syntax, Some(lhs_syntax.id()), v.rhs_parent_id, @@ -729,14 +736,15 @@ pub(crate) fn set_neighbours<'s, 'b>( } } - if let Some(rhs_syntax) = &v.rhs_syntax { + if let Some(rhs_syntax_id) = &v.rhs_syntax { + let rhs_syntax = id_map[rhs_syntax_id]; match rhs_syntax { // Step over this novel atom. Syntax::Atom { .. } => { let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) = pop_all_parents( v.lhs_syntax, - rhs_syntax.next_sibling(), + rhs_syntax.next_sibling().map(|n| n.id()), v.lhs_parent_id, v.rhs_parent_id, &v.parents, @@ -764,12 +772,12 @@ pub(crate) fn set_neighbours<'s, 'b>( // Step into this partially/fully novel list. Syntax::List { children, .. } => { let rhs_next = children.first().copied(); - let parents_next = push_rhs_delimiter(&v.parents, rhs_syntax, alloc); + let parents_next = push_rhs_delimiter(&v.parents, rhs_syntax.id(), alloc); let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) = pop_all_parents( v.lhs_syntax, - rhs_next, + rhs_next.map(|n| n.id()), v.lhs_parent_id, Some(rhs_syntax.id()), &parents_next, @@ -807,8 +815,8 @@ pub(crate) fn set_neighbours<'s, 'b>( pub(crate) fn populate_change_map<'s, 'b>( route: &[(Edge, &'b Vertex<'s, 'b>)], - change_map: &mut ChangeMap<'s>, - id_map: &DftHashMap>, + change_map: &mut ChangeMap, + id_map: &SyntaxIdMap<'s>, ) { for (e, v) in route { match e { @@ -817,8 +825,8 @@ pub(crate) fn populate_change_map<'s, 'b>( let lhs = v.lhs_syntax.unwrap(); let rhs = v.rhs_syntax.unwrap(); - insert_deep_unchanged(lhs, rhs, change_map); - insert_deep_unchanged(rhs, lhs, change_map); + insert_deep_unchanged(lhs, rhs, id_map, change_map); + insert_deep_unchanged(rhs, lhs, id_map, change_map); } EnterUnchangedDelimiter { .. } => { // No change on the outer delimiter, but children may diff --git a/src/diff/sliders.rs b/src/diff/sliders.rs index cd3da98929..fb9aa42533 100644 --- a/src/diff/sliders.rs +++ b/src/diff/sliders.rs @@ -33,20 +33,26 @@ use line_numbers::SingleLineSpan; use crate::{ diff::changes::{insert_deep_novel, insert_deep_unchanged, ChangeKind::*, ChangeMap}, - parse::guess_language, - parse::syntax::Syntax::{self, *}, + parse::{ + guess_language, + syntax::{ + Syntax::{self, *}, + SyntaxIdMap, + }, + }, }; pub(crate) fn fix_all_sliders<'a>( language: guess_language::Language, nodes: &[&'a Syntax<'a>], - change_map: &mut ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, ) { // TODO: fix sliders that require more than two steps. - fix_all_sliders_one_step(nodes, change_map); - fix_all_sliders_one_step(nodes, change_map); + fix_all_sliders_one_step(nodes, id_map, change_map); + fix_all_sliders_one_step(nodes, id_map, change_map); - fix_all_nested_sliders(language, nodes, change_map); + fix_all_nested_sliders(language, nodes, id_map, change_map); } /// Should nester slider correction prefer the inner or outer @@ -72,13 +78,17 @@ fn prefer_outer_delimiter(language: guess_language::Language) -> bool { } } -fn fix_all_sliders_one_step<'a>(nodes: &[&'a Syntax<'a>], change_map: &mut ChangeMap<'a>) { +fn fix_all_sliders_one_step<'a>( + nodes: &[&'a Syntax<'a>], + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, +) { for node in nodes { if let List { children, .. } = node { - fix_all_sliders_one_step(children, change_map); + fix_all_sliders_one_step(children, id_map, change_map); } } - fix_sliders(nodes, change_map); + fix_sliders(nodes, id_map, change_map); } /// Correct sliders in middle insertions. @@ -100,7 +110,8 @@ fn fix_all_sliders_one_step<'a>(nodes: &[&'a Syntax<'a>], change_map: &mut Chang fn fix_all_nested_sliders<'a>( language: guess_language::Language, nodes: &[&'a Syntax<'a>], - change_map: &mut ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, ) { let prefer_outer = prefer_outer_delimiter(language); for node in nodes { @@ -115,10 +126,10 @@ fn fix_all_nested_sliders<'a>( /// When we see code of the form `(old-1 (novel (old-2)))`, prefer /// treating the outer delimiter as novel, so `(novel ...)` in this /// example. -fn fix_nested_slider_prefer_outer<'a>(node: &'a Syntax<'a>, change_map: &mut ChangeMap<'a>) { +fn fix_nested_slider_prefer_outer<'a>(node: &'a Syntax<'a>, change_map: &mut ChangeMap) { if let List { children, .. } = node { match change_map - .get(node) + .get(node.id()) .expect("Changes should be set before slider correction") { Unchanged(_) => { @@ -130,7 +141,7 @@ fn fix_nested_slider_prefer_outer<'a>(node: &'a Syntax<'a>, change_map: &mut Cha // list has novel delimiters. if let [candidate] = candidates[..] { if matches!(candidate, List { .. }) - && matches!(change_map.get(candidate), Some(Novel)) + && matches!(change_map.get(candidate.id()), Some(Novel)) { push_unchanged_to_descendant(node, candidate, change_map); } @@ -148,10 +159,10 @@ fn fix_nested_slider_prefer_outer<'a>(node: &'a Syntax<'a>, change_map: &mut Cha /// When we see code of the form `old1(novel(old2()))`, prefer /// treating the inner delimiter as novel, so `novel(...)` in this /// example. -fn fix_nested_slider_prefer_inner<'a>(node: &'a Syntax<'a>, change_map: &mut ChangeMap<'a>) { +fn fix_nested_slider_prefer_inner<'a>(node: &'a Syntax<'a>, change_map: &mut ChangeMap) { if let List { children, .. } = node { match change_map - .get(node) + .get(node.id()) .expect("Changes should be set before slider correction") { Unchanged(_) => {} @@ -176,7 +187,7 @@ fn fix_nested_slider_prefer_inner<'a>(node: &'a Syntax<'a>, change_map: &mut Cha fn unchanged_descendants<'a>( nodes: &[&'a Syntax<'a>], found: &mut Vec<&'a Syntax<'a>>, - change_map: &ChangeMap<'a>, + change_map: &ChangeMap, ) { // We're only interested if there is exactly one unchanged // descendant, so return early if we find 2 or more. @@ -185,7 +196,7 @@ fn unchanged_descendants<'a>( } for node in nodes { - match change_map.get(node).unwrap() { + match change_map.get(node.id()).unwrap() { Unchanged(_) => { found.push(node); } @@ -212,7 +223,7 @@ fn unchanged_descendants<'a>( fn unchanged_descendants_for_outer_slider<'a>( nodes: &[&'a Syntax<'a>], found: &mut Vec<&'a Syntax<'a>>, - change_map: &ChangeMap<'a>, + change_map: &ChangeMap, ) { // We're only interested if there is exactly one unchanged // descendant, so return early if we find 2 or more. @@ -221,7 +232,7 @@ fn unchanged_descendants_for_outer_slider<'a>( } for node in nodes { - let is_unchanged = matches!(change_map.get(node), Some(Unchanged(_))); + let is_unchanged = matches!(change_map.get(node.id()), Some(Unchanged(_))); match node { Atom { .. } => { @@ -256,7 +267,7 @@ fn unchanged_descendants_for_outer_slider<'a>( let has_unchanged_children = children .iter() - .any(|node| matches!(change_map.get(node), Some(Unchanged(_)))); + .any(|node| matches!(change_map.get(node.id()), Some(Unchanged(_)))); if has_unchanged_children { // The list has unchanged children and novel // delimiters. This is a candidate for @@ -280,10 +291,10 @@ fn unchanged_descendants_for_outer_slider<'a>( fn push_unchanged_to_descendant<'a>( root: &'a Syntax<'a>, inner: &'a Syntax<'a>, - change_map: &mut ChangeMap<'a>, + change_map: &mut ChangeMap, ) { let root_change = change_map - .get(root) + .get(root.id()) .expect("Changes should be set before slider correction"); let delimiters_match = match (root, inner) { @@ -303,8 +314,8 @@ fn push_unchanged_to_descendant<'a>( }; if delimiters_match { - change_map.insert(root, Novel); - change_map.insert(inner, root_change); + change_map.insert(root.id(), Novel); + change_map.insert(inner.id(), root_change); } } @@ -314,9 +325,11 @@ fn push_unchanged_to_descendant<'a>( fn push_unchanged_to_ancestor<'a>( root: &'a Syntax<'a>, inner: &'a Syntax<'a>, - change_map: &mut ChangeMap<'a>, + change_map: &mut ChangeMap, ) { - let inner_change = change_map.get(inner).expect("Node changes should be set"); + let inner_change = change_map + .get(inner.id()) + .expect("Node changes should be set"); let delimiters_match = match (root, inner) { ( @@ -335,20 +348,20 @@ fn push_unchanged_to_ancestor<'a>( }; if delimiters_match { - change_map.insert(root, inner_change); - change_map.insert(inner, Novel); + change_map.insert(root.id(), inner_change); + change_map.insert(inner.id(), Novel); } } /// For every sequence of novel nodes, if it's a potential slider, /// change which nodes are marked as novel if it produces a sequence /// of nodes that are closer together. -fn fix_sliders<'a>(nodes: &[&'a Syntax<'a>], change_map: &mut ChangeMap<'a>) { +fn fix_sliders<'a>(nodes: &[&'a Syntax<'a>], id_map: &SyntaxIdMap<'a>, change_map: &mut ChangeMap) { for (region_start, region_end) in novel_regions_after_unchanged(nodes, change_map) { - slide_to_prev_node(nodes, change_map, region_start, region_end); + slide_to_prev_node(nodes, id_map, change_map, region_start, region_end); } for (region_start, region_end) in novel_regions_before_unchanged(nodes, change_map) { - slide_to_next_node(nodes, change_map, region_start, region_end); + slide_to_next_node(nodes, id_map, change_map, region_start, region_end); } } @@ -356,13 +369,15 @@ fn fix_sliders<'a>(nodes: &[&'a Syntax<'a>], change_map: &mut ChangeMap<'a>) { /// occur after unchanged nodes. fn novel_regions_after_unchanged<'a>( nodes: &[&'a Syntax<'a>], - change_map: &ChangeMap<'a>, + change_map: &ChangeMap, ) -> Vec<(usize, usize)> { let mut regions: Vec> = vec![]; let mut region: Option> = None; for (i, node) in nodes.iter().enumerate() { - let change = change_map.get(node).expect("Node changes should be set"); + let change = change_map + .get(node.id()) + .expect("Node changes should be set"); match change { Unchanged(_) => { @@ -406,13 +421,15 @@ fn novel_regions_after_unchanged<'a>( /// occur before unchanged nodes. fn novel_regions_before_unchanged<'a>( nodes: &[&'a Syntax<'a>], - change_map: &ChangeMap<'a>, + change_map: &ChangeMap, ) -> Vec<(usize, usize)> { let mut regions: Vec> = vec![]; let mut region: Option> = None; for (i, node) in nodes.iter().enumerate() { - let change = change_map.get(node).expect("Node changes should be set"); + let change = change_map + .get(node.id()) + .expect("Node changes should be set"); match change { Unchanged(_) => { @@ -445,10 +462,10 @@ fn novel_regions_before_unchanged<'a>( .collect() } -fn is_novel_deep<'a>(node: &Syntax<'a>, change_map: &ChangeMap<'a>) -> bool { +fn is_novel_deep<'a>(node: &Syntax<'a>, change_map: &ChangeMap) -> bool { match node { List { children, .. } => { - if !matches!(change_map.get(node), Some(Novel)) { + if !matches!(change_map.get(node.id()), Some(Novel)) { return false; } for child in children { @@ -459,7 +476,7 @@ fn is_novel_deep<'a>(node: &Syntax<'a>, change_map: &ChangeMap<'a>) -> bool { true } - Atom { .. } => matches!(change_map.get(node), Some(Novel)), + Atom { .. } => matches!(change_map.get(node.id()), Some(Novel)), } } @@ -483,7 +500,8 @@ fn is_novel_deep<'a>(node: &Syntax<'a>, change_map: &ChangeMap<'a>) -> bool { /// ``` fn slide_to_prev_node<'a>( nodes: &[&'a Syntax<'a>], - change_map: &mut ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, start_idx: usize, end_idx: usize, ) { @@ -508,10 +526,11 @@ fn slide_to_prev_node<'a>( if distance_to_before_start <= distance_to_last { let opposite = match change_map - .get(before_start_node) + .get(before_start_node.id()) .expect("Node changes should be set") { - Unchanged(n) => { + Unchanged(id) => { + let n = id_map[&id]; if before_start_node.content_id() != n.content_id() { return; } @@ -528,9 +547,9 @@ fn slide_to_prev_node<'a>( } } - insert_deep_novel(before_start_node, change_map); - insert_deep_unchanged(last_node, opposite, change_map); - insert_deep_unchanged(opposite, last_node, change_map); + insert_deep_novel(before_start_node.id(), id_map, change_map); + insert_deep_unchanged(last_node.id(), opposite.id(), id_map, change_map); + insert_deep_unchanged(opposite.id(), last_node.id(), id_map, change_map); } } @@ -554,7 +573,8 @@ fn slide_to_prev_node<'a>( /// ``` fn slide_to_next_node<'a>( nodes: &[&'a Syntax<'a>], - change_map: &mut ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, start_idx: usize, end_idx: usize, ) { @@ -579,10 +599,11 @@ fn slide_to_next_node<'a>( if distance_to_after_last < distance_to_start { let opposite = match change_map - .get(after_last_node) + .get(after_last_node.id()) .expect("Node changes should be set") { - Unchanged(n) => { + Unchanged(id) => { + let n = id_map[&id]; if after_last_node.content_id() != n.content_id() { return; } @@ -598,9 +619,9 @@ fn slide_to_next_node<'a>( } } - insert_deep_unchanged(start_node, opposite, change_map); - insert_deep_unchanged(opposite, start_node, change_map); - insert_deep_novel(after_last_node, change_map); + insert_deep_unchanged(start_node.id(), opposite.id(), id_map, change_map); + insert_deep_unchanged(opposite.id(), start_node.id(), id_map, change_map); + insert_deep_novel(after_last_node.id(), id_map, change_map); } } @@ -676,8 +697,11 @@ mod tests { use super::*; use crate::{ - parse::guess_language, - parse::tree_sitter_parser::{from_language, parse}, + parse::{ + guess_language, + syntax::build_id_map, + tree_sitter_parser::{from_language, parse}, + }, syntax::{init_all_info, AtomKind}, }; @@ -717,17 +741,23 @@ mod tests { let rhs = [Syntax::new_atom(&arena, pos, "a", AtomKind::Comment)]; init_all_info(&lhs, &rhs); + let id_map = build_id_map(&lhs, &rhs); let mut change_map = ChangeMap::default(); - change_map.insert(lhs[0], Unchanged(rhs[0])); - change_map.insert(lhs[1], Novel); - change_map.insert(lhs[2], Novel); - - fix_all_sliders(guess_language::Language::EmacsLisp, &lhs, &mut change_map); - assert_eq!(change_map.get(lhs[0]), Some(Novel)); - assert_eq!(change_map.get(lhs[1]), Some(Novel)); - assert_eq!(change_map.get(lhs[2]), Some(Unchanged(rhs[0]))); - assert_eq!(change_map.get(rhs[0]), Some(Unchanged(lhs[2]))); + change_map.insert(lhs[0].id(), Unchanged(rhs[0].id())); + change_map.insert(lhs[1].id(), Novel); + change_map.insert(lhs[2].id(), Novel); + + fix_all_sliders( + guess_language::Language::EmacsLisp, + &lhs, + &id_map, + &mut change_map, + ); + assert_eq!(change_map.get(lhs[0].id()), Some(Novel)); + assert_eq!(change_map.get(lhs[1].id()), Some(Novel)); + assert_eq!(change_map.get(lhs[2].id()), Some(Unchanged(rhs[0].id()))); + assert_eq!(change_map.get(rhs[0].id()), Some(Unchanged(lhs[2].id()))); } /// Test that we slide at the end if the unchanged node is @@ -766,18 +796,24 @@ mod tests { let rhs = [Syntax::new_atom(&arena, pos, "a", AtomKind::Comment)]; init_all_info(&lhs, &rhs); + let id_map = build_id_map(&lhs, &rhs); let mut change_map = ChangeMap::default(); - change_map.insert(lhs[0], Novel); - change_map.insert(lhs[1], Novel); - change_map.insert(lhs[2], Unchanged(rhs[0])); - - fix_all_sliders(guess_language::Language::EmacsLisp, &lhs, &mut change_map); + change_map.insert(lhs[0].id(), Novel); + change_map.insert(lhs[1].id(), Novel); + change_map.insert(lhs[2].id(), Unchanged(rhs[0].id())); + + fix_all_sliders( + guess_language::Language::EmacsLisp, + &lhs, + &id_map, + &mut change_map, + ); - assert_eq!(change_map.get(rhs[0]), Some(Unchanged(lhs[0]))); - assert_eq!(change_map.get(lhs[0]), Some(Unchanged(rhs[0]))); - assert_eq!(change_map.get(lhs[1]), Some(Novel)); - assert_eq!(change_map.get(lhs[2]), Some(Novel)); + assert_eq!(change_map.get(rhs[0].id()), Some(Unchanged(lhs[0].id()))); + assert_eq!(change_map.get(lhs[0].id()), Some(Unchanged(rhs[0].id()))); + assert_eq!(change_map.get(lhs[1].id()), Some(Novel)); + assert_eq!(change_map.get(lhs[2].id()), Some(Novel)); } #[test] @@ -788,19 +824,25 @@ mod tests { let lhs = parse(&arena, "A B", &config, false); let rhs = parse(&arena, "A B X\n A B", &config, false); init_all_info(&lhs, &rhs); + let id_map = build_id_map(&lhs, &rhs); let mut change_map = ChangeMap::default(); - change_map.insert(rhs[0], Unchanged(lhs[0])); - change_map.insert(rhs[1], Unchanged(lhs[1])); - change_map.insert(rhs[2], Novel); - change_map.insert(rhs[3], Novel); - change_map.insert(rhs[4], Novel); - - fix_all_sliders(guess_language::Language::EmacsLisp, &rhs, &mut change_map); - assert_eq!(change_map.get(rhs[0]), Some(Novel)); - assert_eq!(change_map.get(rhs[1]), Some(Novel)); - assert_eq!(change_map.get(rhs[2]), Some(Novel)); - assert_eq!(change_map.get(rhs[3]), Some(Unchanged(rhs[0]))); + change_map.insert(rhs[0].id(), Unchanged(lhs[0].id())); + change_map.insert(rhs[1].id(), Unchanged(lhs[1].id())); + change_map.insert(rhs[2].id(), Novel); + change_map.insert(rhs[3].id(), Novel); + change_map.insert(rhs[4].id(), Novel); + + fix_all_sliders( + guess_language::Language::EmacsLisp, + &rhs, + &id_map, + &mut change_map, + ); + assert_eq!(change_map.get(rhs[0].id()), Some(Novel)); + assert_eq!(change_map.get(rhs[1].id()), Some(Novel)); + assert_eq!(change_map.get(rhs[2].id()), Some(Novel)); + assert_eq!(change_map.get(rhs[3].id()), Some(Unchanged(rhs[0].id()))); } /// If a list is partially unchanged but contains some novel @@ -824,16 +866,16 @@ mod tests { }; let mut change_map = ChangeMap::default(); - change_map.insert(lhs[0], Unchanged(rhs[0])); - change_map.insert(lhs[1], Novel); - insert_deep_novel(lhs[2], &mut change_map); + change_map.insert(lhs[0].id(), Unchanged(rhs[0])); + change_map.insert(lhs[1].id(), Novel); + insert_deep_novel(lhs[2].id(), &mut change_map); change_map.insert( - lhs_first_list_children[0], + lhs_first_list_children[0].id(), Unchanged(rhs_first_list_children[1]), ); change_map.insert( - lhs_first_list_children[1], + lhs_first_list_children[1].id(), Unchanged(rhs_first_list_children[2]), ); diff --git a/src/diff/unchanged.rs b/src/diff/unchanged.rs index 02198d9b75..71f73ae084 100644 --- a/src/diff/unchanged.rs +++ b/src/diff/unchanged.rs @@ -6,7 +6,7 @@ use std::hash::Hash; use crate::diff::changes::{insert_deep_unchanged, ChangeKind, ChangeMap}; use crate::diff::myers_diff; -use crate::parse::syntax::Syntax; +use crate::parse::syntax::{Syntax, SyntaxIdMap}; const TINY_TREE_THRESHOLD: u32 = 10; const MOSTLY_UNCHANGED_MIN_COMMON_CHILDREN: usize = 4; @@ -16,15 +16,17 @@ const MOSTLY_UNCHANGED_MIN_COMMON_CHILDREN: usize = 4; pub(crate) fn mark_unchanged<'a>( lhs_nodes: &[&'a Syntax<'a>], rhs_nodes: &[&'a Syntax<'a>], - change_map: &mut ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, ) -> Vec<(Vec<&'a Syntax<'a>>, Vec<&'a Syntax<'a>>)> { - let (_, lhs_nodes, rhs_nodes) = shrink_unchanged_at_ends(lhs_nodes, rhs_nodes, change_map); + let (_, lhs_nodes, rhs_nodes) = + shrink_unchanged_at_ends(lhs_nodes, rhs_nodes, id_map, change_map); let mut nodes_to_diff = vec![]; for (lhs_nodes, rhs_nodes) in split_mostly_unchanged_toplevel(&lhs_nodes, &rhs_nodes) { let (_, lhs_nodes, rhs_nodes) = - shrink_unchanged_at_ends(&lhs_nodes, &rhs_nodes, change_map); - nodes_to_diff.extend(split_unchanged(&lhs_nodes, &rhs_nodes, change_map)); + shrink_unchanged_at_ends(&lhs_nodes, &rhs_nodes, id_map, change_map); + nodes_to_diff.extend(split_unchanged(&lhs_nodes, &rhs_nodes, id_map, change_map)); } nodes_to_diff @@ -40,7 +42,8 @@ enum ChangeState { fn split_unchanged<'a>( lhs_nodes: &[&'a Syntax<'a>], rhs_nodes: &[&'a Syntax<'a>], - change_map: &mut ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, ) -> Vec<(Vec<&'a Syntax<'a>>, Vec<&'a Syntax<'a>>)> { let size_threshold = if let Ok(env_threshold) = std::env::var("DFT_TINY_THRESHOLD") { env_threshold @@ -61,8 +64,14 @@ fn split_unchanged<'a>( for (lhs_section_node, rhs_section_node) in lhs_section_nodes.iter().zip(rhs_section_nodes.iter()) { - change_map.insert(lhs_section_node, ChangeKind::Unchanged(rhs_section_node)); - change_map.insert(rhs_section_node, ChangeKind::Unchanged(lhs_section_node)); + change_map.insert( + lhs_section_node.id(), + ChangeKind::Unchanged(rhs_section_node.id()), + ); + change_map.insert( + rhs_section_node.id(), + ChangeKind::Unchanged(lhs_section_node.id()), + ); } } ChangeState::UnchangedNode => { @@ -70,8 +79,18 @@ fn split_unchanged<'a>( for (lhs_section_node, rhs_section_node) in lhs_section_nodes.iter().zip(rhs_section_nodes.iter()) { - insert_deep_unchanged(lhs_section_node, rhs_section_node, change_map); - insert_deep_unchanged(rhs_section_node, lhs_section_node, change_map); + insert_deep_unchanged( + lhs_section_node.id(), + rhs_section_node.id(), + id_map, + change_map, + ); + insert_deep_unchanged( + rhs_section_node.id(), + lhs_section_node.id(), + id_map, + change_map, + ); } } ChangeState::PossiblyChanged => { @@ -373,7 +392,8 @@ fn as_singleton_list_children<'a>( fn shrink_unchanged_delimiters<'a>( lhs_nodes: &[&'a Syntax<'a>], rhs_nodes: &[&'a Syntax<'a>], - change_map: &mut ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, ) -> (bool, Vec<&'a Syntax<'a>>, Vec<&'a Syntax<'a>>) { if let ( [Syntax::List { @@ -392,10 +412,10 @@ fn shrink_unchanged_delimiters<'a>( { if lhs_open == rhs_open && lhs_close == rhs_close { let (changed_later, lhs_shrunk_nodes, rhs_shrunk_nodes) = - shrink_unchanged_at_ends(lhs_children, rhs_children, change_map); + shrink_unchanged_at_ends(lhs_children, rhs_children, id_map, change_map); if changed_later { - change_map.insert(lhs_nodes[0], ChangeKind::Unchanged(rhs_nodes[0])); - change_map.insert(rhs_nodes[0], ChangeKind::Unchanged(lhs_nodes[0])); + change_map.insert(lhs_nodes[0].id(), ChangeKind::Unchanged(rhs_nodes[0].id())); + change_map.insert(rhs_nodes[0].id(), ChangeKind::Unchanged(lhs_nodes[0].id())); return (true, lhs_shrunk_nodes, rhs_shrunk_nodes); } @@ -413,7 +433,8 @@ fn shrink_unchanged_delimiters<'a>( fn shrink_unchanged_at_ends<'a>( lhs_nodes: &[&'a Syntax<'a>], rhs_nodes: &[&'a Syntax<'a>], - change_map: &mut ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &mut ChangeMap, ) -> (bool, Vec<&'a Syntax<'a>>, Vec<&'a Syntax<'a>>) { let mut lhs_nodes = lhs_nodes; let mut rhs_nodes = rhs_nodes; @@ -425,8 +446,8 @@ fn shrink_unchanged_at_ends<'a>( // file. There's no risk we split unrelated regions with a // trivial unchanged node in the middle. if lhs_node.content_id() == rhs_node.content_id() { - insert_deep_unchanged(lhs_node, rhs_node, change_map); - insert_deep_unchanged(rhs_node, lhs_node, change_map); + insert_deep_unchanged(lhs_node.id(), rhs_node.id(), id_map, change_map); + insert_deep_unchanged(rhs_node.id(), lhs_node.id(), id_map, change_map); changed = true; lhs_nodes = &lhs_nodes[1..]; @@ -438,8 +459,8 @@ fn shrink_unchanged_at_ends<'a>( while let (Some(lhs_node), Some(rhs_node)) = (lhs_nodes.last(), rhs_nodes.last()) { if lhs_node.content_id() == rhs_node.content_id() { - insert_deep_unchanged(lhs_node, rhs_node, change_map); - insert_deep_unchanged(rhs_node, lhs_node, change_map); + insert_deep_unchanged(lhs_node.id(), rhs_node.id(), id_map, change_map); + insert_deep_unchanged(rhs_node.id(), lhs_node.id(), id_map, change_map); changed = true; lhs_nodes = &lhs_nodes[..lhs_nodes.len() - 1]; @@ -451,7 +472,7 @@ fn shrink_unchanged_at_ends<'a>( if lhs_nodes.len() == 1 && rhs_nodes.len() == 1 { let (changed_later, lhs_nodes, rhs_nodes) = - shrink_unchanged_delimiters(lhs_nodes, rhs_nodes, change_map); + shrink_unchanged_delimiters(lhs_nodes, rhs_nodes, id_map, change_map); (changed || changed_later, lhs_nodes, rhs_nodes) } else { (changed, Vec::from(lhs_nodes), Vec::from(rhs_nodes)) @@ -464,8 +485,11 @@ mod tests { use super::*; use crate::{ - parse::guess_language, - parse::tree_sitter_parser::{from_language, parse}, + parse::{ + guess_language, + syntax::build_id_map, + tree_sitter_parser::{from_language, parse}, + }, syntax::init_all_info, }; @@ -478,9 +502,11 @@ mod tests { let rhs_nodes = parse(&arena, "unchanged X", &config, false); init_all_info(&lhs_nodes, &rhs_nodes); + let id_map = build_id_map(&lhs_nodes, &rhs_nodes); + let mut change_map = ChangeMap::default(); let (_, lhs_after_skip, rhs_after_skip) = - shrink_unchanged_at_ends(&lhs_nodes, &rhs_nodes, &mut change_map); + shrink_unchanged_at_ends(&lhs_nodes, &rhs_nodes, &id_map, &mut change_map); assert_eq!( change_map.get(lhs_nodes[0]), diff --git a/src/main.rs b/src/main.rs index fcdaf1df7a..c203ef3871 100644 --- a/src/main.rs +++ b/src/main.rs @@ -582,7 +582,7 @@ fn diff_file_content( let possibly_changed = if env::var("DFT_DBG_KEEP_UNCHANGED").is_ok() { vec![(lhs.clone(), rhs.clone())] } else { - unchanged::mark_unchanged(&lhs, &rhs, &mut change_map) + unchanged::mark_unchanged(&lhs, &rhs, &id_map, &mut change_map) }; let mut exceeded_graph_limit = false; @@ -617,11 +617,13 @@ fn diff_file_content( rhs_positions, ) } else { - fix_all_sliders(language, &lhs, &mut change_map); - fix_all_sliders(language, &rhs, &mut change_map); + fix_all_sliders(language, &lhs, &id_map, &mut change_map); + fix_all_sliders(language, &rhs, &id_map, &mut change_map); - let mut lhs_positions = syntax::change_positions(&lhs, &change_map); - let mut rhs_positions = syntax::change_positions(&rhs, &change_map); + let mut lhs_positions = + syntax::change_positions(&lhs, &id_map, &change_map); + let mut rhs_positions = + syntax::change_positions(&rhs, &id_map, &change_map); if diff_options.ignore_comments { let lhs_comments = diff --git a/src/parse/syntax.rs b/src/parse/syntax.rs index fedc61ae32..a4baf6f847 100644 --- a/src/parse/syntax.rs +++ b/src/parse/syntax.rs @@ -21,10 +21,10 @@ use crate::{ /// A Debug implementation that does not recurse into the /// corresponding node mentioned for Unchanged. Otherwise we will /// infinitely loop on unchanged nodes, which both point to the other. -impl<'a> fmt::Debug for ChangeKind<'a> { +impl fmt::Debug for ChangeKind { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let desc = match self { - Unchanged(node) => format!("Unchanged(ID: {})", node.id()), + Unchanged(node) => format!("Unchanged(ID: {})", node), ReplacedComment(lhs_node, rhs_node) | ReplacedString(lhs_node, rhs_node) => { let change_kind = if let ReplacedComment(_, _) = self { "ReplacedComment" @@ -34,9 +34,7 @@ impl<'a> fmt::Debug for ChangeKind<'a> { format!( "{}(lhs ID: {}, rhs ID: {})", - change_kind, - lhs_node.id(), - rhs_node.id() + change_kind, lhs_node, rhs_node ) } Novel => "Novel".to_owned(), @@ -47,6 +45,8 @@ impl<'a> fmt::Debug for ChangeKind<'a> { pub(crate) type SyntaxId = NonZeroU32; +pub(crate) type SyntaxIdMap<'a> = DftHashMap>; + /// Fields that are common to both `Syntax::List` and `Syntax::Atom`. pub(crate) struct SyntaxInfo<'a> { /// The previous node with the same parent as this one. @@ -367,7 +367,7 @@ pub(crate) fn init_all_info<'a>(lhs_roots: &[&'a Syntax<'a>], rhs_roots: &[&'a S pub(crate) fn build_id_map<'a>( lhs_roots: &[&'a Syntax<'a>], rhs_roots: &[&'a Syntax<'a>], -) -> DftHashMap> { +) -> SyntaxIdMap<'a> { let mut id_map = DftHashMap::default(); build_id_map_(lhs_roots, &mut id_map); build_id_map_(rhs_roots, &mut id_map); @@ -375,7 +375,7 @@ pub(crate) fn build_id_map<'a>( id_map } -fn build_id_map_<'a>(nodes: &[&'a Syntax<'a>], id_map: &mut DftHashMap>) { +fn build_id_map_<'a>(nodes: &[&'a Syntax<'a>], id_map: &mut SyntaxIdMap<'a>) { for node in nodes { id_map.insert(node.id(), *node); @@ -791,6 +791,7 @@ impl MatchedPos { highlight: TokenKind, pos: &[SingleLineSpan], is_close_delim: bool, + id_map: &SyntaxIdMap<'_>, ) -> Vec { // Don't create a MatchedPos for empty positions at the start // or end. We still want empty positions in the middle of @@ -799,7 +800,10 @@ impl MatchedPos { let pos = filter_empty_ends(pos); match ck { - ReplacedComment(this, opposite) | ReplacedString(this, opposite) => { + ReplacedComment(this_id, opposite_id) | ReplacedString(this_id, opposite_id) => { + let this = id_map[&this_id]; + let opposite = id_map[&opposite_id]; + let this_content = match this { List { .. } => unreachable!(), Atom { content, .. } => content, @@ -811,7 +815,8 @@ impl MatchedPos { } => (content, position), }; - let kind = if let ReplacedString(this, _) = ck { + let kind = if let ReplacedString(this_id, _) = ck { + let this = id_map[&this_id]; match this { Atom { kind: AtomKind::String(StringKind::Text), @@ -825,7 +830,8 @@ impl MatchedPos { split_atom_words(this_content, &pos, opposite_content, opposite_pos, kind) } - Unchanged(opposite) => { + Unchanged(opposite_id) => { + let opposite = id_map[&opposite_id]; let opposite_pos = match opposite { List { open_position, @@ -893,21 +899,23 @@ impl MatchedPos { /// Walk `nodes` and return a vec of all the changed positions. pub(crate) fn change_positions<'a>( nodes: &[&'a Syntax<'a>], - change_map: &ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &ChangeMap, ) -> Vec { let mut positions = Vec::new(); - change_positions_(nodes, change_map, &mut positions); + change_positions_(nodes, id_map, change_map, &mut positions); positions } fn change_positions_<'a>( nodes: &[&'a Syntax<'a>], - change_map: &ChangeMap<'a>, + id_map: &SyntaxIdMap<'a>, + change_map: &ChangeMap, positions: &mut Vec, ) { for node in nodes { let change = change_map - .get(node) + .get(node.id()) .unwrap_or_else(|| panic!("Should have changes set in all nodes: {:#?}", node)); match node { @@ -922,15 +930,17 @@ fn change_positions_<'a>( TokenKind::Delimiter, open_position, false, + id_map, )); - change_positions_(children, change_map, positions); + change_positions_(children, id_map, change_map, positions); positions.extend(MatchedPos::new( change, TokenKind::Delimiter, close_position, true, + id_map, )); } Atom { position, kind, .. } => { @@ -939,6 +949,7 @@ fn change_positions_<'a>( TokenKind::Atom(*kind), position, false, + id_map, )); } }