Skip to content

Commit

Permalink
revset: implement RevWalk::map() and filter_map(), remove filter()
Browse files Browse the repository at this point in the history
We'll need to propagate error from predicate function, so .filter() will no
longer be usable. .map() will be used in order to wrap infallible ancestry
lookup with Ok(_).

Some RevsetImpl methods are migrated to .map() as example.
  • Loading branch information
yuja committed Oct 23, 2024
1 parent ad676cd commit e37378d
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 38 deletions.
88 changes: 63 additions & 25 deletions lib/src/default_index/rev_walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,23 @@ pub(super) trait RevWalk<I: ?Sized> {
// The following methods are provided for convenience. They are not supposed
// to be reimplemented.

/// Wraps in adapter that will filter items by the given `predicate`.
fn filter<P>(self, predicate: P) -> FilterRevWalk<Self, P>
/// Wraps in adapter that will filter and transform items by the given
/// function.
fn filter_map<B, F>(self, f: F) -> FilterMapRevWalk<Self, F>
where
Self: Sized,
P: FnMut(&I, &Self::Item) -> bool,
F: FnMut(&I, Self::Item) -> Option<B>,
{
FilterRevWalk {
walk: self,
predicate,
}
FilterMapRevWalk { walk: self, f }
}

/// Wraps in adapter that will transform items by the given function.
fn map<B, F>(self, f: F) -> MapRevWalk<Self, F>
where
Self: Sized,
F: FnMut(&I, Self::Item) -> B,
{
MapRevWalk { walk: self, f }
}

/// Wraps in adapter that can peek one more item without consuming.
Expand Down Expand Up @@ -108,29 +115,49 @@ impl<I: ?Sized, T: Iterator> RevWalk<I> for EagerRevWalk<T> {

#[derive(Clone, Debug)]
#[must_use]
pub(super) struct FilterRevWalk<W, P> {
pub(super) struct FilterMapRevWalk<W, F> {
walk: W,
predicate: P,
f: F,
}

impl<I, W, P> RevWalk<I> for FilterRevWalk<W, P>
impl<B, I, W, F> RevWalk<I> for FilterMapRevWalk<W, F>
where
I: ?Sized,
W: RevWalk<I>,
P: FnMut(&I, &W::Item) -> bool,
F: FnMut(&I, W::Item) -> Option<B>,
{
type Item = W::Item;
type Item = B;

fn next(&mut self, index: &I) -> Option<Self::Item> {
while let Some(item) = self.walk.next(index) {
if (self.predicate)(index, &item) {
return Some(item);
if let Some(new_item) = (self.f)(index, item) {
return Some(new_item);
}
}
None
}
}

#[derive(Clone, Debug)]
#[must_use]
pub(super) struct MapRevWalk<W, F> {
walk: W,
f: F,
}

impl<B, I, W, F> RevWalk<I> for MapRevWalk<W, F>
where
I: ?Sized,
W: RevWalk<I>,
F: FnMut(&I, W::Item) -> B,
{
type Item = B;

fn next(&mut self, index: &I) -> Option<Self::Item> {
self.walk.next(index).map(|item| (self.f)(index, item))
}
}

#[derive(Clone, Debug)]
#[must_use]
pub(super) struct PeekableRevWalk<I: ?Sized, W: RevWalk<I>> {
Expand Down Expand Up @@ -797,6 +824,28 @@ mod tests {
.collect()
}

#[test]
fn test_filter_map_rev_walk() {
let source = EagerRevWalk::new(vec![0, 1, 2, 3, 4].into_iter());
let mut filtered = source.filter_map(|_, v| (v & 1 == 0).then_some(v + 5));
assert_eq!(filtered.next(&()), Some(5));
assert_eq!(filtered.next(&()), Some(7));
assert_eq!(filtered.next(&()), Some(9));
assert_eq!(filtered.next(&()), None);
assert_eq!(filtered.next(&()), None);
}

#[test]
fn test_map_rev_walk() {
let source = EagerRevWalk::new(vec![0, 1, 2].into_iter());
let mut mapped = source.map(|_, v| v + 5);
assert_eq!(mapped.next(&()), Some(5));
assert_eq!(mapped.next(&()), Some(6));
assert_eq!(mapped.next(&()), Some(7));
assert_eq!(mapped.next(&()), None);
assert_eq!(mapped.next(&()), None);
}

#[test]
fn test_peekable_rev_walk() {
let source = EagerRevWalk::new(vec![0, 1, 2, 3].into_iter());
Expand All @@ -821,17 +870,6 @@ mod tests {
assert_eq!(peekable.next(&()), None);
}

#[test]
fn test_filter_rev_walk() {
let source = EagerRevWalk::new(vec![0, 1, 2, 3, 4].into_iter());
let mut filtered = source.filter(|_, &v| v & 1 == 0);
assert_eq!(filtered.next(&()), Some(0));
assert_eq!(filtered.next(&()), Some(2));
assert_eq!(filtered.next(&()), Some(4));
assert_eq!(filtered.next(&()), None);
assert_eq!(filtered.next(&()), None);
}

#[test]
fn test_walk_ancestors() {
let mut new_change_id = change_id_generator();
Expand Down
23 changes: 10 additions & 13 deletions lib/src/default_index/revset_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,11 @@ impl<I: AsCompositeIndex + Clone> Revset for RevsetImpl<I> {
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(Ok(index.entry_by_pos(pos).commit_id()))
}))
let mut walk = self
.inner
.positions()
.map(|index, pos| Ok(index.entry_by_pos(pos).commit_id()));
Box::new(iter::from_fn(move || walk.next(index.as_composite())))
}

fn commit_change_ids<'a>(
Expand All @@ -168,13 +167,11 @@ impl<I: AsCompositeIndex + Clone> Revset for RevsetImpl<I> {
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 mut walk = self.inner.positions().map(|index, pos| {
let entry = index.entry_by_pos(pos);
Some(Ok((entry.commit_id(), entry.change_id())))
}))
Ok((entry.commit_id(), entry.change_id()))
});
Box::new(iter::from_fn(move || walk.next(index.as_composite())))
}

fn iter_graph<'a>(
Expand Down Expand Up @@ -388,7 +385,7 @@ where
Box::new(
self.candidates
.positions()
.filter(move |index, &pos| p(index, pos)),
.filter_map(move |index, pos| p(index, pos).then_some(pos)),
)
}

Expand Down

0 comments on commit e37378d

Please sign in to comment.