From 1f1c6867c7bcb337eaa843f60c712efa28dce78f Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 12 Jun 2023 04:18:25 -0700 Subject: [PATCH] conflicts: add a `simplify()` method, taken from `tree.rs` It seems generally useful to be able to simplify a conflict, and it's not specific to merging trees, so let's move it to `conflicts.rs`. Once we're done with the migration to tree-level conflicts, I think `Conflict::simplify()` will remain but `tree::simplify_conflict()` will be gone. The tests I added there are quite similar to those of `trivial_merge()`. I hope we can make `Conflict::simplify()` call `trivial_merge()` later. I think it would also make sense to move `trivial_merge()` onto `Conflict`, or at least have a `Conflict::resolve_trivial()` calling `trivial_merge()`. --- lib/src/conflicts.rs | 89 ++++++++++++++++++++++++++++++++++++++++++++ lib/src/tree.rs | 17 +-------- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 770d95711..7dab67a15 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -60,6 +60,27 @@ impl Conflict { pub fn set_add(&mut self, i: usize, value: T) { self.adds[i] = value; } + + /// Remove pairs of entries that match in the removes and adds. + pub fn simplify(mut self) -> Self + where + T: PartialEq, + { + let mut add_index = 0; + while add_index < self.adds.len() { + let add = &self.adds[add_index]; + add_index += 1; + for (remove_index, remove) in self.removes.iter().enumerate() { + if remove == add { + self.removes.remove(remove_index); + add_index -= 1; + self.adds.remove(add_index); + break; + } + } + } + self + } } impl Conflict> { @@ -644,4 +665,72 @@ mod tests { ); test_roundtrip(&backend_conflict); } + + #[test] + fn test_simplify() { + fn c(removes: &[u32], adds: &[u32]) -> Conflict { + Conflict::new(removes.to_vec(), adds.to_vec()) + } + // 1-way "conflict" + assert_eq!(c(&[], &[0]).simplify(), c(&[], &[0])); + // 3-way conflict + assert_eq!(c(&[0], &[0, 0]).simplify(), c(&[], &[0])); + assert_eq!(c(&[0], &[0, 1]).simplify(), c(&[], &[1])); + assert_eq!(c(&[0], &[1, 0]).simplify(), c(&[], &[1])); + assert_eq!(c(&[0], &[1, 1]).simplify(), c(&[0], &[1, 1])); + assert_eq!(c(&[0], &[1, 2]).simplify(), c(&[0], &[1, 2])); + // Irreducible 5-way conflict + assert_eq!(c(&[0, 0], &[0, 0, 0]).simplify(), c(&[], &[0])); + assert_eq!(c(&[0, 0], &[0, 0, 1]).simplify(), c(&[], &[1])); + assert_eq!(c(&[0, 0], &[0, 1, 0]).simplify(), c(&[], &[1])); + assert_eq!(c(&[0, 0], &[0, 1, 1]).simplify(), c(&[0], &[1, 1])); + assert_eq!(c(&[0, 0], &[0, 1, 2]).simplify(), c(&[0], &[1, 2])); + assert_eq!(c(&[0, 0], &[1, 0, 0]).simplify(), c(&[], &[1])); + assert_eq!(c(&[0, 0], &[1, 0, 1]).simplify(), c(&[0], &[1, 1])); + assert_eq!(c(&[0, 0], &[1, 0, 2]).simplify(), c(&[0], &[1, 2])); + assert_eq!(c(&[0, 0], &[1, 1, 0]).simplify(), c(&[0], &[1, 1])); + assert_eq!(c(&[0, 0], &[1, 1, 1]).simplify(), c(&[0, 0], &[1, 1, 1])); + assert_eq!(c(&[0, 0], &[1, 1, 2]).simplify(), c(&[0, 0], &[1, 1, 2])); + assert_eq!(c(&[0, 0], &[1, 2, 0]).simplify(), c(&[0], &[1, 2])); + assert_eq!(c(&[0, 0], &[1, 2, 1]).simplify(), c(&[0, 0], &[1, 2, 1])); + assert_eq!(c(&[0, 0], &[1, 2, 2]).simplify(), c(&[0, 0], &[1, 2, 2])); + assert_eq!(c(&[0, 0], &[1, 2, 3]).simplify(), c(&[0, 0], &[1, 2, 3])); + assert_eq!(c(&[0, 1], &[0, 0, 0]).simplify(), c(&[1], &[0, 0])); + assert_eq!(c(&[0, 1], &[0, 0, 1]).simplify(), c(&[], &[0])); + assert_eq!(c(&[0, 1], &[0, 0, 2]).simplify(), c(&[1], &[0, 2])); + assert_eq!(c(&[0, 1], &[0, 1, 0]).simplify(), c(&[], &[0])); + assert_eq!(c(&[0, 1], &[0, 1, 1]).simplify(), c(&[], &[1])); + assert_eq!(c(&[0, 1], &[0, 1, 2]).simplify(), c(&[], &[2])); + assert_eq!(c(&[0, 1], &[0, 2, 0]).simplify(), c(&[1], &[2, 0])); + assert_eq!(c(&[0, 1], &[0, 2, 1]).simplify(), c(&[], &[2])); + assert_eq!(c(&[0, 1], &[0, 2, 2]).simplify(), c(&[1], &[2, 2])); + assert_eq!(c(&[0, 1], &[0, 2, 3]).simplify(), c(&[1], &[2, 3])); + assert_eq!(c(&[0, 1], &[1, 0, 0]).simplify(), c(&[], &[0])); + assert_eq!(c(&[0, 1], &[1, 0, 1]).simplify(), c(&[], &[1])); + assert_eq!(c(&[0, 1], &[1, 0, 2]).simplify(), c(&[], &[2])); + assert_eq!(c(&[0, 1], &[1, 1, 0]).simplify(), c(&[], &[1])); + assert_eq!(c(&[0, 1], &[1, 1, 1]).simplify(), c(&[0], &[1, 1])); + assert_eq!(c(&[0, 1], &[1, 1, 2]).simplify(), c(&[0], &[1, 2])); + assert_eq!(c(&[0, 1], &[1, 2, 0]).simplify(), c(&[], &[2])); + assert_eq!(c(&[0, 1], &[1, 2, 1]).simplify(), c(&[0], &[2, 1])); + assert_eq!(c(&[0, 1], &[1, 2, 2]).simplify(), c(&[0], &[2, 2])); + assert_eq!(c(&[0, 1], &[1, 2, 3]).simplify(), c(&[0], &[2, 3])); + assert_eq!(c(&[0, 1], &[2, 0, 0]).simplify(), c(&[1], &[2, 0])); + assert_eq!(c(&[0, 1], &[2, 0, 1]).simplify(), c(&[], &[2])); + assert_eq!(c(&[0, 1], &[2, 0, 2]).simplify(), c(&[1], &[2, 2])); + assert_eq!(c(&[0, 1], &[2, 0, 3]).simplify(), c(&[1], &[2, 3])); + assert_eq!(c(&[0, 1], &[2, 1, 0]).simplify(), c(&[], &[2])); + assert_eq!(c(&[0, 1], &[2, 1, 1]).simplify(), c(&[0], &[2, 1])); + assert_eq!(c(&[0, 1], &[2, 1, 2]).simplify(), c(&[0], &[2, 2])); + assert_eq!(c(&[0, 1], &[2, 1, 3]).simplify(), c(&[0], &[2, 3])); + assert_eq!(c(&[0, 1], &[2, 2, 0]).simplify(), c(&[1], &[2, 2])); + assert_eq!(c(&[0, 1], &[2, 2, 1]).simplify(), c(&[0], &[2, 2])); + assert_eq!(c(&[0, 1], &[2, 2, 2]).simplify(), c(&[0, 1], &[2, 2, 2])); + assert_eq!(c(&[0, 1], &[2, 2, 3]).simplify(), c(&[0, 1], &[2, 2, 3])); + assert_eq!(c(&[0, 1], &[2, 3, 0]).simplify(), c(&[1], &[2, 3])); + assert_eq!(c(&[0, 1], &[2, 3, 1]).simplify(), c(&[0], &[2, 3])); + assert_eq!(c(&[0, 1], &[2, 3, 2]).simplify(), c(&[0, 1], &[2, 3, 2])); + assert_eq!(c(&[0, 1], &[2, 3, 3]).simplify(), c(&[0, 1], &[2, 3, 3])); + assert_eq!(c(&[0, 1], &[2, 3, 4]).simplify(), c(&[0, 1], &[2, 3, 4])); + } } diff --git a/lib/src/tree.rs b/lib/src/tree.rs index 7f77d1f13..e39d59f75 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -786,24 +786,9 @@ fn simplify_conflict( } } - // Remove pairs of entries that match in the removes and adds. - let mut add_index = 0; - while add_index < new_adds.len() { - let add = &new_adds[add_index]; - add_index += 1; - for (remove_index, remove) in new_removes.iter().enumerate() { - if remove == add { - new_removes.remove(remove_index); - add_index -= 1; - new_adds.remove(add_index); - break; - } - } - } - // TODO: We should probably remove duplicate entries here too. So if we have // {+A+A}, that would become just {+A}. Similarly {+B-A+B} would be just // {+B-A}. - Ok(Conflict::new(new_removes, new_adds)) + Ok(Conflict::new(new_removes, new_adds).simplify()) }