From 9c5470a89908cc41bd9fee07df57d66d72942851 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 11 Dec 2023 12:47:26 +0900 Subject: [PATCH 1/7] revset: extract inner method that constructs IndexEntry iterator --- lib/src/default_revset_engine.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 49b06b9fb3..93e63372c6 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -76,8 +76,12 @@ impl<'index> RevsetImpl<'index> { } } + fn entries(&self) -> Box> + '_> { + self.inner.iter() + } + pub fn iter_graph_impl(&self) -> RevsetGraphIterator<'_, 'index> { - RevsetGraphIterator::new(self.index, self.inner.iter()) + RevsetGraphIterator::new(self.index, self.entries()) } } @@ -91,13 +95,12 @@ impl fmt::Debug for RevsetImpl<'_> { impl<'index> Revset<'index> for RevsetImpl<'index> { fn iter(&self) -> Box + '_> { - Box::new(self.inner.iter().map(|index_entry| index_entry.commit_id())) + Box::new(self.entries().map(|index_entry| index_entry.commit_id())) } fn commit_change_ids(&self) -> Box + '_> { Box::new( - self.inner - .iter() + self.entries() .map(|index_entry| (index_entry.commit_id(), index_entry.change_id())), ) } @@ -109,7 +112,7 @@ impl<'index> Revset<'index> for RevsetImpl<'index> { fn change_id_index(&self) -> Box { // TODO: Create a persistent lookup from change id to commit ids. let mut pos_by_change = IdIndex::builder(); - for entry in self.inner.iter() { + for entry in self.entries() { pos_by_change.insert(&entry.change_id(), entry.position()); } Box::new(ChangeIdIndexImpl { @@ -119,11 +122,11 @@ impl<'index> Revset<'index> for RevsetImpl<'index> { } fn is_empty(&self) -> bool { - self.iter().next().is_none() + self.entries().next().is_none() } fn count(&self) -> usize { - self.inner.iter().count() + self.entries().count() } } From c14e3c04989848f8b11d3bba83df9a62b29d40b9 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 29 May 2023 12:50:33 +0900 Subject: [PATCH 2/7] revset: pass in index to InternalRevset as an argument The idea is that InternalRevset will store a 'static boilerplate function that borrows an 'index passed by function argument. This way, we can abstract the index type over Arc and &T without introducing too much generics. --- lib/src/default_revset_engine.rs | 237 ++++++++++++++++++++++--------- 1 file changed, 171 insertions(+), 66 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 93e63372c6..1b63d4cd6d 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -42,18 +42,30 @@ trait ToPredicateFn: fmt::Debug { /// Creates function that tests if the given entry is included in the set. /// /// The predicate function is evaluated in order of `RevsetIterator`. - fn to_predicate_fn(&self) -> Box) -> bool + '_>; + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a>; } impl ToPredicateFn for Box { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - ::to_predicate_fn(self) + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + ::to_predicate_fn(self, index) } } +// TODO: move <'index> parameter to iter<'a, 'index: 'a> trait InternalRevset<'index>: fmt::Debug + ToPredicateFn { // All revsets currently iterate in order of descending index position - fn iter(&self) -> Box> + '_>; + fn iter<'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> + where + 'index: 'a; fn into_predicate<'a>(self: Box) -> Box where @@ -77,7 +89,7 @@ impl<'index> RevsetImpl<'index> { } fn entries(&self) -> Box> + '_> { - self.inner.iter() + self.inner.iter(self.index) } pub fn iter_graph_impl(&self) -> RevsetGraphIterator<'_, 'index> { @@ -176,7 +188,13 @@ impl EagerRevset<'static> { } impl<'index> InternalRevset<'index> for EagerRevset<'index> { - fn iter(&self) -> Box> + '_> { + fn iter<'a>( + &'a self, + _index: CompositeIndex<'index>, + ) -> Box> + 'a> + where + 'index: 'a, + { Box::new(self.index_entries.iter().cloned()) } @@ -189,8 +207,11 @@ impl<'index> InternalRevset<'index> for EagerRevset<'index> { } impl ToPredicateFn for EagerRevset<'_> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - predicate_fn_from_iter(self.iter()) + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + _index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + predicate_fn_from_iter(self.index_entries.iter().cloned()) } } @@ -208,7 +229,13 @@ impl<'index, T> InternalRevset<'index> for RevWalkRevset where T: Iterator> + Clone, { - fn iter(&self) -> Box> + '_> { + fn iter<'a>( + &'a self, + _index: CompositeIndex<'index>, + ) -> Box> + 'a> + where + 'index: 'a, + { Box::new(self.walk.clone()) } @@ -224,7 +251,11 @@ impl<'index, T> ToPredicateFn for RevWalkRevset where T: Iterator> + Clone, { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { + // TODO: remove 'index from RevWalkRevset<'index, F> + fn to_predicate_fn<'a, 'index2: 'a>( + &'a self, + _index: CompositeIndex<'index2>, + ) -> Box) -> bool + 'a> { predicate_fn_from_iter(self.walk.clone()) } } @@ -248,9 +279,15 @@ struct FilterRevset<'index, P> { } impl<'index, P: ToPredicateFn> InternalRevset<'index> for FilterRevset<'index, P> { - fn iter(&self) -> Box> + '_> { - let p = self.predicate.to_predicate_fn(); - Box::new(self.candidates.iter().filter(p)) + fn iter<'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> + where + 'index: 'a, + { + let p = self.predicate.to_predicate_fn(index); + Box::new(self.candidates.iter(index).filter(p)) } fn into_predicate<'a>(self: Box) -> Box @@ -262,9 +299,12 @@ impl<'index, P: ToPredicateFn> InternalRevset<'index> for FilterRevset<'index, P } impl ToPredicateFn for FilterRevset<'_, P> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.candidates.to_predicate_fn(); - let mut p2 = self.predicate.to_predicate_fn(); + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.candidates.to_predicate_fn(index); + let mut p2 = self.predicate.to_predicate_fn(index); Box::new(move |entry| p1(entry) && p2(entry)) } } @@ -273,8 +313,11 @@ impl ToPredicateFn for FilterRevset<'_, P> { struct NotInPredicate(S); impl ToPredicateFn for NotInPredicate { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p = self.0.to_predicate_fn(); + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p = self.0.to_predicate_fn(index); Box::new(move |entry| !p(entry)) } } @@ -286,10 +329,16 @@ struct UnionRevset<'index> { } impl<'index> InternalRevset<'index> for UnionRevset<'index> { - fn iter(&self) -> Box> + '_> { + fn iter<'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> + where + 'index: 'a, + { Box::new(UnionRevsetIterator { - iter1: self.set1.iter().peekable(), - iter2: self.set2.iter().peekable(), + iter1: self.set1.iter(index).peekable(), + iter2: self.set2.iter(index).peekable(), }) } @@ -302,9 +351,12 @@ impl<'index> InternalRevset<'index> for UnionRevset<'index> { } impl ToPredicateFn for UnionRevset<'_> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.set1.to_predicate_fn(); - let mut p2 = self.set2.to_predicate_fn(); + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.set1.to_predicate_fn(index); + let mut p2 = self.set2.to_predicate_fn(index); Box::new(move |entry| p1(entry) || p2(entry)) } } @@ -320,9 +372,12 @@ where S1: ToPredicateFn, S2: ToPredicateFn, { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.set1.to_predicate_fn(); - let mut p2 = self.set2.to_predicate_fn(); + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.set1.to_predicate_fn(index); + let mut p2 = self.set2.to_predicate_fn(index); Box::new(move |entry| p1(entry) || p2(entry)) } } @@ -364,10 +419,16 @@ struct IntersectionRevset<'index> { } impl<'index> InternalRevset<'index> for IntersectionRevset<'index> { - fn iter(&self) -> Box> + '_> { + fn iter<'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> + where + 'index: 'a, + { Box::new(IntersectionRevsetIterator { - iter1: self.set1.iter().peekable(), - iter2: self.set2.iter().peekable(), + iter1: self.set1.iter(index).peekable(), + iter2: self.set2.iter(index).peekable(), }) } @@ -380,9 +441,12 @@ impl<'index> InternalRevset<'index> for IntersectionRevset<'index> { } impl ToPredicateFn for IntersectionRevset<'_> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.set1.to_predicate_fn(); - let mut p2 = self.set2.to_predicate_fn(); + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.set1.to_predicate_fn(index); + let mut p2 = self.set2.to_predicate_fn(index); Box::new(move |entry| p1(entry) && p2(entry)) } } @@ -436,10 +500,16 @@ struct DifferenceRevset<'index> { } impl<'index> InternalRevset<'index> for DifferenceRevset<'index> { - fn iter(&self) -> Box> + '_> { + fn iter<'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box> + 'a> + where + 'index: 'a, + { Box::new(DifferenceRevsetIterator { - iter1: self.set1.iter().peekable(), - iter2: self.set2.iter().peekable(), + iter1: self.set1.iter(index).peekable(), + iter2: self.set2.iter(index).peekable(), }) } @@ -452,9 +522,12 @@ impl<'index> InternalRevset<'index> for DifferenceRevset<'index> { } impl ToPredicateFn for DifferenceRevset<'_> { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { - let mut p1 = self.set1.to_predicate_fn(); - let mut p2 = self.set2.to_predicate_fn(); + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { + let mut p1 = self.set1.to_predicate_fn(index); + let mut p2 = self.set2.to_predicate_fn(index); Box::new(move |entry| p1(entry) && !p2(entry)) } } @@ -533,13 +606,17 @@ impl<'index> EvaluationContext<'index> { &self, expression: &ResolvedExpression, ) -> Result + 'index>, RevsetEvaluationError> { + let index = self.index; match expression { ResolvedExpression::Commits(commit_ids) => { Ok(Box::new(self.revset_for_commit_ids(commit_ids))) } ResolvedExpression::Ancestors { heads, generation } => { let head_set = self.evaluate(heads)?; - let head_positions = head_set.iter().map(|entry| entry.position()).collect_vec(); + let head_positions = head_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); let walk = self.index.walk_revs(&head_positions, &[]); if generation == &GENERATION_RANGE_FULL { Ok(Box::new(RevWalkRevset { walk })) @@ -554,9 +631,15 @@ impl<'index> EvaluationContext<'index> { generation, } => { let root_set = self.evaluate(roots)?; - let root_positions = root_set.iter().map(|entry| entry.position()).collect_vec(); + let root_positions = root_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); let head_set = self.evaluate(heads)?; - let head_positions = head_set.iter().map(|entry| entry.position()).collect_vec(); + let head_positions = head_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); let walk = self.index.walk_revs(&head_positions, &root_positions); if generation == &GENERATION_RANGE_FULL { Ok(Box::new(RevWalkRevset { walk })) @@ -570,11 +653,16 @@ impl<'index> EvaluationContext<'index> { heads, generation_from_roots, } => { - let index = self.index; let root_set = self.evaluate(roots)?; - let root_positions = root_set.iter().map(|entry| entry.position()).collect_vec(); + let root_positions = root_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); let head_set = self.evaluate(heads)?; - let head_positions = head_set.iter().map(|entry| entry.position()).collect_vec(); + let head_positions = head_set + .iter(index) + .map(|entry| entry.position()) + .collect_vec(); if generation_from_roots == &(1..2) { let walk = index .walk_revs(&head_positions, &[]) @@ -617,24 +705,26 @@ impl<'index> EvaluationContext<'index> { } ResolvedExpression::Heads(candidates) => { let candidate_set = self.evaluate(candidates)?; - let head_positions: BTreeSet<_> = self - .index - .heads_pos(candidate_set.iter().map(|entry| entry.position()).collect()); + let head_positions: BTreeSet<_> = index.heads_pos( + candidate_set + .iter(index) + .map(|entry| entry.position()) + .collect(), + ); let index_entries = head_positions .into_iter() .rev() - .map(|pos| self.index.entry_by_pos(pos)) + .map(|pos| index.entry_by_pos(pos)) .collect(); Ok(Box::new(EagerRevset { index_entries })) } ResolvedExpression::Roots(candidates) => { - let candidate_entries = self.evaluate(candidates)?.iter().collect_vec(); + let candidate_entries = self.evaluate(candidates)?.iter(index).collect_vec(); let candidate_positions = candidate_entries .iter() .map(|entry| entry.position()) .collect_vec(); - let filled = self - .index + let filled = index .walk_revs(&candidate_positions, &[]) .descendants(&candidate_positions) .collect_positions_set(); @@ -742,7 +832,7 @@ impl<'index> EvaluationContext<'index> { // Maintain min-heap containing the latest (greatest) count items. For small // count and large candidate set, this is probably cheaper than building vec // and applying selection algorithm. - let mut candidate_iter = candidate_set.iter().map(make_rev_item).fuse(); + let mut candidate_iter = candidate_set.iter(self.index).map(make_rev_item).fuse(); let mut latest_items = BinaryHeap::from_iter(candidate_iter.by_ref().take(count)); for item in candidate_iter { let mut earliest = latest_items.peek_mut().unwrap(); @@ -770,7 +860,10 @@ impl fmt::Debug for PurePredicateFn { } impl) -> bool> ToPredicateFn for PurePredicateFn { - fn to_predicate_fn(&self) -> Box) -> bool + '_> { + fn to_predicate_fn<'a, 'index: 'a>( + &'a self, + _index: CompositeIndex<'index>, + ) -> Box) -> bool + 'a> { Box::new(&self.0) } } @@ -891,14 +984,14 @@ mod tests { }; let set = make_set(&[&id_4, &id_3, &id_2, &id_0]); - let mut p = set.to_predicate_fn(); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(p(&get_entry(&id_4))); assert!(p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); assert!(!p(&get_entry(&id_1))); assert!(p(&get_entry(&id_0))); // Uninteresting entries can be skipped - let mut p = set.to_predicate_fn(); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(p(&get_entry(&id_3))); assert!(!p(&get_entry(&id_1))); assert!(p(&get_entry(&id_0))); @@ -907,8 +1000,11 @@ mod tests { candidates: make_set(&[&id_4, &id_2, &id_0]), predicate: pure_predicate_fn(|entry| entry.commit_id() != id_4), }; - assert_eq!(set.iter().collect_vec(), make_entries(&[&id_2, &id_0])); - let mut p = set.to_predicate_fn(); + assert_eq!( + set.iter(index.as_composite()).collect_vec(), + make_entries(&[&id_2, &id_0]) + ); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(!p(&get_entry(&id_4))); assert!(!p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); @@ -920,8 +1016,11 @@ mod tests { candidates: make_set(&[&id_4, &id_2, &id_0]), predicate: make_set(&[&id_3, &id_2, &id_1]), }; - assert_eq!(set.iter().collect_vec(), make_entries(&[&id_2])); - let mut p = set.to_predicate_fn(); + assert_eq!( + set.iter(index.as_composite()).collect_vec(), + make_entries(&[&id_2]) + ); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(!p(&get_entry(&id_4))); assert!(!p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); @@ -933,10 +1032,10 @@ mod tests { set2: make_set(&[&id_3, &id_2, &id_1]), }; assert_eq!( - set.iter().collect_vec(), + set.iter(index.as_composite()).collect_vec(), make_entries(&[&id_4, &id_3, &id_2, &id_1]) ); - let mut p = set.to_predicate_fn(); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(p(&get_entry(&id_4))); assert!(p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); @@ -947,8 +1046,11 @@ mod tests { set1: make_set(&[&id_4, &id_2, &id_0]), set2: make_set(&[&id_3, &id_2, &id_1]), }; - assert_eq!(set.iter().collect_vec(), make_entries(&[&id_2])); - let mut p = set.to_predicate_fn(); + assert_eq!( + set.iter(index.as_composite()).collect_vec(), + make_entries(&[&id_2]) + ); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(!p(&get_entry(&id_4))); assert!(!p(&get_entry(&id_3))); assert!(p(&get_entry(&id_2))); @@ -959,8 +1061,11 @@ mod tests { set1: make_set(&[&id_4, &id_2, &id_0]), set2: make_set(&[&id_3, &id_2, &id_1]), }; - assert_eq!(set.iter().collect_vec(), make_entries(&[&id_4, &id_0])); - let mut p = set.to_predicate_fn(); + assert_eq!( + set.iter(index.as_composite()).collect_vec(), + make_entries(&[&id_4, &id_0]) + ); + let mut p = set.to_predicate_fn(index.as_composite()); assert!(p(&get_entry(&id_4))); assert!(!p(&get_entry(&id_3))); assert!(!p(&get_entry(&id_2))); From 1e3251328fd28bc4859e8cc2d822676f335d8163 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 7 Dec 2023 14:25:16 +0900 Subject: [PATCH 3/7] revset: store IndexPosition in EagerRevset to drop 'index lifetime This adds overhead to re-look up IndexEntry, but I don't think that would have significant impact on performance. --- lib/src/default_index/entry.rs | 16 ----- lib/src/default_index/mod.rs | 2 +- lib/src/default_revset_engine.rs | 105 +++++++++++++++++-------------- 3 files changed, 58 insertions(+), 65 deletions(-) diff --git a/lib/src/default_index/entry.rs b/lib/src/default_index/entry.rs index cfe4c57ed3..d741da06ec 100644 --- a/lib/src/default_index/entry.rs +++ b/lib/src/default_index/entry.rs @@ -14,7 +14,6 @@ #![allow(missing_docs)] -use std::cmp::Ordering; use std::fmt::{Debug, Formatter}; use std::hash::{Hash, Hasher}; @@ -107,21 +106,6 @@ impl<'a> IndexEntry<'a> { } } -#[derive(Clone, Eq, PartialEq)] -pub struct IndexEntryByPosition<'a>(pub IndexEntry<'a>); - -impl Ord for IndexEntryByPosition<'_> { - fn cmp(&self, other: &Self) -> Ordering { - self.0.pos.cmp(&other.0.pos) - } -} - -impl PartialOrd for IndexEntryByPosition<'_> { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - /// Wrapper to sort `IndexPosition` by its generation number. /// /// This is similar to `IndexEntry` newtypes, but optimized for size and cache diff --git a/lib/src/default_index/mod.rs b/lib/src/default_index/mod.rs index ea3a0f41ac..4db14c9277 100644 --- a/lib/src/default_index/mod.rs +++ b/lib/src/default_index/mod.rs @@ -22,7 +22,7 @@ mod rev_walk; mod store; pub use self::composite::{CompositeIndex, IndexLevelStats, IndexStats}; -pub use self::entry::{IndexEntry, IndexEntryByPosition, IndexPosition}; +pub use self::entry::{IndexEntry, IndexPosition}; pub use self::mutable::DefaultMutableIndex; pub use self::readonly::DefaultReadonlyIndex; pub use self::rev_walk::{ diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 1b63d4cd6d..e91f223738 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -24,7 +24,7 @@ use std::sync::Arc; use itertools::Itertools; use crate::backend::{ChangeId, CommitId, MillisSinceEpoch}; -use crate::default_index::{CompositeIndex, IndexEntry, IndexEntryByPosition, IndexPosition}; +use crate::default_index::{CompositeIndex, IndexEntry, IndexPosition}; use crate::default_revset_graph_iterator::RevsetGraphIterator; use crate::id_prefix::{IdIndex, IdIndexSource, IdIndexSourceEntry}; use crate::index::{HexPrefix, PrefixResolution}; @@ -175,27 +175,31 @@ impl IdIndexSourceEntry for IndexEntry<'_> { } #[derive(Debug)] -struct EagerRevset<'index> { - index_entries: Vec>, +struct EagerRevset { + positions: Vec, } -impl EagerRevset<'static> { +impl EagerRevset { pub const fn empty() -> Self { EagerRevset { - index_entries: Vec::new(), + positions: Vec::new(), } } } -impl<'index> InternalRevset<'index> for EagerRevset<'index> { +impl<'index> InternalRevset<'index> for EagerRevset { fn iter<'a>( &'a self, - _index: CompositeIndex<'index>, + index: CompositeIndex<'index>, ) -> Box> + 'a> where 'index: 'a, { - Box::new(self.index_entries.iter().cloned()) + let entries = self + .positions + .iter() + .map(move |&pos| index.entry_by_pos(pos)); + Box::new(entries) } fn into_predicate<'a>(self: Box) -> Box @@ -206,12 +210,12 @@ impl<'index> InternalRevset<'index> for EagerRevset<'index> { } } -impl ToPredicateFn for EagerRevset<'_> { +impl ToPredicateFn for EagerRevset { fn to_predicate_fn<'a, 'index: 'a>( &'a self, _index: CompositeIndex<'index>, ) -> Box) -> bool + 'a> { - predicate_fn_from_iter(self.index_entries.iter().cloned()) + predicate_fn_from_positions(self.positions.iter().copied()) } } @@ -256,19 +260,25 @@ where &'a self, _index: CompositeIndex<'index2>, ) -> Box) -> bool + 'a> { - predicate_fn_from_iter(self.walk.clone()) + predicate_fn_from_entries(self.walk.clone()) } } -fn predicate_fn_from_iter<'index, 'iter>( +fn predicate_fn_from_entries<'index, 'iter>( iter: impl Iterator> + 'iter, +) -> Box) -> bool + 'iter> { + predicate_fn_from_positions(iter.map(|entry| entry.position())) +} + +fn predicate_fn_from_positions<'iter>( + iter: impl Iterator + 'iter, ) -> Box) -> bool + 'iter> { let mut iter = iter.fuse().peekable(); Box::new(move |entry| { - while iter.next_if(|e| e.position() > entry.position()).is_some() { + while iter.next_if(|&pos| pos > entry.position()).is_some() { continue; } - iter.next_if(|e| e.position() == entry.position()).is_some() + iter.next_if(|&pos| pos == entry.position()).is_some() }) } @@ -682,25 +692,27 @@ impl<'index> EvaluationContext<'index> { predicate, })) } else if generation_from_roots == &GENERATION_RANGE_FULL { - let mut index_entries = index + let mut positions = index .walk_revs(&head_positions, &[]) .descendants(&root_positions) + .map(|entry| entry.position()) .collect_vec(); - index_entries.reverse(); - Ok(Box::new(EagerRevset { index_entries })) + positions.reverse(); + Ok(Box::new(EagerRevset { positions })) } else { // For small generation range, it might be better to build a reachable map // with generation bit set, which can be calculated incrementally from roots: // reachable[pos] = (reachable[parent_pos] | ...) << 1 - let mut index_entries = index + let mut positions = index .walk_revs(&head_positions, &[]) .descendants_filtered_by_generation( &root_positions, to_u32_generation_range(generation_from_roots)?, ) + .map(|entry| entry.position()) .collect_vec(); - index_entries.reverse(); - Ok(Box::new(EagerRevset { index_entries })) + positions.reverse(); + Ok(Box::new(EagerRevset { positions })) } } ResolvedExpression::Heads(candidates) => { @@ -711,12 +723,8 @@ impl<'index> EvaluationContext<'index> { .map(|entry| entry.position()) .collect(), ); - let index_entries = head_positions - .into_iter() - .rev() - .map(|pos| index.entry_by_pos(pos)) - .collect(); - Ok(Box::new(EagerRevset { index_entries })) + let positions = head_positions.into_iter().rev().collect(); + Ok(Box::new(EagerRevset { positions })) } ResolvedExpression::Roots(candidates) => { let candidate_entries = self.evaluate(candidates)?.iter(index).collect_vec(); @@ -728,17 +736,17 @@ impl<'index> EvaluationContext<'index> { .walk_revs(&candidate_positions, &[]) .descendants(&candidate_positions) .collect_positions_set(); - let mut index_entries = vec![]; + let mut positions = vec![]; for candidate in candidate_entries { if !candidate .parent_positions() .iter() .any(|parent| filled.contains(parent)) { - index_entries.push(candidate); + positions.push(candidate.position()); } } - Ok(Box::new(EagerRevset { index_entries })) + Ok(Box::new(EagerRevset { positions })) } ResolvedExpression::Latest { candidates, count } => { let candidate_set = self.evaluate(candidates)?; @@ -796,36 +804,36 @@ impl<'index> EvaluationContext<'index> { } } - fn revset_for_commit_ids(&self, commit_ids: &[CommitId]) -> EagerRevset<'index> { - let mut index_entries = vec![]; - for id in commit_ids { - index_entries.push(self.index.entry_by_id(id).unwrap()); - } - index_entries.sort_unstable_by_key(|b| Reverse(b.position())); - index_entries.dedup(); - EagerRevset { index_entries } + fn revset_for_commit_ids(&self, commit_ids: &[CommitId]) -> EagerRevset { + let mut positions = commit_ids + .iter() + .map(|id| self.index.commit_id_to_pos(id).unwrap()) + .collect_vec(); + positions.sort_unstable_by_key(|&pos| Reverse(pos)); + positions.dedup(); + EagerRevset { positions } } fn take_latest_revset( &self, candidate_set: &dyn InternalRevset<'index>, count: usize, - ) -> EagerRevset<'index> { + ) -> EagerRevset { if count == 0 { return EagerRevset::empty(); } #[derive(Clone, Eq, Ord, PartialEq, PartialOrd)] - struct Item<'a> { + struct Item { timestamp: MillisSinceEpoch, - entry: IndexEntryByPosition<'a>, // tie-breaker + pos: IndexPosition, // tie-breaker } - let make_rev_item = |entry: IndexEntry<'index>| { + let make_rev_item = |entry: IndexEntry<'_>| { let commit = self.store.get_commit(&entry.commit_id()).unwrap(); Reverse(Item { timestamp: commit.committer().timestamp.timestamp.clone(), - entry: IndexEntryByPosition(entry), + pos: entry.position(), }) }; @@ -842,12 +850,12 @@ impl<'index> EvaluationContext<'index> { } assert!(latest_items.len() <= count); - let mut index_entries = latest_items + let mut positions = latest_items .into_iter() - .map(|item| item.0.entry.0) + .map(|item| item.0.pos) .collect_vec(); - index_entries.sort_unstable_by_key(|b| Reverse(b.position())); - EagerRevset { index_entries } + positions.sort_unstable_by_key(|&pos| Reverse(pos)); + EagerRevset { positions } } } @@ -976,11 +984,12 @@ mod tests { index.add_commit_data(id_3.clone(), new_change_id(), &[id_2.clone()]); index.add_commit_data(id_4.clone(), new_change_id(), &[id_3.clone()]); + let get_pos = |id: &CommitId| index.as_composite().commit_id_to_pos(id).unwrap(); let get_entry = |id: &CommitId| index.as_composite().entry_by_id(id).unwrap(); let make_entries = |ids: &[&CommitId]| ids.iter().map(|id| get_entry(id)).collect_vec(); let make_set = |ids: &[&CommitId]| -> Box { - let index_entries = make_entries(ids); - Box::new(EagerRevset { index_entries }) + let positions = ids.iter().copied().map(get_pos).collect(); + Box::new(EagerRevset { positions }) }; let set = make_set(&[&id_4, &id_3, &id_2, &id_0]); From 8d3fc6eecd4d9ac3e66f2a1123fb6937bb11a720 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 26 May 2023 17:12:35 +0900 Subject: [PATCH 4/7] revset: store RevWalk factory function in RevWalkRevset The returned iterator is boxed by caller due to the limitation of the type system. There's a workaround, but it's super ugly. https://users.rust-lang.org/t/hrtb-on-multiple-generics/34255/3 --- lib/src/default_revset_engine.rs | 87 ++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 26 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index e91f223738..4b242a85f0 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -219,28 +219,47 @@ impl ToPredicateFn for EagerRevset { } } -struct RevWalkRevset { - walk: T, +struct RevWalkRevset { + walk: F, } -impl fmt::Debug for RevWalkRevset { +impl RevWalkRevset +where + // Returns trait object because we can't express the following constraints + // without using named lifetime and type parameter: + // + // for<'index> + // F: Fn(CompositeIndex<'index>) -> _, + // F::Output: Iterator> + 'index + // + // There's a workaround, but it doesn't help infer closure types. + // https://github.com/rust-lang/rust/issues/47815 + // https://users.rust-lang.org/t/hrtb-on-multiple-generics/34255 + F: Fn(CompositeIndex<'_>) -> Box> + '_>, +{ + fn new(walk: F) -> Self { + RevWalkRevset { walk } + } +} + +impl fmt::Debug for RevWalkRevset { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("RevWalkRevset").finish_non_exhaustive() } } -impl<'index, T> InternalRevset<'index> for RevWalkRevset +impl<'index, F> InternalRevset<'index> for RevWalkRevset where - T: Iterator> + Clone, + F: Fn(CompositeIndex<'_>) -> Box> + '_>, { fn iter<'a>( &'a self, - _index: CompositeIndex<'index>, + index: CompositeIndex<'index>, ) -> Box> + 'a> where 'index: 'a, { - Box::new(self.walk.clone()) + (self.walk)(index) } fn into_predicate<'a>(self: Box) -> Box @@ -251,16 +270,15 @@ where } } -impl<'index, T> ToPredicateFn for RevWalkRevset +impl ToPredicateFn for RevWalkRevset where - T: Iterator> + Clone, + F: Fn(CompositeIndex<'_>) -> Box> + '_>, { - // TODO: remove 'index from RevWalkRevset<'index, F> - fn to_predicate_fn<'a, 'index2: 'a>( + fn to_predicate_fn<'a, 'index: 'a>( &'a self, - _index: CompositeIndex<'index2>, + index: CompositeIndex<'index>, ) -> Box) -> bool + 'a> { - predicate_fn_from_entries(self.walk.clone()) + predicate_fn_from_entries(self.iter(index)) } } @@ -627,12 +645,19 @@ impl<'index> EvaluationContext<'index> { .iter(index) .map(|entry| entry.position()) .collect_vec(); - let walk = self.index.walk_revs(&head_positions, &[]); if generation == &GENERATION_RANGE_FULL { - Ok(Box::new(RevWalkRevset { walk })) + Ok(Box::new(RevWalkRevset::new(move |index| { + Box::new(index.walk_revs(&head_positions, &[])) + }))) } else { - let walk = walk.filter_by_generation(to_u32_generation_range(generation)?); - Ok(Box::new(RevWalkRevset { walk })) + let generation = to_u32_generation_range(generation)?; + Ok(Box::new(RevWalkRevset::new(move |index| { + Box::new( + index + .walk_revs(&head_positions, &[]) + .filter_by_generation(generation.clone()), + ) + }))) } } ResolvedExpression::Range { @@ -650,12 +675,19 @@ impl<'index> EvaluationContext<'index> { .iter(index) .map(|entry| entry.position()) .collect_vec(); - let walk = self.index.walk_revs(&head_positions, &root_positions); if generation == &GENERATION_RANGE_FULL { - Ok(Box::new(RevWalkRevset { walk })) + Ok(Box::new(RevWalkRevset::new(move |index| { + Box::new(index.walk_revs(&head_positions, &root_positions)) + }))) } else { - let walk = walk.filter_by_generation(to_u32_generation_range(generation)?); - Ok(Box::new(RevWalkRevset { walk })) + let generation = to_u32_generation_range(generation)?; + Ok(Box::new(RevWalkRevset::new(move |index| { + Box::new( + index + .walk_revs(&head_positions, &root_positions) + .filter_by_generation(generation.clone()), + ) + }))) } } ResolvedExpression::DagRange { @@ -674,11 +706,14 @@ impl<'index> EvaluationContext<'index> { .map(|entry| entry.position()) .collect_vec(); if generation_from_roots == &(1..2) { - let walk = index - .walk_revs(&head_positions, &[]) - .take_until_roots(&root_positions); - let root_positions_set: HashSet<_> = root_positions.into_iter().collect(); - let candidates = Box::new(RevWalkRevset { walk }); + let root_positions_set: HashSet<_> = root_positions.iter().copied().collect(); + let candidates = Box::new(RevWalkRevset::new(move |index| { + Box::new( + index + .walk_revs(&head_positions, &[]) + .take_until_roots(&root_positions), + ) + })); let predicate = PurePredicateFn(move |entry: &IndexEntry| { entry .parent_positions() From 06be65dab0aece30fb1bc0400dd06083ca726b08 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 7 Dec 2023 22:46:05 +0900 Subject: [PATCH 5/7] revset: add helper that coerces closure to PurePredicateFn Also renamed the boxed version to discriminate it from the cast helper. --- lib/src/default_revset_engine.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 4b242a85f0..8a15a5c2cb 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -714,7 +714,7 @@ impl<'index> EvaluationContext<'index> { .take_until_roots(&root_positions), ) })); - let predicate = PurePredicateFn(move |entry: &IndexEntry| { + let predicate = as_pure_predicate_fn(move |entry| { entry .parent_positions() .iter() @@ -911,7 +911,14 @@ impl) -> bool> ToPredicateFn for PurePredicateFn { } } -fn pure_predicate_fn<'index>( +fn as_pure_predicate_fn(f: F) -> PurePredicateFn +where + F: Fn(&IndexEntry<'_>) -> bool, +{ + PurePredicateFn(f) +} + +fn box_pure_predicate_fn<'index>( f: impl Fn(&IndexEntry<'_>) -> bool + 'index, ) -> Box { Box::new(PurePredicateFn(f)) @@ -925,11 +932,11 @@ fn build_predicate_fn<'index>( match predicate { RevsetFilterPredicate::ParentCount(parent_count_range) => { let parent_count_range = parent_count_range.clone(); - pure_predicate_fn(move |entry| parent_count_range.contains(&entry.num_parents())) + box_pure_predicate_fn(move |entry| parent_count_range.contains(&entry.num_parents())) } RevsetFilterPredicate::Description(pattern) => { let pattern = pattern.clone(); - pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(commit.description()) }) @@ -939,14 +946,14 @@ fn build_predicate_fn<'index>( // TODO: Make these functions that take a needle to search for accept some // syntax for specifying whether it's a regex and whether it's // case-sensitive. - pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(&commit.author().name) || pattern.matches(&commit.author().email) }) } RevsetFilterPredicate::Committer(pattern) => { let pattern = pattern.clone(); - pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(&commit.committer().name) || pattern.matches(&commit.committer().email) @@ -959,11 +966,11 @@ fn build_predicate_fn<'index>( } else { Box::new(EverythingMatcher) }; - pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |entry| { has_diff_from_parent(&store, index, entry, matcher.as_ref()) }) } - RevsetFilterPredicate::HasConflict => pure_predicate_fn(move |entry| { + RevsetFilterPredicate::HasConflict => box_pure_predicate_fn(move |entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); commit.has_conflict().unwrap() }), @@ -1042,7 +1049,7 @@ mod tests { let set = FilterRevset { candidates: make_set(&[&id_4, &id_2, &id_0]), - predicate: pure_predicate_fn(|entry| entry.commit_id() != id_4), + predicate: as_pure_predicate_fn(|entry| entry.commit_id() != id_4), }; assert_eq!( set.iter(index.as_composite()).collect_vec(), From edd292d192ae9d4adf941698957db6005fb63650 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 7 Dec 2023 22:54:59 +0900 Subject: [PATCH 6/7] revset: pass in index to PurePredicateFn as an argument to make it 'static --- lib/src/default_revset_engine.rs | 49 +++++++++++++++++--------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 8a15a5c2cb..f268a09dcf 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -714,7 +714,7 @@ impl<'index> EvaluationContext<'index> { .take_until_roots(&root_positions), ) })); - let predicate = as_pure_predicate_fn(move |entry| { + let predicate = as_pure_predicate_fn(move |_index, entry| { entry .parent_positions() .iter() @@ -819,11 +819,9 @@ impl<'index> EvaluationContext<'index> { expression: &ResolvedPredicateExpression, ) -> Result, RevsetEvaluationError> { match expression { - ResolvedPredicateExpression::Filter(predicate) => Ok(build_predicate_fn( - self.store.clone(), - self.index, - predicate, - )), + ResolvedPredicateExpression::Filter(predicate) => { + Ok(build_predicate_fn(self.store.clone(), predicate)) + } ResolvedPredicateExpression::Set(expression) => { Ok(self.evaluate(expression)?.into_predicate()) } @@ -902,41 +900,46 @@ impl fmt::Debug for PurePredicateFn { } } -impl) -> bool> ToPredicateFn for PurePredicateFn { +impl ToPredicateFn for PurePredicateFn +where + F: Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool, +{ fn to_predicate_fn<'a, 'index: 'a>( &'a self, - _index: CompositeIndex<'index>, + index: CompositeIndex<'index>, ) -> Box) -> bool + 'a> { - Box::new(&self.0) + let f = &self.0; + Box::new(move |entry| f(index, entry)) } } fn as_pure_predicate_fn(f: F) -> PurePredicateFn where - F: Fn(&IndexEntry<'_>) -> bool, + F: Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool, { PurePredicateFn(f) } -fn box_pure_predicate_fn<'index>( - f: impl Fn(&IndexEntry<'_>) -> bool + 'index, -) -> Box { +fn box_pure_predicate_fn<'a>( + f: impl Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool + 'a, +) -> Box { Box::new(PurePredicateFn(f)) } -fn build_predicate_fn<'index>( +fn build_predicate_fn( store: Arc, - index: CompositeIndex<'index>, predicate: &RevsetFilterPredicate, -) -> Box { +) -> Box { match predicate { RevsetFilterPredicate::ParentCount(parent_count_range) => { let parent_count_range = parent_count_range.clone(); - box_pure_predicate_fn(move |entry| parent_count_range.contains(&entry.num_parents())) + box_pure_predicate_fn(move |_index, entry| { + parent_count_range.contains(&entry.num_parents()) + }) } RevsetFilterPredicate::Description(pattern) => { let pattern = pattern.clone(); - box_pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |_index, entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(commit.description()) }) @@ -946,14 +949,14 @@ fn build_predicate_fn<'index>( // TODO: Make these functions that take a needle to search for accept some // syntax for specifying whether it's a regex and whether it's // case-sensitive. - box_pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |_index, entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(&commit.author().name) || pattern.matches(&commit.author().email) }) } RevsetFilterPredicate::Committer(pattern) => { let pattern = pattern.clone(); - box_pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |_index, entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); pattern.matches(&commit.committer().name) || pattern.matches(&commit.committer().email) @@ -966,11 +969,11 @@ fn build_predicate_fn<'index>( } else { Box::new(EverythingMatcher) }; - box_pure_predicate_fn(move |entry| { + box_pure_predicate_fn(move |index, entry| { has_diff_from_parent(&store, index, entry, matcher.as_ref()) }) } - RevsetFilterPredicate::HasConflict => box_pure_predicate_fn(move |entry| { + RevsetFilterPredicate::HasConflict => box_pure_predicate_fn(move |_index, entry| { let commit = store.get_commit(&entry.commit_id()).unwrap(); commit.has_conflict().unwrap() }), @@ -1049,7 +1052,7 @@ mod tests { let set = FilterRevset { candidates: make_set(&[&id_4, &id_2, &id_0]), - predicate: as_pure_predicate_fn(|entry| entry.commit_id() != id_4), + predicate: as_pure_predicate_fn(|_index, entry| entry.commit_id() != id_4), }; assert_eq!( set.iter(index.as_composite()).collect_vec(), From ad32d769f037e604eedb67471f96c78b2113e600 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Thu, 7 Dec 2023 20:49:17 +0900 Subject: [PATCH 7/7] revset: remove 'index lifetime from InternalRevset --- lib/src/default_revset_engine.rs | 110 ++++++++++++------------------- 1 file changed, 41 insertions(+), 69 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index f268a09dcf..3e91ab37ca 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -57,15 +57,12 @@ impl ToPredicateFn for Box { } } -// TODO: move <'index> parameter to iter<'a, 'index: 'a> -trait InternalRevset<'index>: fmt::Debug + ToPredicateFn { +trait InternalRevset: fmt::Debug + ToPredicateFn { // All revsets currently iterate in order of descending index position - fn iter<'a>( + fn iter<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, - ) -> Box> + 'a> - where - 'index: 'a; + ) -> Box> + 'a>; fn into_predicate<'a>(self: Box) -> Box where @@ -73,15 +70,12 @@ trait InternalRevset<'index>: fmt::Debug + ToPredicateFn { } pub struct RevsetImpl<'index> { - inner: Box + 'index>, + inner: Box, index: CompositeIndex<'index>, } impl<'index> RevsetImpl<'index> { - fn new( - revset: Box + 'index>, - index: CompositeIndex<'index>, - ) -> Self { + fn new(revset: Box, index: CompositeIndex<'index>) -> Self { Self { inner: revset, index, @@ -187,14 +181,11 @@ impl EagerRevset { } } -impl<'index> InternalRevset<'index> for EagerRevset { - fn iter<'a>( +impl InternalRevset for EagerRevset { + fn iter<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, - ) -> Box> + 'a> - where - 'index: 'a, - { + ) -> Box> + 'a> { let entries = self .positions .iter() @@ -248,17 +239,14 @@ impl fmt::Debug for RevWalkRevset { } } -impl<'index, F> InternalRevset<'index> for RevWalkRevset +impl InternalRevset for RevWalkRevset where F: Fn(CompositeIndex<'_>) -> Box> + '_>, { - fn iter<'a>( + fn iter<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, - ) -> Box> + 'a> - where - 'index: 'a, - { + ) -> Box> + 'a> { (self.walk)(index) } @@ -301,19 +289,16 @@ fn predicate_fn_from_positions<'iter>( } #[derive(Debug)] -struct FilterRevset<'index, P> { - candidates: Box + 'index>, +struct FilterRevset

{ + candidates: Box, predicate: P, } -impl<'index, P: ToPredicateFn> InternalRevset<'index> for FilterRevset<'index, P> { - fn iter<'a>( +impl InternalRevset for FilterRevset

{ + fn iter<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, - ) -> Box> + 'a> - where - 'index: 'a, - { + ) -> Box> + 'a> { let p = self.predicate.to_predicate_fn(index); Box::new(self.candidates.iter(index).filter(p)) } @@ -326,7 +311,7 @@ impl<'index, P: ToPredicateFn> InternalRevset<'index> for FilterRevset<'index, P } } -impl ToPredicateFn for FilterRevset<'_, P> { +impl ToPredicateFn for FilterRevset

{ fn to_predicate_fn<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, @@ -351,19 +336,16 @@ impl ToPredicateFn for NotInPredicate { } #[derive(Debug)] -struct UnionRevset<'index> { - set1: Box + 'index>, - set2: Box + 'index>, +struct UnionRevset { + set1: Box, + set2: Box, } -impl<'index> InternalRevset<'index> for UnionRevset<'index> { - fn iter<'a>( +impl InternalRevset for UnionRevset { + fn iter<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, - ) -> Box> + 'a> - where - 'index: 'a, - { + ) -> Box> + 'a> { Box::new(UnionRevsetIterator { iter1: self.set1.iter(index).peekable(), iter2: self.set2.iter(index).peekable(), @@ -378,7 +360,7 @@ impl<'index> InternalRevset<'index> for UnionRevset<'index> { } } -impl ToPredicateFn for UnionRevset<'_> { +impl ToPredicateFn for UnionRevset { fn to_predicate_fn<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, @@ -441,19 +423,16 @@ impl<'index, I1: Iterator>, I2: Iterator { - set1: Box + 'index>, - set2: Box + 'index>, +struct IntersectionRevset { + set1: Box, + set2: Box, } -impl<'index> InternalRevset<'index> for IntersectionRevset<'index> { - fn iter<'a>( +impl InternalRevset for IntersectionRevset { + fn iter<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, - ) -> Box> + 'a> - where - 'index: 'a, - { + ) -> Box> + 'a> { Box::new(IntersectionRevsetIterator { iter1: self.set1.iter(index).peekable(), iter2: self.set2.iter(index).peekable(), @@ -468,7 +447,7 @@ impl<'index> InternalRevset<'index> for IntersectionRevset<'index> { } } -impl ToPredicateFn for IntersectionRevset<'_> { +impl ToPredicateFn for IntersectionRevset { fn to_predicate_fn<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, @@ -520,21 +499,18 @@ impl<'index, I1: Iterator>, I2: Iterator { +struct DifferenceRevset { // The minuend (what to subtract from) - set1: Box + 'index>, + set1: Box, // The subtrahend (what to subtract) - set2: Box + 'index>, + set2: Box, } -impl<'index> InternalRevset<'index> for DifferenceRevset<'index> { - fn iter<'a>( +impl InternalRevset for DifferenceRevset { + fn iter<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, - ) -> Box> + 'a> - where - 'index: 'a, - { + ) -> Box> + 'a> { Box::new(DifferenceRevsetIterator { iter1: self.set1.iter(index).peekable(), iter2: self.set2.iter(index).peekable(), @@ -549,7 +525,7 @@ impl<'index> InternalRevset<'index> for DifferenceRevset<'index> { } } -impl ToPredicateFn for DifferenceRevset<'_> { +impl ToPredicateFn for DifferenceRevset { fn to_predicate_fn<'a, 'index: 'a>( &'a self, index: CompositeIndex<'index>, @@ -633,7 +609,7 @@ impl<'index> EvaluationContext<'index> { fn evaluate( &self, expression: &ResolvedExpression, - ) -> Result + 'index>, RevsetEvaluationError> { + ) -> Result, RevsetEvaluationError> { let index = self.index; match expression { ResolvedExpression::Commits(commit_ids) => { @@ -817,7 +793,7 @@ impl<'index> EvaluationContext<'index> { fn evaluate_predicate( &self, expression: &ResolvedPredicateExpression, - ) -> Result, RevsetEvaluationError> { + ) -> Result, RevsetEvaluationError> { match expression { ResolvedPredicateExpression::Filter(predicate) => { Ok(build_predicate_fn(self.store.clone(), predicate)) @@ -847,11 +823,7 @@ impl<'index> EvaluationContext<'index> { EagerRevset { positions } } - fn take_latest_revset( - &self, - candidate_set: &dyn InternalRevset<'index>, - count: usize, - ) -> EagerRevset { + fn take_latest_revset(&self, candidate_set: &dyn InternalRevset, count: usize) -> EagerRevset { if count == 0 { return EagerRevset::empty(); }