From 7e1e9efa380c84e5859ef4f34e5ae49cdd7b7dac Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Thu, 6 Apr 2023 13:03:20 +0900
Subject: [PATCH] revset: resolve "all()" prior to evaluation

---
 lib/src/default_revset_engine.rs | 11 -----------
 lib/src/revset.rs                | 13 +++++++++++--
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs
index 24d2007ab..0f3de2b2d 100644
--- a/lib/src/default_revset_engine.rs
+++ b/lib/src/default_revset_engine.rs
@@ -574,17 +574,6 @@ impl<'index, 'heads> EvaluationContext<'index, 'heads> {
         expression: &ResolvedExpression,
     ) -> Result<Box<dyn InternalRevset<'index> + 'index>, RevsetEvaluationError> {
         match expression {
-            ResolvedExpression::All => {
-                // Since `all()` does not include hidden commits, some of the logical
-                // transformation rules may subtly change the evaluated set. For example,
-                // `all() & x` is not `x` if `x` is hidden. This wouldn't matter in practice,
-                // but if it does, the heads set could be extended to include the commits
-                // (and `remote_branches()`) specified in the revset expression. Alternatively,
-                // some optimization rules could be removed, but that means `author(_) & x`
-                // would have to test `:visble_heads() & x`.
-                let walk = self.composite_index.walk_revs(self.visible_heads, &[]);
-                Ok(Box::new(RevWalkRevset { walk }))
-            }
             ResolvedExpression::Commits(commit_ids) => {
                 Ok(Box::new(self.revset_for_commit_ids(commit_ids)))
             }
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index f051fb067..15152efd5 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -460,7 +460,6 @@ pub enum ResolvedPredicateExpression {
 /// Use `RevsetExpression` API to build a query programmatically.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum ResolvedExpression {
-    All, // TODO: should be substituted at resolve_visibility()
     Commits(Vec<CommitId>),
     Children {
         roots: Box<ResolvedExpression>,
@@ -1901,7 +1900,17 @@ impl VisibilityResolutionContext {
     }
 
     fn resolve_all(&self) -> ResolvedExpression {
-        ResolvedExpression::All
+        // Since `all()` does not include hidden commits, some of the logical
+        // transformation rules may subtly change the evaluated set. For example,
+        // `all() & x` is not `x` if `x` is hidden. This wouldn't matter in practice,
+        // but if it does, the heads set could be extended to include the commits
+        // (and `remote_branches()`) specified in the revset expression. Alternatively,
+        // some optimization rules could be removed, but that means `author(_) & x`
+        // would have to test `:visble_heads() & x`.
+        ResolvedExpression::Ancestors {
+            heads: self.resolve_visible_heads().into(),
+            generation: GENERATION_RANGE_FULL,
+        }
     }
 
     fn resolve_visible_heads(&self) -> ResolvedExpression {