From 7dc35b82b03905de28e7a8d0f009a89818bfa808 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 6 Apr 2023 23:35:04 +0900
Subject: [PATCH] revset: evaluate ancestors without using RevsetExpression
 builder API

I'm thinking of transforming RevsetExpression to a enum dedicated for
the evaluation stage. To help the migration, I want to remove the use of
the RevsetExpression builder API from the evaluation engine.

Fewer virtual dispatch is also better.
---
 lib/src/default_revset_engine.rs | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs
index 6cdb6a547..9e7f5eb2a 100644
--- a/lib/src/default_revset_engine.rs
+++ b/lib/src/default_revset_engine.rs
@@ -21,7 +21,9 @@ use std::sync::Arc;
 use itertools::Itertools;
 
 use crate::backend::{ChangeId, CommitId, MillisSinceEpoch, ObjectId};
-use crate::default_index_store::{CompositeIndex, IndexEntry, IndexEntryByPosition, IndexPosition};
+use crate::default_index_store::{
+    CompositeIndex, IndexEntry, IndexEntryByPosition, IndexPosition, RevWalk,
+};
 use crate::default_revset_graph_iterator::RevsetGraphIterator;
 use crate::index::{HexPrefix, Index, PrefixResolution};
 use crate::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit};
@@ -643,12 +645,14 @@ impl<'index, 'heads> EvaluationContext<'index, 'heads> {
                 }))
             }
             RevsetExpression::Ancestors { heads, generation } => {
-                let range_expression = RevsetExpression::Range {
-                    roots: RevsetExpression::none(),
-                    heads: heads.clone(),
-                    generation: generation.clone(),
-                };
-                self.evaluate(&range_expression)
+                let head_set = self.evaluate(heads)?;
+                let walk = self.walk_ancestors(&*head_set);
+                if generation == &GENERATION_RANGE_FULL {
+                    Ok(Box::new(RevWalkRevset { walk }))
+                } else {
+                    let walk = walk.filter_by_generation(generation.clone());
+                    Ok(Box::new(RevWalkRevset { walk }))
+                }
             }
             RevsetExpression::Range {
                 roots,
@@ -669,11 +673,11 @@ impl<'index, 'heads> EvaluationContext<'index, 'heads> {
             }
             RevsetExpression::DagRange { roots, heads } => {
                 let root_set = self.evaluate(roots)?;
-                let candidate_set = self.evaluate(&heads.ancestors())?;
+                let head_set = self.evaluate(heads)?;
                 let mut reachable: HashSet<_> =
                     root_set.iter().map(|entry| entry.position()).collect();
                 let mut result = vec![];
-                let candidates = candidate_set.iter().collect_vec();
+                let candidates = self.walk_ancestors(&*head_set).collect_vec();
                 for candidate in candidates.into_iter().rev() {
                     if reachable.contains(&candidate.position())
                         || candidate
@@ -816,6 +820,14 @@ impl<'index, 'heads> EvaluationContext<'index, 'heads> {
         }
     }
 
+    fn walk_ancestors<'a, S>(&self, head_set: &S) -> RevWalk<'index>
+    where
+        S: InternalRevset<'a> + ?Sized,
+    {
+        let head_ids = head_set.iter().map(|entry| entry.commit_id()).collect_vec();
+        self.composite_index.walk_revs(&head_ids, &[])
+    }
+
     fn revset_for_commit_ids(
         &self,
         commit_ids: &[CommitId],