Skip to content

Commit

Permalink
revset: extend lifetime of CommitId/ChangeId iterators
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
yuja committed Mar 13, 2024
1 parent 3bf41d0 commit 17e46e0
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 10 deletions.
32 changes: 24 additions & 8 deletions lib/src/default_index/revset_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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>)> + '_> {
Expand Down
8 changes: 6 additions & 2 deletions lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>)> + '_>;

Expand Down

0 comments on commit 17e46e0

Please sign in to comment.