From e37378dc186df321af45a518de4d893eb963cc08 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 20 Oct 2024 16:09:27 +0900 Subject: [PATCH] revset: implement RevWalk::map() and filter_map(), remove filter() 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. --- lib/src/default_index/rev_walk.rs | 88 ++++++++++++++++++-------- lib/src/default_index/revset_engine.rs | 23 +++---- 2 files changed, 73 insertions(+), 38 deletions(-) diff --git a/lib/src/default_index/rev_walk.rs b/lib/src/default_index/rev_walk.rs index ffa12fa3b2..8a266f224a 100644 --- a/lib/src/default_index/rev_walk.rs +++ b/lib/src/default_index/rev_walk.rs @@ -44,16 +44,23 @@ pub(super) trait RevWalk { // 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

(self, predicate: P) -> FilterRevWalk + /// Wraps in adapter that will filter and transform items by the given + /// function. + fn filter_map(self, f: F) -> FilterMapRevWalk where Self: Sized, - P: FnMut(&I, &Self::Item) -> bool, + F: FnMut(&I, Self::Item) -> Option, { - FilterRevWalk { - walk: self, - predicate, - } + FilterMapRevWalk { walk: self, f } + } + + /// Wraps in adapter that will transform items by the given function. + fn map(self, f: F) -> MapRevWalk + where + Self: Sized, + F: FnMut(&I, Self::Item) -> B, + { + MapRevWalk { walk: self, f } } /// Wraps in adapter that can peek one more item without consuming. @@ -108,29 +115,49 @@ impl RevWalk for EagerRevWalk { #[derive(Clone, Debug)] #[must_use] -pub(super) struct FilterRevWalk { +pub(super) struct FilterMapRevWalk { walk: W, - predicate: P, + f: F, } -impl RevWalk for FilterRevWalk +impl RevWalk for FilterMapRevWalk where I: ?Sized, W: RevWalk, - P: FnMut(&I, &W::Item) -> bool, + F: FnMut(&I, W::Item) -> Option, { - type Item = W::Item; + type Item = B; fn next(&mut self, index: &I) -> Option { 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 { + walk: W, + f: F, +} + +impl RevWalk for MapRevWalk +where + I: ?Sized, + W: RevWalk, + F: FnMut(&I, W::Item) -> B, +{ + type Item = B; + + fn next(&mut self, index: &I) -> Option { + self.walk.next(index).map(|item| (self.f)(index, item)) + } +} + #[derive(Clone, Debug)] #[must_use] pub(super) struct PeekableRevWalk> { @@ -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()); @@ -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(); diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index d240ee149a..81be55a1bf 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -153,12 +153,11 @@ impl Revset for RevsetImpl { 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>( @@ -168,13 +167,11 @@ impl Revset for RevsetImpl { 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>( @@ -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)), ) }