diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index f6d449b4ee..052adbbccf 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -386,56 +386,34 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { } /// Panics if `parent_mapping` contains cycles - fn new_parents(&self, old_ids: &[CommitId]) -> Vec { - fn single_substitution_round( - parent_mapping: &HashMap>, - ids: Vec, - ) -> (Vec, bool) { - let mut made_replacements = false; - let mut new_ids = vec![]; - // TODO(ilyagr): (Maybe?) optimize common case of replacements all - // being singletons. If CommitId-s were Copy. no allocations would be needed in - // that case, but it probably doesn't matter much while they are Vec-s. - for id in ids.into_iter() { - match parent_mapping.get(&id) { - None => new_ids.push(id), - Some(replacements) => { - assert!( - // Each commit must have a parent, so a parent can - // not just be mapped to nothing. This assertion - // could be removed if this function is used for - // mapping something other than a commit's parents. - !replacements.is_empty(), - "Found empty value for key {id:?} in the parent mapping", - ); - made_replacements = true; - new_ids.extend(replacements.iter().cloned()) - } - }; - } - (new_ids, made_replacements) - } - - let mut new_ids: Vec = old_ids.to_vec(); - // The longest possible non-cycle substitution sequence goes through each key of - // parent_mapping once. - let mut allowed_iterations = 0..self.parent_mapping.len(); - loop { - let made_replacements; - (new_ids, made_replacements) = single_substitution_round(&self.parent_mapping, new_ids); - if !made_replacements { - break; + fn new_parents(&self, mut ids: Vec) -> Vec { + let mut i = 0; + while i < ids.len() { + // The longest possible non-cycle substitution sequence goes through each key of + // parent_mapping once. + let mut allowed_iterations = 0..self.parent_mapping.len(); + while let Some(replacements) = self.parent_mapping.get(&ids[i]) { + allowed_iterations + .next() + .expect("cycle detected in the parent mapping"); + assert!( + // Each commit must have a parent, so a parent can + // not just be mapped to nothing. If this assertion is + // removed, adjust the looping logic. + !replacements.is_empty(), + "Found empty value for key {:?} in the parent mapping", + &ids[i] + ); + ids.splice(i..i + 1, replacements.iter().cloned()); } - allowed_iterations - .next() - .expect("cycle detected in the parent mapping"); + i += 1; } - match new_ids.as_slice() { + match ids.as_slice() { // The first two cases are an optimization for the common case of commits with <=2 // parents - [_singleton] => new_ids, - [a, b] if a != b => new_ids, - _ => new_ids.into_iter().unique().collect(), + [_singleton] => ids, + [a, b] if a != b => ids, + _ => ids.into_iter().unique().collect(), } } @@ -539,7 +517,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { continue; } let old_parent_ids = old_commit.parent_ids(); - let new_parent_ids = self.new_parents(old_parent_ids); + let new_parent_ids = self.new_parents(old_parent_ids.to_vec()); if self.abandoned.contains(&old_commit_id) { // Update the `new_parents` map so descendants are rebased correctly. self.parent_mapping