revset: replace RevsetIterator wrapper by extension

The type doesn't seem to provide any benefit. I don't think I had a
good reason for creating it in the first place; it was probably just
unfamiliarity with Rust.

lib/src/revset.rs

@@ -1522,7 +1522,7 @@ pub fn optimize(expression: Rc<RevsetExpression>) -> Rc<RevsetExpression> {
 
 pub trait Revset<'index>: ToPredicateFn<'index> {
     // All revsets currently iterate in order of descending index position
-    fn iter(&self) -> RevsetIterator<'_, 'index>;
+    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_>;
 
     fn is_empty(&self) -> bool {
         self.iter().next().is_none()
@@ -1546,33 +1546,25 @@ where
     }
 }
 
-pub struct RevsetIterator<'revset, 'index: 'revset> {
-    inner: Box<dyn Iterator<Item = IndexEntry<'index>> + 'revset>,
+pub trait RevsetIteratorExt<'index, I> {
+    fn commit_ids(self) -> RevsetCommitIdIterator<I>;
+    fn commits(self, store: &Arc<Store>) -> RevsetCommitIterator<I>;
+    fn reversed(self) -> ReverseRevsetIterator<'index>;
 }
 
-impl<'revset, 'index> RevsetIterator<'revset, 'index> {
-    fn new(inner: Box<dyn Iterator<Item = IndexEntry<'index>> + 'revset>) -> Self {
-        Self { inner }
-    }
-
-    pub fn commit_ids(self) -> RevsetCommitIdIterator<Self>
-    where
-        Self: Sized + Iterator<Item = IndexEntry<'index>>,
-    {
+impl<'index, I: Iterator<Item = IndexEntry<'index>>> RevsetIteratorExt<'index, I> for I {
+    fn commit_ids(self) -> RevsetCommitIdIterator<I> {
         RevsetCommitIdIterator(self)
     }
 
-    pub fn commits(self, store: &Arc<Store>) -> RevsetCommitIterator<Self>
-    where
-        Self: Sized + Iterator<Item = IndexEntry<'index>>,
-    {
+    fn commits(self, store: &Arc<Store>) -> RevsetCommitIterator<I> {
         RevsetCommitIterator {
             iter: self,
             store: store.clone(),
         }
     }
 
-    pub fn reversed(self) -> ReverseRevsetIterator<'index> {
+    fn reversed(self) -> ReverseRevsetIterator<'index> {
         ReverseRevsetIterator {
             entries: self.into_iter().collect_vec(),
         }
@@ -1591,14 +1583,6 @@ fn predicate_fn_from_iter<'index, 'iter>(
     })
 }
 
-impl<'index> Iterator for RevsetIterator<'_, 'index> {
-    type Item = IndexEntry<'index>;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        self.inner.next()
-    }
-}
-
 pub struct RevsetCommitIdIterator<I>(I);
 
 impl<'index, I: Iterator<Item = IndexEntry<'index>>> Iterator for RevsetCommitIdIterator<I> {
@@ -1649,8 +1633,8 @@ impl EagerRevset<'static> {
 }
 
 impl<'index> Revset<'index> for EagerRevset<'index> {
-    fn iter(&self) -> RevsetIterator<'_, 'index> {
-        RevsetIterator::new(Box::new(self.index_entries.iter().cloned()))
+    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_> {
+        Box::new(self.index_entries.iter().cloned())
     }
 }
 
@@ -1673,8 +1657,8 @@ impl<'index, T> Revset<'index> for RevWalkRevset<'index, T>
 where
     T: Iterator<Item = IndexEntry<'index>> + Clone,
 {
-    fn iter(&self) -> RevsetIterator<'_, 'index> {
-        RevsetIterator::new(Box::new(self.walk.clone()))
+    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_> {
+        Box::new(self.walk.clone())
     }
 }
 
@@ -1695,21 +1679,19 @@ struct ChildrenRevset<'index> {
 }
 
 impl<'index> Revset<'index> for ChildrenRevset<'index> {
-    fn iter(&self) -> RevsetIterator<'_, 'index> {
+    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_> {
         let roots: HashSet<_> = self
             .root_set
             .iter()
             .map(|parent| parent.position())
             .collect();
 
-        RevsetIterator::new(Box::new(self.candidate_set.iter().filter(
-            move |candidate| {
-                candidate
-                    .parent_positions()
-                    .iter()
-                    .any(|parent_pos| roots.contains(parent_pos))
-            },
-        )))
+        Box::new(self.candidate_set.iter().filter(move |candidate| {
+            candidate
+                .parent_positions()
+                .iter()
+                .any(|parent_pos| roots.contains(parent_pos))
+        }))
     }
 }
 
@@ -1729,9 +1711,9 @@ impl<'index, P> Revset<'index> for FilterRevset<'index, P>
 where
     P: ToPredicateFn<'index>,
 {
-    fn iter(&self) -> RevsetIterator<'_, 'index> {
+    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_> {
         let p = self.predicate.to_predicate_fn();
-        RevsetIterator::new(Box::new(self.candidates.iter().filter(p)))
+        Box::new(self.candidates.iter().filter(p))
     }
 }
 
@@ -1753,11 +1735,11 @@ struct UnionRevset<'index> {
 }
 
 impl<'index> Revset<'index> for UnionRevset<'index> {
