From d5c5f38b23386797064ebf4786bd11f534a684c3 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 10 Mar 2024 11:18:00 +0900 Subject: [PATCH 1/4] index: add type alias for "dyn IndexSegment" to clarify it's 'static This helps to migrate CompositeIndex<'_> wrapper to &CompositeIndex. If the wrapped reference had a lifetimed field, it couldn't be represented as a trivial reference type. --- lib/src/default_index/composite.rs | 10 ++++++---- lib/src/default_index/entry.rs | 6 +++--- lib/src/default_index/mod.rs | 10 +++++----- lib/src/default_index/mutable.rs | 6 ++++-- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/lib/src/default_index/composite.rs b/lib/src/default_index/composite.rs index 39fe1565a4..2da1e4dc0b 100644 --- a/lib/src/default_index/composite.rs +++ b/lib/src/default_index/composite.rs @@ -76,6 +76,8 @@ pub(super) trait IndexSegment: Send + Sync { fn parent_positions(&self, local_pos: LocalPosition) -> SmallIndexPositionsVec; } +pub(super) type DynIndexSegment = dyn IndexSegment; + /// Abstraction over owned and borrowed types that can be cheaply converted to /// a `CompositeIndex` reference. pub trait AsCompositeIndex { @@ -97,10 +99,10 @@ impl AsCompositeIndex for &mut T { /// Reference wrapper that provides global access to nested index segments. #[derive(Clone, Copy)] -pub struct CompositeIndex<'a>(&'a dyn IndexSegment); +pub struct CompositeIndex<'a>(&'a DynIndexSegment); impl<'a> CompositeIndex<'a> { - pub(super) fn new(segment: &'a dyn IndexSegment) -> Self { + pub(super) fn new(segment: &'a DynIndexSegment) -> Self { CompositeIndex(segment) } @@ -113,10 +115,10 @@ impl<'a> CompositeIndex<'a> { } /// Iterates self and its ancestor index segments. - pub(super) fn ancestor_index_segments(&self) -> impl Iterator { + pub(super) fn ancestor_index_segments(&self) -> impl Iterator { iter::once(self.0).chain( self.ancestor_files_without_local() - .map(|file| file.as_ref() as &dyn IndexSegment), + .map(|file| file.as_ref() as &DynIndexSegment), ) } diff --git a/lib/src/default_index/entry.rs b/lib/src/default_index/entry.rs index 56a0d630f4..7cfbfdfa30 100644 --- a/lib/src/default_index/entry.rs +++ b/lib/src/default_index/entry.rs @@ -19,7 +19,7 @@ use std::hash::{Hash, Hasher}; use smallvec::SmallVec; -use super::composite::{CompositeIndex, IndexSegment}; +use super::composite::{CompositeIndex, DynIndexSegment}; use crate::backend::{ChangeId, CommitId}; use crate::object_id::ObjectId; @@ -43,7 +43,7 @@ pub(super) type SmallLocalPositionsVec = SmallVec<[LocalPosition; 4]>; #[derive(Clone)] pub struct IndexEntry<'a> { - source: &'a dyn IndexSegment, + source: &'a DynIndexSegment, pos: IndexPosition, /// Position within the source segment local_pos: LocalPosition, @@ -75,7 +75,7 @@ impl Hash for IndexEntry<'_> { impl<'a> IndexEntry<'a> { pub(super) fn new( - source: &'a dyn IndexSegment, + source: &'a DynIndexSegment, pos: IndexPosition, local_pos: LocalPosition, ) -> Self { diff --git a/lib/src/default_index/mod.rs b/lib/src/default_index/mod.rs index 5bb8610a15..cd833b78c1 100644 --- a/lib/src/default_index/mod.rs +++ b/lib/src/default_index/mod.rs @@ -37,7 +37,7 @@ mod tests { use smallvec::smallvec_inline; use test_case::test_case; - use super::composite::IndexSegment; + use super::composite::{DynIndexSegment, IndexSegment}; use super::entry::SmallIndexPositionsVec; use super::mutable::MutableIndexSegment; use super::*; @@ -63,7 +63,7 @@ mod tests { fn index_empty(on_disk: bool) { let temp_dir = testutils::new_temp_dir(); let mutable_segment = MutableIndexSegment::full(3, 16); - let index_segment: Box = if on_disk { + let index_segment: Box = if on_disk { let saved_index = mutable_segment.save_in(temp_dir.path()).unwrap(); Box::new(Arc::try_unwrap(saved_index).unwrap()) } else { @@ -94,7 +94,7 @@ mod tests { let id_0 = CommitId::from_hex("000000"); let change_id0 = new_change_id(); mutable_segment.add_commit_data(id_0.clone(), change_id0.clone(), &[]); - let index_segment: Box = if on_disk { + let index_segment: Box = if on_disk { let saved_index = mutable_segment.save_in(temp_dir.path()).unwrap(); Box::new(Arc::try_unwrap(saved_index).unwrap()) } else { @@ -179,7 +179,7 @@ mod tests { mutable_segment.add_commit_data(id_3.clone(), change_id3.clone(), &[id_2.clone()]); mutable_segment.add_commit_data(id_4.clone(), change_id4, &[id_1.clone()]); mutable_segment.add_commit_data(id_5.clone(), change_id5, &[id_4.clone(), id_2.clone()]); - let index_segment: Box = if on_disk { + let index_segment: Box = if on_disk { let saved_index = mutable_segment.save_in(temp_dir.path()).unwrap(); Box::new(Arc::try_unwrap(saved_index).unwrap()) } else { @@ -291,7 +291,7 @@ mod tests { new_change_id(), &[id_1, id_2, id_3, id_4, id_5], ); - let index_segment: Box = if on_disk { + let index_segment: Box = if on_disk { let saved_index = mutable_segment.save_in(temp_dir.path()).unwrap(); Box::new(Arc::try_unwrap(saved_index).unwrap()) } else { diff --git a/lib/src/default_index/mutable.rs b/lib/src/default_index/mutable.rs index f88c121c58..9d17565a73 100644 --- a/lib/src/default_index/mutable.rs +++ b/lib/src/default_index/mutable.rs @@ -29,7 +29,9 @@ use itertools::Itertools; use smallvec::{smallvec, SmallVec}; use tempfile::NamedTempFile; -use super::composite::{AsCompositeIndex, ChangeIdIndexImpl, CompositeIndex, IndexSegment}; +use super::composite::{ + AsCompositeIndex, ChangeIdIndexImpl, CompositeIndex, DynIndexSegment, IndexSegment, +}; use super::entry::{IndexPosition, LocalPosition, SmallIndexPositionsVec, SmallLocalPositionsVec}; use super::readonly::{ DefaultReadonlyIndex, ReadonlyIndexSegment, INDEX_SEGMENT_FILE_FORMAT_VERSION, OVERFLOW_FLAG, @@ -137,7 +139,7 @@ impl MutableIndexSegment { self.graph.push(entry); } - pub(super) fn add_commits_from(&mut self, other_segment: &dyn IndexSegment) { + pub(super) fn add_commits_from(&mut self, other_segment: &DynIndexSegment) { let other = CompositeIndex::new(other_segment); for pos in other_segment.num_parent_commits()..other.num_commits() { let entry = other.entry_by_pos(IndexPosition(pos)); From 49f696fc691a3ebeecc3eae52f0109f64890fc19 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 10 Mar 2024 10:25:27 +0900 Subject: [PATCH 2/4] index: turn CompositeIndex into transparent reference type This helps to eliminate higher-ranked trait bounds from RevWalkRevset and RevWalk combinators to be added. Since &CompositeIndex is now a real reference, it can be passed to functions as index: &T. --- lib/src/default_index/composite.rs | 43 ++++++------ lib/src/default_index/mutable.rs | 4 +- lib/src/default_index/readonly.rs | 4 +- lib/src/default_index/rev_walk.rs | 22 +++--- lib/src/default_index/revset_engine.rs | 70 +++++++++---------- .../default_index/revset_graph_iterator.rs | 6 +- lib/tests/test_index.rs | 6 +- 7 files changed, 78 insertions(+), 77 deletions(-) diff --git a/lib/src/default_index/composite.rs b/lib/src/default_index/composite.rs index 2da1e4dc0b..a75caf442a 100644 --- a/lib/src/default_index/composite.rs +++ b/lib/src/default_index/composite.rs @@ -20,6 +20,7 @@ use std::iter; use std::sync::{Arc, Mutex}; use itertools::Itertools; +use ref_cast::{ref_cast_custom, RefCastCustom}; use super::entry::{ IndexEntry, IndexPosition, IndexPositionByGeneration, LocalPosition, SmallIndexPositionsVec, @@ -82,41 +83,41 @@ pub(super) type DynIndexSegment = dyn IndexSegment; /// a `CompositeIndex` reference. pub trait AsCompositeIndex { /// Returns reference wrapper that provides global access to this index. - fn as_composite(&self) -> CompositeIndex<'_>; + fn as_composite(&self) -> &CompositeIndex; } impl AsCompositeIndex for &T { - fn as_composite(&self) -> CompositeIndex<'_> { + fn as_composite(&self) -> &CompositeIndex { ::as_composite(self) } } impl AsCompositeIndex for &mut T { - fn as_composite(&self) -> CompositeIndex<'_> { + fn as_composite(&self) -> &CompositeIndex { ::as_composite(self) } } /// Reference wrapper that provides global access to nested index segments. -#[derive(Clone, Copy)] -pub struct CompositeIndex<'a>(&'a DynIndexSegment); +#[derive(RefCastCustom)] +#[repr(transparent)] +pub struct CompositeIndex(DynIndexSegment); -impl<'a> CompositeIndex<'a> { - pub(super) fn new(segment: &'a DynIndexSegment) -> Self { - CompositeIndex(segment) - } +impl CompositeIndex { + #[ref_cast_custom] + pub(super) const fn new(segment: &DynIndexSegment) -> &Self; /// Iterates parent and its ancestor readonly index segments. pub(super) fn ancestor_files_without_local( &self, - ) -> impl Iterator> { + ) -> impl Iterator> { let parent_file = self.0.parent_file(); iter::successors(parent_file, |file| file.parent_file()) } /// Iterates self and its ancestor index segments. - pub(super) fn ancestor_index_segments(&self) -> impl Iterator { - iter::once(self.0).chain( + pub(super) fn ancestor_index_segments(&self) -> impl Iterator { + iter::once(&self.0).chain( self.ancestor_files_without_local() .map(|file| file.as_ref() as &DynIndexSegment), ) @@ -160,7 +161,7 @@ impl<'a> CompositeIndex<'a> { } } - pub fn entry_by_pos(&self, pos: IndexPosition) -> IndexEntry<'a> { + pub fn entry_by_pos(&self, pos: IndexPosition) -> IndexEntry<'_> { self.ancestor_index_segments() .find_map(|segment| { u32::checked_sub(pos.0, segment.num_parent_commits()) @@ -169,7 +170,7 @@ impl<'a> CompositeIndex<'a> { .unwrap() } - pub fn entry_by_id(&self, commit_id: &CommitId) -> Option> { + pub fn entry_by_id(&self, commit_id: &CommitId) -> Option> { self.ancestor_index_segments().find_map(|segment| { let local_pos = segment.commit_id_to_pos(commit_id)?; let pos = IndexPosition(local_pos.0 + segment.num_parent_commits()); @@ -332,7 +333,7 @@ impl<'a> CompositeIndex<'a> { self.heads_pos(result) } - pub(super) fn all_heads(self) -> impl Iterator + 'a { + pub(super) fn all_heads(&self) -> impl Iterator + '_ { self.all_heads_pos() .map(move |pos| self.entry_by_pos(pos).commit_id()) } @@ -392,19 +393,19 @@ impl<'a> CompositeIndex<'a> { &self, expression: &ResolvedExpression, store: &Arc, - ) -> Result, RevsetEvaluationError> { - let revset_impl = revset_engine::evaluate(expression, store, *self)?; + ) -> Result, RevsetEvaluationError> { + let revset_impl = revset_engine::evaluate(expression, store, self)?; Ok(Box::new(revset_impl)) } } -impl AsCompositeIndex for CompositeIndex<'_> { - fn as_composite(&self) -> CompositeIndex<'_> { - *self +impl AsCompositeIndex for &CompositeIndex { + fn as_composite(&self) -> &CompositeIndex { + self } } -impl Index for CompositeIndex<'_> { +impl Index for &CompositeIndex { /// Suppose the given `commit_id` exists, returns the minimum prefix length /// to disambiguate it. The length to be returned is a number of hexadecimal /// digits. diff --git a/lib/src/default_index/mutable.rs b/lib/src/default_index/mutable.rs index 9d17565a73..67cc2dfbf6 100644 --- a/lib/src/default_index/mutable.rs +++ b/lib/src/default_index/mutable.rs @@ -90,7 +90,7 @@ impl MutableIndexSegment { } } - pub(super) fn as_composite(&self) -> CompositeIndex { + pub(super) fn as_composite(&self) -> &CompositeIndex { CompositeIndex::new(self) } @@ -467,7 +467,7 @@ impl DefaultMutableIndex { } impl AsCompositeIndex for DefaultMutableIndex { - fn as_composite(&self) -> CompositeIndex<'_> { + fn as_composite(&self) -> &CompositeIndex { self.0.as_composite() } } diff --git a/lib/src/default_index/readonly.rs b/lib/src/default_index/readonly.rs index d8822b3cdf..8365b45581 100644 --- a/lib/src/default_index/readonly.rs +++ b/lib/src/default_index/readonly.rs @@ -342,7 +342,7 @@ impl ReadonlyIndexSegment { })) } - pub(super) fn as_composite(&self) -> CompositeIndex { + pub(super) fn as_composite(&self) -> &CompositeIndex { CompositeIndex::new(self) } @@ -570,7 +570,7 @@ impl DefaultReadonlyIndex { } impl AsCompositeIndex for DefaultReadonlyIndex { - fn as_composite(&self) -> CompositeIndex<'_> { + fn as_composite(&self) -> &CompositeIndex { self.0.as_composite() } } diff --git a/lib/src/default_index/rev_walk.rs b/lib/src/default_index/rev_walk.rs index 47ac17d242..0758fa4dd9 100644 --- a/lib/src/default_index/rev_walk.rs +++ b/lib/src/default_index/rev_walk.rs @@ -100,7 +100,7 @@ pub(super) trait RevWalkIndex { fn adjacent_positions(&self, pos: Self::Position) -> Self::AdjacentPositions; } -impl RevWalkIndex for CompositeIndex<'_> { +impl RevWalkIndex for &CompositeIndex { type Position = IndexPosition; type AdjacentPositions = SmallIndexPositionsVec; @@ -118,7 +118,7 @@ pub(super) struct RevWalkDescendantsIndex { type DescendantIndexPositionsVec = SmallVec<[Reverse; 4]>; impl RevWalkDescendantsIndex { - fn build(index: CompositeIndex, positions: impl IntoIterator) -> Self { + fn build(index: &CompositeIndex, positions: impl IntoIterator) -> Self { // For dense set, it's probably cheaper to use `Vec` instead of `HashMap`. let mut children_map: HashMap = HashMap::new(); for pos in positions { @@ -242,13 +242,13 @@ impl RevWalkQueue { #[derive(Clone)] #[must_use] pub(super) struct RevWalkBuilder<'a> { - index: CompositeIndex<'a>, + index: &'a CompositeIndex, wanted: Vec, unwanted: Vec, } impl<'a> RevWalkBuilder<'a> { - pub fn new(index: CompositeIndex<'a>) -> Self { + pub fn new(index: &'a CompositeIndex) -> Self { RevWalkBuilder { index, wanted: Vec::new(), @@ -383,7 +383,7 @@ impl<'a> RevWalkBuilder<'a> { } pub(super) type RevWalkAncestors<'a> = - RevWalkBorrowedIndexIter, RevWalkImpl>; + RevWalkBorrowedIndexIter<&'a CompositeIndex, RevWalkImpl>; #[derive(Clone)] #[must_use] @@ -420,7 +420,7 @@ impl RevWalk for RevWalkImpl { } pub(super) type RevWalkAncestorsGenerationRange<'a> = - RevWalkBorrowedIndexIter, RevWalkGenerationRangeImpl>; + RevWalkBorrowedIndexIter<&'a CompositeIndex, RevWalkGenerationRangeImpl>; pub(super) type RevWalkDescendantsGenerationRange = RevWalkOwnedIndexIter< RevWalkDescendantsIndex, RevWalkGenerationRangeImpl>, @@ -540,7 +540,7 @@ impl RevWalkItemGenerationRange { /// Walks descendants from the roots, in order of ascending index position. pub(super) type RevWalkDescendants<'a> = - RevWalkBorrowedIndexIter, RevWalkDescendantsImpl>; + RevWalkBorrowedIndexIter<&'a CompositeIndex, RevWalkDescendantsImpl>; #[derive(Clone)] #[must_use] @@ -561,10 +561,10 @@ impl RevWalkDescendants<'_> { } } -impl RevWalk> for RevWalkDescendantsImpl { +impl RevWalk<&CompositeIndex> for RevWalkDescendantsImpl { type Item = IndexPosition; - fn next(&mut self, index: &CompositeIndex) -> Option { + fn next(&mut self, index: &&CompositeIndex) -> Option { while let Some(candidate_pos) = self.candidate_positions.pop() { if self.root_positions.contains(&candidate_pos) || index @@ -626,7 +626,7 @@ impl AncestorsBitSet { } /// Updates set by visiting ancestors until the given `to_visit_pos`. - pub fn visit_until(&mut self, index: CompositeIndex, to_visit_pos: IndexPosition) { + pub fn visit_until(&mut self, index: &CompositeIndex, to_visit_pos: IndexPosition) { let to_visit_bitset_pos = to_visit_pos.0 / u64::BITS; if to_visit_bitset_pos >= self.last_visited_bitset_pos { return; @@ -673,7 +673,7 @@ mod tests { move || iter.next().unwrap() } - fn to_positions_vec(index: CompositeIndex<'_>, commit_ids: &[CommitId]) -> Vec { + fn to_positions_vec(index: &CompositeIndex, commit_ids: &[CommitId]) -> Vec { commit_ids .iter() .map(|id| index.commit_id_to_pos(id).unwrap()) diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index 42ffaae59c..b5846afb4c 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -44,14 +44,14 @@ trait ToPredicateFn: fmt::Debug { /// The predicate function is evaluated in order of `RevsetIterator`. fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a>; } impl ToPredicateFn for Box { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { ::to_predicate_fn(self, index) } @@ -61,12 +61,12 @@ trait InternalRevset: fmt::Debug + ToPredicateFn { // All revsets currently iterate in order of descending index position fn entries<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box> + 'a>; fn positions<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box + 'a>; fn into_predicate<'a>(self: Box) -> Box @@ -77,14 +77,14 @@ trait InternalRevset: fmt::Debug + ToPredicateFn { impl InternalRevset for Box { fn entries<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box> + 'a> { ::entries(self, index) } fn positions<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box + 'a> { ::positions(self, index) } @@ -174,13 +174,13 @@ impl Revset for RevsetImpl { /// Incrementally consumes positions iterator of the revset collecting /// positions. struct PositionsAccumulator<'revset, 'index> { - index: CompositeIndex<'index>, + index: &'index CompositeIndex, inner: RefCell>, } impl<'revset, 'index> PositionsAccumulator<'revset, 'index> { fn new( - index: CompositeIndex<'index>, + index: &'index CompositeIndex, positions_iter: Box + 'revset>, ) -> Self { let inner = RefCell::new(PositionsAccumulatorInner { @@ -255,7 +255,7 @@ impl EagerRevset { impl InternalRevset for EagerRevset { fn entries<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box> + 'a> { let entries = self .positions @@ -266,7 +266,7 @@ impl InternalRevset for EagerRevset { fn positions<'a, 'index: 'a>( &'a self, - _index: CompositeIndex<'index>, + _index: &'index CompositeIndex, ) -> Box + 'a> { Box::new(self.positions.iter().copied()) } @@ -282,7 +282,7 @@ impl InternalRevset for EagerRevset { impl ToPredicateFn for EagerRevset { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - _index: CompositeIndex<'index>, + _index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { predicate_fn_from_positions(self.positions.iter().copied()) } @@ -300,11 +300,11 @@ impl fmt::Debug for RevWalkRevset { impl InternalRevset for RevWalkRevset where - W: for<'index> RevWalk, Item = IndexPosition> + Clone, + W: for<'index> RevWalk<&'index CompositeIndex, Item = IndexPosition> + Clone, { fn entries<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box> + 'a> { let positions = self.walk.clone().attach(index); Box::new(positions.map(move |pos| index.entry_by_pos(pos))) @@ -312,7 +312,7 @@ where fn positions<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box + 'a> { Box::new(self.walk.clone().attach(index)) } @@ -327,11 +327,11 @@ where impl ToPredicateFn for RevWalkRevset where - W: for<'index> RevWalk, Item = IndexPosition> + Clone, + W: for<'index> RevWalk<&'index CompositeIndex, Item = IndexPosition> + Clone, { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { let positions = self.walk.clone().attach(index); predicate_fn_from_positions(positions) @@ -363,7 +363,7 @@ where { fn entries<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box> + 'a> { let p = self.predicate.to_predicate_fn(index); Box::new(self.candidates.entries(index).filter(p)) @@ -371,7 +371,7 @@ where fn positions<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box + 'a> { Box::new(self.entries(index).map(|entry| entry.position())) } @@ -391,7 +391,7 @@ where { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { let mut p1 = self.candidates.to_predicate_fn(index); let mut p2 = self.predicate.to_predicate_fn(index); @@ -405,7 +405,7 @@ struct NotInPredicate(S); impl ToPredicateFn for NotInPredicate { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { let mut p = self.0.to_predicate_fn(index); Box::new(move |entry| !p(entry)) @@ -425,7 +425,7 @@ where { fn entries<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box> + 'a> { Box::new(union_by( self.set1.entries(index), @@ -436,7 +436,7 @@ where fn positions<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box + 'a> { Box::new(union_by( self.set1.positions(index), @@ -460,7 +460,7 @@ where { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { let mut p1 = self.set1.to_predicate_fn(index); let mut p2 = self.set2.to_predicate_fn(index); @@ -531,7 +531,7 @@ where { fn entries<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box> + 'a> { Box::new(intersection_by( self.set1.entries(index), @@ -542,7 +542,7 @@ where fn positions<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box + 'a> { Box::new(intersection_by( self.set1.positions(index), @@ -566,7 +566,7 @@ where { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { let mut p1 = self.set1.to_predicate_fn(index); let mut p2 = self.set2.to_predicate_fn(index); @@ -649,7 +649,7 @@ where { fn entries<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box> + 'a> { Box::new(difference_by( self.set1.entries(index), @@ -660,7 +660,7 @@ where fn positions<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box + 'a> { Box::new(difference_by( self.set1.positions(index), @@ -684,7 +684,7 @@ where { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { let mut p1 = self.set1.to_predicate_fn(index); let mut p2 = self.set2.to_predicate_fn(index); @@ -767,7 +767,7 @@ pub fn evaluate( struct EvaluationContext<'index> { store: Arc, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, } fn to_u32_generation_range(range: &Range) -> Result, RevsetEvaluationError> { @@ -1027,11 +1027,11 @@ impl fmt::Debug for PurePredicateFn { impl ToPredicateFn for PurePredicateFn where - F: Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool, + F: Fn(&CompositeIndex, &IndexEntry<'_>) -> bool, { fn to_predicate_fn<'a, 'index: 'a>( &'a self, - index: CompositeIndex<'index>, + index: &'index CompositeIndex, ) -> Box) -> bool + 'a> { let f = &self.0; Box::new(move |entry| f(index, entry)) @@ -1040,13 +1040,13 @@ where fn as_pure_predicate_fn(f: F) -> PurePredicateFn where - F: Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool, + F: Fn(&CompositeIndex, &IndexEntry<'_>) -> bool, { PurePredicateFn(f) } fn box_pure_predicate_fn<'a>( - f: impl Fn(CompositeIndex<'_>, &IndexEntry<'_>) -> bool + 'a, + f: impl Fn(&CompositeIndex, &IndexEntry<'_>) -> bool + 'a, ) -> Box { Box::new(PurePredicateFn(f)) } @@ -1107,7 +1107,7 @@ fn build_predicate_fn( fn has_diff_from_parent( store: &Arc, - index: CompositeIndex<'_>, + index: &CompositeIndex, entry: &IndexEntry<'_>, matcher: &dyn Matcher, ) -> bool { diff --git a/lib/src/default_index/revset_graph_iterator.rs b/lib/src/default_index/revset_graph_iterator.rs index 7d02bc58ab..8cd9e5f96f 100644 --- a/lib/src/default_index/revset_graph_iterator.rs +++ b/lib/src/default_index/revset_graph_iterator.rs @@ -46,7 +46,7 @@ impl IndexGraphEdge { IndexGraphEdge { target, edge_type } } - fn to_revset_edge(self, index: CompositeIndex<'_>) -> RevsetGraphEdge { + fn to_revset_edge(self, index: &CompositeIndex) -> RevsetGraphEdge { RevsetGraphEdge { target: index.entry_by_pos(self.target).commit_id(), edge_type: self.edge_type, @@ -121,7 +121,7 @@ impl IndexGraphEdge { /// could lead to "D", but that would require extra book-keeping to remember for /// later that the edges from "f" and "H" are only partially computed. pub struct RevsetGraphIterator<'revset, 'index> { - index: CompositeIndex<'index>, + index: &'index CompositeIndex, input_set_iter: Box> + 'revset>, /// Commits in the input set we had to take out of the iterator while /// walking external edges. Does not necessarily include the commit @@ -137,7 +137,7 @@ pub struct RevsetGraphIterator<'revset, 'index> { impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> { pub fn new( - index: CompositeIndex<'index>, + index: &'index CompositeIndex, input_set_iter: Box> + 'revset>, ) -> RevsetGraphIterator<'revset, 'index> { RevsetGraphIterator { diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index 0cbd3208a7..50bf815e4d 100644 --- a/lib/tests/test_index.rs +++ b/lib/tests/test_index.rs @@ -46,7 +46,7 @@ fn child_commit<'repo>( } // Helper just to reduce line wrapping -fn generation_number(index: CompositeIndex, commit_id: &CommitId) -> u32 { +fn generation_number(index: &CompositeIndex, commit_id: &CommitId) -> u32 { index.entry_by_id(commit_id).unwrap().generation_number() } @@ -510,7 +510,7 @@ fn create_n_commits( tx.commit("test") } -fn as_readonly_composite(repo: &Arc) -> CompositeIndex<'_> { +fn as_readonly_composite(repo: &Arc) -> &CompositeIndex { repo.readonly_index() .as_any() .downcast_ref::() @@ -518,7 +518,7 @@ fn as_readonly_composite(repo: &Arc) -> CompositeIndex<'_> { .as_composite() } -fn as_mutable_composite(repo: &MutableRepo) -> CompositeIndex<'_> { +fn as_mutable_composite(repo: &MutableRepo) -> &CompositeIndex { repo.mutable_index() .as_any() .downcast_ref::() From 6b07aa1566d16342e170bbf2514e89d361201d43 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 10 Mar 2024 21:48:47 +0900 Subject: [PATCH 3/4] index: implement AsCompositeIndex for CompositeIndex, not for &CompositeIndex Just a minor code cleanup. We still need Index for &CompositeIndex because the type is unsized, and unsized type cannot be converted to another dyn reference. --- lib/src/default_index/composite.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/src/default_index/composite.rs b/lib/src/default_index/composite.rs index a75caf442a..a544829174 100644 --- a/lib/src/default_index/composite.rs +++ b/lib/src/default_index/composite.rs @@ -399,12 +399,13 @@ impl CompositeIndex { } } -impl AsCompositeIndex for &CompositeIndex { +impl AsCompositeIndex for CompositeIndex { fn as_composite(&self) -> &CompositeIndex { self } } +// In revset engine, we need to convert &CompositeIndex to &dyn Index. impl Index for &CompositeIndex { /// Suppose the given `commit_id` exists, returns the minimum prefix length /// to disambiguate it. The length to be returned is a number of hexadecimal From 0bed671d00fde36d7ced5fcf00f1b8de983b5905 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 10 Mar 2024 11:27:59 +0900 Subject: [PATCH 4/4] index: remove HRTB stuff by implementing RevWalkIndex for CompositeIndex --- lib/src/default_index/rev_walk.rs | 28 ++++++++++++-------------- lib/src/default_index/revset_engine.rs | 4 ++-- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/lib/src/default_index/rev_walk.rs b/lib/src/default_index/rev_walk.rs index 0758fa4dd9..9c291f919d 100644 --- a/lib/src/default_index/rev_walk.rs +++ b/lib/src/default_index/rev_walk.rs @@ -40,10 +40,9 @@ pub(super) trait RevWalk { // to be reimplemented. /// Reattaches the underlying `index`. - fn attach(self, index: I) -> RevWalkBorrowedIndexIter + fn attach(self, index: &I) -> RevWalkBorrowedIndexIter<'_, I, Self> where Self: Sized, - I: Sized, { RevWalkBorrowedIndexIter { index, walk: self } } @@ -52,28 +51,27 @@ pub(super) trait RevWalk { /// Adapter that turns `RevWalk` into `Iterator` by attaching borrowed `index`. #[derive(Clone, Debug)] #[must_use] -pub(super) struct RevWalkBorrowedIndexIter { - // TODO: `index: I` will be a reference type. - index: I, +pub(super) struct RevWalkBorrowedIndexIter<'a, I: ?Sized, W> { + index: &'a I, walk: W, } -impl RevWalkBorrowedIndexIter { +impl RevWalkBorrowedIndexIter<'_, I, W> { /// Turns into `'static`-lifetime walk object by detaching the index. pub fn detach(self) -> W { self.walk } } -impl> Iterator for RevWalkBorrowedIndexIter { +impl> Iterator for RevWalkBorrowedIndexIter<'_, I, W> { type Item = W::Item; fn next(&mut self) -> Option { - self.walk.next(&self.index) + self.walk.next(self.index) } } -impl> FusedIterator for RevWalkBorrowedIndexIter {} +impl> FusedIterator for RevWalkBorrowedIndexIter<'_, I, W> {} /// Adapter that turns `RevWalk` into `Iterator` by attaching owned `index`. #[derive(Clone, Debug)] @@ -100,7 +98,7 @@ pub(super) trait RevWalkIndex { fn adjacent_positions(&self, pos: Self::Position) -> Self::AdjacentPositions; } -impl RevWalkIndex for &CompositeIndex { +impl RevWalkIndex for CompositeIndex { type Position = IndexPosition; type AdjacentPositions = SmallIndexPositionsVec; @@ -383,7 +381,7 @@ impl<'a> RevWalkBuilder<'a> { } pub(super) type RevWalkAncestors<'a> = - RevWalkBorrowedIndexIter<&'a CompositeIndex, RevWalkImpl>; + RevWalkBorrowedIndexIter<'a, CompositeIndex, RevWalkImpl>; #[derive(Clone)] #[must_use] @@ -420,7 +418,7 @@ impl RevWalk for RevWalkImpl { } pub(super) type RevWalkAncestorsGenerationRange<'a> = - RevWalkBorrowedIndexIter<&'a CompositeIndex, RevWalkGenerationRangeImpl>; + RevWalkBorrowedIndexIter<'a, CompositeIndex, RevWalkGenerationRangeImpl>; pub(super) type RevWalkDescendantsGenerationRange = RevWalkOwnedIndexIter< RevWalkDescendantsIndex, RevWalkGenerationRangeImpl>, @@ -540,7 +538,7 @@ impl RevWalkItemGenerationRange { /// Walks descendants from the roots, in order of ascending index position. pub(super) type RevWalkDescendants<'a> = - RevWalkBorrowedIndexIter<&'a CompositeIndex, RevWalkDescendantsImpl>; + RevWalkBorrowedIndexIter<'a, CompositeIndex, RevWalkDescendantsImpl>; #[derive(Clone)] #[must_use] @@ -561,10 +559,10 @@ impl RevWalkDescendants<'_> { } } -impl RevWalk<&CompositeIndex> for RevWalkDescendantsImpl { +impl RevWalk for RevWalkDescendantsImpl { type Item = IndexPosition; - fn next(&mut self, index: &&CompositeIndex) -> Option { + fn next(&mut self, index: &CompositeIndex) -> Option { while let Some(candidate_pos) = self.candidate_positions.pop() { if self.root_positions.contains(&candidate_pos) || index diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index b5846afb4c..f7addf9404 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -300,7 +300,7 @@ impl fmt::Debug for RevWalkRevset { impl InternalRevset for RevWalkRevset where - W: for<'index> RevWalk<&'index CompositeIndex, Item = IndexPosition> + Clone, + W: RevWalk + Clone, { fn entries<'a, 'index: 'a>( &'a self, @@ -327,7 +327,7 @@ where impl ToPredicateFn for RevWalkRevset where - W: for<'index> RevWalk<&'index CompositeIndex, Item = IndexPosition> + Clone, + W: RevWalk + Clone, { fn to_predicate_fn<'a, 'index: 'a>( &'a self,