From ad676cdf7e7489404fc1c719c78bacf84bf4f46d Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Sun, 20 Oct 2024 23:22:11 +0900
Subject: [PATCH] revset: collect RevWalkBuilder arguments by caller

This will make error propagation simpler in the following patches. The input
iterators will be changed to Item = Result<IndexPosition, _>.
---
 lib/src/default_index/rev_walk.rs      | 21 ++++++++-------------
 lib/src/default_index/revset_engine.rs | 14 +++++++-------
 2 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/lib/src/default_index/rev_walk.rs b/lib/src/default_index/rev_walk.rs
index 43ff667bf..ffa12fa3b 100644
--- a/lib/src/default_index/rev_walk.rs
+++ b/lib/src/default_index/rev_walk.rs
@@ -377,15 +377,15 @@ impl<'a> RevWalkBuilder<'a> {
         }
     }
 
-    /// Adds head positions to be included.
-    pub fn wanted_heads(mut self, positions: impl IntoIterator<Item = IndexPosition>) -> Self {
-        self.wanted.extend(positions);
+    /// Sets head positions to be included.
+    pub fn wanted_heads(mut self, positions: Vec<IndexPosition>) -> Self {
+        self.wanted = positions;
         self
     }
 
-    /// Adds root positions to be excluded. The roots precede the heads.
-    pub fn unwanted_roots(mut self, positions: impl IntoIterator<Item = IndexPosition>) -> Self {
-        self.unwanted.extend(positions);
+    /// Sets root positions to be excluded. The roots precede the heads.
+    pub fn unwanted_roots(mut self, positions: Vec<IndexPosition>) -> Self {
+        self.unwanted = positions;
         self
     }
 
@@ -449,12 +449,8 @@ impl<'a> RevWalkBuilder<'a> {
     ///
     /// The returned iterator yields entries in order of ascending index
     /// position.
-    pub fn descendants(
-        self,
-        root_positions: impl IntoIterator<Item = IndexPosition>,
-    ) -> RevWalkDescendants<'a> {
+    pub fn descendants(self, root_positions: HashSet<IndexPosition>) -> RevWalkDescendants<'a> {
         let index = self.index;
-        let root_positions = HashSet::from_iter(root_positions);
         let candidate_positions = self
             .ancestors_until_roots(root_positions.iter().copied())
             .collect();
@@ -477,11 +473,10 @@ impl<'a> RevWalkBuilder<'a> {
     /// position.
     pub fn descendants_filtered_by_generation(
         self,
-        root_positions: impl IntoIterator<Item = IndexPosition>,
+        root_positions: Vec<IndexPosition>,
         generation_range: Range<u32>,
     ) -> RevWalkDescendantsGenerationRange {
         let index = self.index;
-        let root_positions = Vec::from_iter(root_positions);
         let positions = self.ancestors_until_roots(root_positions.iter().copied());
         let descendants_index = RevWalkDescendantsIndex::build(index, positions);
 
diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs
index 7a0310226..d240ee149 100644
--- a/lib/src/default_index/revset_engine.rs
+++ b/lib/src/default_index/revset_engine.rs
@@ -770,7 +770,7 @@ impl EvaluationContext<'_> {
             ResolvedExpression::Ancestors { heads, generation } => {
                 let head_set = self.evaluate(heads)?;
                 let head_positions = head_set.positions().attach(index);
-                let builder = RevWalkBuilder::new(index).wanted_heads(head_positions);
+                let builder = RevWalkBuilder::new(index).wanted_heads(head_positions.collect());
                 if generation == &GENERATION_RANGE_FULL {
                     let walk = builder.ancestors().detach();
                     Ok(Box::new(RevWalkRevset { walk }))
@@ -800,7 +800,7 @@ impl EvaluationContext<'_> {
                 )
                 .attach(index);
                 let builder = RevWalkBuilder::new(index)
-                    .wanted_heads(head_positions)
+                    .wanted_heads(head_positions.collect())
                     .unwanted_roots(root_positions);
                 if generation == &GENERATION_RANGE_FULL {
                     let walk = builder.ancestors().detach();
@@ -822,7 +822,7 @@ impl EvaluationContext<'_> {
                 let root_positions = root_set.positions().attach(index);
                 let head_set = self.evaluate(heads)?;
                 let head_positions = head_set.positions().attach(index);
-                let builder = RevWalkBuilder::new(index).wanted_heads(head_positions);
+                let builder = RevWalkBuilder::new(index).wanted_heads(head_positions.collect());
                 if generation_from_roots == &(1..2) {
                     let root_positions: HashSet<_> = root_positions.collect();
                     let walk = builder
@@ -843,7 +843,7 @@ impl EvaluationContext<'_> {
                         predicate,
                     }))
                 } else if generation_from_roots == &GENERATION_RANGE_FULL {
-                    let mut positions = builder.descendants(root_positions).collect_vec();
+                    let mut positions = builder.descendants(root_positions.collect()).collect_vec();
                     positions.reverse();
                     Ok(Box::new(EagerRevset { positions }))
                 } else {
@@ -852,7 +852,7 @@ impl EvaluationContext<'_> {
                     //   reachable[pos] = (reachable[parent_pos] | ...) << 1
                     let mut positions = builder
                         .descendants_filtered_by_generation(
-                            root_positions,
+                            root_positions.collect(),
                             to_u32_generation_range(generation_from_roots)?,
                         )
                         .map(|Reverse(pos)| pos)
@@ -906,8 +906,8 @@ impl EvaluationContext<'_> {
                     .attach(index)
                     .collect_vec();
                 let filled = RevWalkBuilder::new(index)
-                    .wanted_heads(positions.iter().copied())
-                    .descendants(positions.iter().copied())
+                    .wanted_heads(positions.clone())
+                    .descendants(positions.iter().copied().collect())
                     .collect_positions_set();
                 positions.retain(|&pos| {
                     !index