-    fn iter(&self) -> RevsetIterator<'_, 'index> {
-        RevsetIterator::new(Box::new(UnionRevsetIterator {
+    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_> {
+        Box::new(UnionRevsetIterator {
             iter1: self.set1.iter().peekable(),
             iter2: self.set2.iter().peekable(),
-        }))
+        })
     }
 }
 
@@ -1805,11 +1787,11 @@ struct IntersectionRevset<'index> {
 }
 
 impl<'index> Revset<'index> for IntersectionRevset<'index> {
-    fn iter(&self) -> RevsetIterator<'_, 'index> {
-        RevsetIterator::new(Box::new(IntersectionRevsetIterator {
+    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_> {
+        Box::new(IntersectionRevsetIterator {
             iter1: self.set1.iter().peekable(),
             iter2: self.set2.iter().peekable(),
-        }))
+        })
     }
 }
 
@@ -1869,11 +1851,11 @@ struct DifferenceRevset<'index> {
 }
 
 impl<'index> Revset<'index> for DifferenceRevset<'index> {
-    fn iter(&self) -> RevsetIterator<'_, 'index> {
-        RevsetIterator::new(Box::new(DifferenceRevsetIterator {
+    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_> {
+        Box::new(DifferenceRevsetIterator {
             iter1: self.set1.iter().peekable(),
             iter2: self.set2.iter().peekable(),
-        }))
+        })
     }
 }
 

lib/src/revset_graph_iterator.rs

@@ -17,7 +17,7 @@ use std::collections::{BTreeMap, HashMap, HashSet};
 
 use crate::index::{IndexEntry, IndexPosition};
 use crate::nightly_shims::BTreeMapExt;
-use crate::revset::{Revset, RevsetIterator};
+use crate::revset::Revset;
 
 #[derive(Debug, PartialEq, Eq, Clone, Hash)]
 pub struct RevsetGraphEdge {
@@ -120,7 +120,7 @@ pub enum RevsetGraphEdgeType {
 // "D", but that would require extra book-keeping to remember for later that the
 // edges from "f" and "H" are only partially computed.
 pub struct RevsetGraphIterator<'revset, 'index> {
-    input_set_iter: RevsetIterator<'revset, 'index>,
+    input_set_iter: Box<dyn Iterator<Item = IndexEntry<'index>> + 'revset>,
     // Commits in the input set we had to take out of the iterator while walking external
     // edges. Does not necessarily include the commit we're currently about to emit.
     look_ahead: BTreeMap<IndexPosition, IndexEntry<'index>>,

lib/src/rewrite.rs

@@ -22,7 +22,7 @@ use crate::dag_walk;
 use crate::op_store::RefTarget;
 use crate::repo::{MutableRepo, Repo};
 use crate::repo_path::RepoPath;
-use crate::revset::RevsetExpression;
+use crate::revset::{RevsetExpression, RevsetIteratorExt};
 use crate::settings::UserSettings;
 use crate::tree::{merge_trees, Tree};
 use crate::view::RefName;

lib/tests/test_revset.rs

@@ -23,7 +23,7 @@ use jujutsu_lib::repo::Repo;
 use jujutsu_lib::repo_path::RepoPath;
 use jujutsu_lib::revset::{
     self, optimize, parse, resolve_symbol, RevsetAliasesMap, RevsetError, RevsetExpression,
-    RevsetWorkspaceContext,
+    RevsetIteratorExt, RevsetWorkspaceContext,
 };
 use jujutsu_lib::settings::GitSettings;
 use jujutsu_lib::workspace::Workspace;

src/cli_util.rs

@@ -43,7 +43,7 @@ use jujutsu_lib::repo::{
 };
 use jujutsu_lib::repo_path::{FsPathParseError, RepoPath};
 use jujutsu_lib::revset::{
-    Revset, RevsetAliasesMap, RevsetError, RevsetExpression, RevsetParseError,
+    Revset, RevsetAliasesMap, RevsetError, RevsetExpression, RevsetIteratorExt, RevsetParseError,
     RevsetWorkspaceContext,
 };
 use jujutsu_lib::settings::UserSettings;

src/commands/mod.rs

@@ -35,7 +35,7 @@ use jujutsu_lib::matchers::EverythingMatcher;
 use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
 use jujutsu_lib::repo::{ReadonlyRepo, Repo};
 use jujutsu_lib::repo_path::RepoPath;
-use jujutsu_lib::revset::{RevsetAliasesMap, RevsetExpression};
+use jujutsu_lib::revset::{RevsetAliasesMap, RevsetExpression, RevsetIteratorExt};
 use jujutsu_lib::revset_graph_iterator::{
     RevsetGraphEdge, RevsetGraphEdgeType, RevsetGraphIterator,
 };