From 17e46e09327c47687189ffce171916e7ddd314e0 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Mon, 11 Mar 2024 19:53:05 +0900
Subject: [PATCH] revset: extend lifetime of CommitId/ChangeId iterators

For the same reason as the previous commit. Since self.inner.positions()
basically clones the underlying evaluation tree, there is no reason to stick
to &self lifetime. Perhaps, some of the CLI utility can be changed to not
collect() the iterator.

Migrating iter_graph() requires non-trivial changes, so it will be done
separately.
---
 lib/src/default_index/revset_engine.rs | 32 +++++++++++++++++++-------
 lib/src/revset.rs                      |  8 +++++--
 2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs
index 7f12a721a..6a17ebbf0 100644
--- a/lib/src/default_index/revset_engine.rs
+++ b/lib/src/default_index/revset_engine.rs
@@ -17,10 +17,10 @@
 use std::cell::RefCell;
 use std::cmp::{Ordering, Reverse};
 use std::collections::{BTreeSet, BinaryHeap, HashSet};
-use std::fmt;
 use std::ops::Range;
 use std::rc::Rc;
 use std::sync::Arc;
+use std::{fmt, iter};
 
 use itertools::Itertools;
 
@@ -119,15 +119,31 @@ impl<I> fmt::Debug for RevsetImpl<I> {
 }
 
 impl<I: AsCompositeIndex + Clone> Revset for RevsetImpl<I> {
-    fn iter(&self) -> Box<dyn Iterator<Item = CommitId> + '_> {
-        Box::new(self.entries().map(|index_entry| index_entry.commit_id()))
+    fn iter<'a>(&self) -> Box<dyn Iterator<Item = CommitId> + 'a>
+    where
+        Self: 'a,
+    {
+        let index = self.index.clone();
+        let mut walk = self.inner.positions();
+        Box::new(iter::from_fn(move || {
+            let index = index.as_composite();
+            let pos = walk.next(index)?;
+            Some(index.entry_by_pos(pos).commit_id())
+        }))
     }
 
-    fn commit_change_ids(&self) -> Box<dyn Iterator<Item = (CommitId, ChangeId)> + '_> {
-        Box::new(
-            self.entries()
-                .map(|index_entry| (index_entry.commit_id(), index_entry.change_id())),
-        )
+    fn commit_change_ids<'a>(&self) -> Box<dyn Iterator<Item = (CommitId, ChangeId)> + 'a>
+    where
+        Self: 'a,
+    {
+        let index = self.index.clone();
+        let mut walk = self.inner.positions();
+        Box::new(iter::from_fn(move || {
+            let index = index.as_composite();
+            let pos = walk.next(index)?;
+            let entry = index.entry_by_pos(pos);
+            Some((entry.commit_id(), entry.change_id()))
+        }))
     }
 
     fn iter_graph(&self) -> Box<dyn Iterator<Item = (CommitId, Vec<RevsetGraphEdge>)> + '_> {
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 416d985fb..f65385025 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -2401,10 +2401,14 @@ impl VisibilityResolutionContext<'_> {
 
 pub trait Revset: fmt::Debug {
     /// Iterate in topological order with children before parents.
-    fn iter(&self) -> Box<dyn Iterator<Item = CommitId> + '_>;
+    fn iter<'a>(&self) -> Box<dyn Iterator<Item = CommitId> + 'a>
+    where
+        Self: 'a;
 
     /// Iterates commit/change id pairs in topological order.
-    fn commit_change_ids(&self) -> Box<dyn Iterator<Item = (CommitId, ChangeId)> + '_>;
+    fn commit_change_ids<'a>(&self) -> Box<dyn Iterator<Item = (CommitId, ChangeId)> + 'a>
+    where
+        Self: 'a;
 
     fn iter_graph(&self) -> Box<dyn Iterator<Item = (CommitId, Vec<RevsetGraphEdge>)> + '_>;