From 28cbd7b1c5d1d200f13a4080d30486fa03b7c2cd Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Sun, 12 Mar 2023 23:00:02 -0700
Subject: [PATCH] revset: move evaluation into index

This commit adds an `evaluate_revset()` function to the `Index`
trait. It will require some further cleanup, but it already achieves
the goal of letting the index implementation decide which revset
engine to use.
---
 lib/src/default_index_store.rs | 22 +++++++++++++++++++++-
 lib/src/index.rs               | 13 +++++++++++++
 lib/src/revset.rs              |  3 +--
 3 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/lib/src/default_index_store.rs b/lib/src/default_index_store.rs
index 29297256d..6013890c7 100644
--- a/lib/src/default_index_store.rs
+++ b/lib/src/default_index_store.rs
@@ -44,8 +44,10 @@ use crate::index::{
 use crate::nightly_shims::BTreeSetExt;
 use crate::op_store::OperationId;
 use crate::operation::Operation;
+use crate::repo::Repo;
+use crate::revset::{Revset, RevsetError, RevsetExpression, RevsetWorkspaceContext};
 use crate::store::Store;
-use crate::{backend, dag_walk};
+use crate::{backend, dag_walk, default_revset_engine};
 
 #[derive(Debug)]
 pub struct DefaultIndexStore {
@@ -733,6 +735,15 @@ impl Index for MutableIndexImpl {
     fn topo_order(&self, input: &mut dyn Iterator<Item = &CommitId>) -> Vec<IndexEntry> {
         CompositeIndex(self).topo_order(input)
     }
+
+    fn evaluate_revset<'index>(
+        &'index self,
+        repo: &'index dyn Repo,
+        expression: &RevsetExpression,
+        workspace_ctx: Option<&RevsetWorkspaceContext>,
+    ) -> Result<Box<dyn Revset<'index> + 'index>, RevsetError> {
+        default_revset_engine::evaluate(repo, expression, workspace_ctx)
+    }
 }
 
 impl MutableIndex for MutableIndexImpl {
@@ -1806,6 +1817,15 @@ impl Index for ReadonlyIndexImpl {
     fn topo_order(&self, input: &mut dyn Iterator<Item = &CommitId>) -> Vec<IndexEntry> {
         CompositeIndex(self).topo_order(input)
     }
+
+    fn evaluate_revset<'index>(
+        &'index self,
+        repo: &'index dyn Repo,
+        expression: &RevsetExpression,
+        workspace_ctx: Option<&RevsetWorkspaceContext>,
+    ) -> Result<Box<dyn Revset<'index> + 'index>, RevsetError> {
+        default_revset_engine::evaluate(repo, expression, workspace_ctx)
+    }
 }
 
 #[cfg(test)]
diff --git a/lib/src/index.rs b/lib/src/index.rs
index ce6fb0c58..adf172cba 100644
--- a/lib/src/index.rs
+++ b/lib/src/index.rs
@@ -23,6 +23,8 @@ use crate::commit::Commit;
 use crate::default_index_store::{IndexEntry, IndexPosition, RevWalk};
 use crate::op_store::OperationId;
 use crate::operation::Operation;
+use crate::repo::Repo;
+use crate::revset::{Revset, RevsetError, RevsetExpression, RevsetWorkspaceContext};
 use crate::store::Store;
 
 #[derive(Debug, Error)]
@@ -70,6 +72,17 @@ pub trait Index {
 
     /// Parents before children
     fn topo_order(&self, input: &mut dyn Iterator<Item = &CommitId>) -> Vec<IndexEntry>;
+
+    // TODO: It's weird that we pass in the repo here since the repo is a
+    // higher-level concept. We should probably pass in the view and store
+    // instead, or maybe we should resolve symbols in the expression before we
+    // get here.
+    fn evaluate_revset<'index>(
+        &'index self,
+        repo: &'index dyn Repo,
+        expression: &RevsetExpression,
+        workspace_ctx: Option<&RevsetWorkspaceContext>,
+    ) -> Result<Box<dyn Revset<'index> + 'index>, RevsetError>;
 }
 
 pub trait ReadonlyIndex: Send + Sync {
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 4ef45165e..13d34d3e7 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -30,7 +30,6 @@ use thiserror::Error;
 use crate::backend::{BackendError, BackendResult, CommitId};
 use crate::commit::Commit;
 use crate::default_index_store::IndexEntry;
-use crate::default_revset_engine;
 use crate::op_store::WorkspaceId;
 use crate::repo::Repo;
 use crate::repo_path::{FsPathParseError, RepoPath};
@@ -394,7 +393,7 @@ impl RevsetExpression {
         repo: &'index dyn Repo,
         workspace_ctx: Option<&RevsetWorkspaceContext>,
     ) -> Result<Box<dyn Revset<'index> + 'index>, RevsetError> {
-        default_revset_engine::evaluate(repo, self, workspace_ctx)
+        repo.index().evaluate_revset(repo, self, workspace_ctx)
     }
 }