diff --git a/cli/examples/custom-commit-templater/main.rs b/cli/examples/custom-commit-templater/main.rs index 4b1c67c77..404e5ad71 100644 --- a/cli/examples/custom-commit-templater/main.rs +++ b/cli/examples/custom-commit-templater/main.rs @@ -39,6 +39,7 @@ use jj_lib::revset::RevsetParseContext; use jj_lib::revset::RevsetParseError; use jj_lib::revset::RevsetResolutionError; use jj_lib::revset::SymbolResolverExtension; +use jj_lib::revset::UserRevsetExpression; use once_cell::sync::OnceCell; struct HexCounter; @@ -191,7 +192,7 @@ fn even_digits( _diagnostics: &mut RevsetDiagnostics, function: &FunctionCallNode, _context: &RevsetParseContext, -) -> Result, RevsetParseError> { +) -> Result, RevsetParseError> { function.expect_no_arguments()?; Ok(RevsetExpression::filter(RevsetFilterPredicate::Extension( Rc::new(EvenDigitsFilter), diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 8342fa8fe..42e125c47 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -91,6 +91,7 @@ use jj_lib::repo_path::RepoPathBuf; use jj_lib::repo_path::RepoPathUiConverter; use jj_lib::repo_path::UiPathParseError; use jj_lib::revset; +use jj_lib::revset::ResolvedRevsetExpression; use jj_lib::revset::RevsetAliasesMap; use jj_lib::revset::RevsetDiagnostics; use jj_lib::revset::RevsetExpression; @@ -102,6 +103,7 @@ use jj_lib::revset::RevsetModifier; use jj_lib::revset::RevsetParseContext; use jj_lib::revset::RevsetWorkspaceContext; use jj_lib::revset::SymbolResolverExtension; +use jj_lib::revset::UserRevsetExpression; use jj_lib::rewrite::restore_tree; use jj_lib::settings::ConfigResultExt as _; use jj_lib::settings::UserSettings; @@ -604,8 +606,8 @@ pub struct WorkspaceCommandEnvironment { template_aliases_map: TemplateAliasesMap, path_converter: RepoPathUiConverter, workspace_id: WorkspaceId, - immutable_heads_expression: Rc, - short_prefixes_expression: Option>, + immutable_heads_expression: Rc, + short_prefixes_expression: Option>, } impl WorkspaceCommandEnvironment { @@ -676,21 +678,21 @@ impl WorkspaceCommandEnvironment { } /// User-configured expression defining the immutable set. - pub fn immutable_expression(&self) -> Rc { + pub fn immutable_expression(&self) -> Rc { // Negated ancestors expression `~::( | root())` is slightly // easier to optimize than negated union `~(:: | root())`. self.immutable_heads_expression.ancestors() } /// User-configured expression defining the heads of the immutable set. - pub fn immutable_heads_expression(&self) -> &Rc { + pub fn immutable_heads_expression(&self) -> &Rc { &self.immutable_heads_expression } fn load_immutable_heads_expression( &self, ui: &Ui, - ) -> Result, CommandError> { + ) -> Result, CommandError> { let mut diagnostics = RevsetDiagnostics::new(); let expression = revset_util::parse_immutable_heads_expression( &mut diagnostics, @@ -704,7 +706,7 @@ impl WorkspaceCommandEnvironment { fn load_short_prefixes_expression( &self, ui: &Ui, - ) -> Result>, CommandError> { + ) -> Result>, CommandError> { let revset_string = self .settings() .config() @@ -1373,7 +1375,7 @@ impl WorkspaceCommandHelper { pub fn attach_revset_evaluator( &self, - expression: Rc, + expression: Rc, ) -> RevsetExpressionEvaluator<'_> { RevsetExpressionEvaluator::new( self.repo().as_ref(), @@ -1829,14 +1831,15 @@ See https://martinvonz.github.io/jj/latest/working-copy/#stale-working-copy \ let removed_conflicts_expr = new_heads.range(&old_heads).intersection(&conflicts); let added_conflicts_expr = old_heads.range(&new_heads).intersection(&conflicts); - let get_commits = |expr: Rc| -> Result, CommandError> { - let commits = expr - .evaluate_programmatic(new_repo)? - .iter() - .commits(new_repo.store()) - .try_collect()?; - Ok(commits) - }; + let get_commits = + |expr: Rc| -> Result, CommandError> { + let commits = expr + .evaluate_programmatic(new_repo)? + .iter() + .commits(new_repo.store()) + .try_collect()?; + Ok(commits) + }; let removed_conflict_commits = get_commits(removed_conflicts_expr)?; let added_conflict_commits = get_commits(added_conflicts_expr)?; diff --git a/cli/src/commands/bench/revset.rs b/cli/src/commands/bench/revset.rs index 7dc91d52c..251198d3d 100644 --- a/cli/src/commands/bench/revset.rs +++ b/cli/src/commands/bench/revset.rs @@ -21,8 +21,8 @@ use criterion::BenchmarkGroup; use criterion::BenchmarkId; use jj_lib::revset; use jj_lib::revset::DefaultSymbolResolver; -use jj_lib::revset::RevsetExpression; use jj_lib::revset::SymbolResolverExtension; +use jj_lib::revset::UserRevsetExpression; use super::new_criterion; use super::CriterionArgs; @@ -87,7 +87,8 @@ fn bench_revset( .clone(), ); // Time both evaluation and iteration. - let routine = |workspace_command: &WorkspaceCommandHelper, expression: Rc| { + let routine = |workspace_command: &WorkspaceCommandHelper, + expression: Rc| { // Evaluate the expression without parsing/evaluating short-prefixes. let repo = workspace_command.repo().as_ref(); let symbol_resolver = diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index d08531b0e..c30a68afd 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -21,6 +21,7 @@ use jj_lib::backend::CommitId; use jj_lib::commit::CommitIteratorExt; use jj_lib::repo::ReadonlyRepo; use jj_lib::repo::Repo; +use jj_lib::revset::ResolvedRevsetExpression; use jj_lib::revset::RevsetExpression; use jj_lib::revset::RevsetIteratorExt; use jj_lib::rewrite::merge_commit_trees; @@ -234,8 +235,8 @@ pub(crate) fn cmd_new( /// parents of the new commit. fn ensure_no_commit_loop( repo: &ReadonlyRepo, - children_expression: &Rc, - parents_expression: &Rc, + children_expression: &Rc, + parents_expression: &Rc, ) -> Result<(), CommandError> { if let Some(commit_id) = children_expression .dag_range_to(parents_expression) diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index 5658ef21b..de6ba6514 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -25,6 +25,7 @@ use jj_lib::commit::CommitIteratorExt; use jj_lib::object_id::ObjectId; use jj_lib::repo::ReadonlyRepo; use jj_lib::repo::Repo; +use jj_lib::revset::ResolvedRevsetExpression; use jj_lib::revset::RevsetExpression; use jj_lib::revset::RevsetIteratorExt; use jj_lib::rewrite::move_commits; @@ -598,8 +599,8 @@ fn rebase_revisions_transaction( /// parents of rebased commits. fn ensure_no_commit_loop( repo: &ReadonlyRepo, - children_expression: &Rc, - parents_expression: &Rc, + children_expression: &Rc, + parents_expression: &Rc, ) -> Result<(), CommandError> { if let Some(commit_id) = children_expression .dag_range_to(parents_expression) diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index 60b36d31d..796b7f9e6 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -44,9 +44,9 @@ use jj_lib::revset; use jj_lib::revset::Revset; use jj_lib::revset::RevsetContainingFn; use jj_lib::revset::RevsetDiagnostics; -use jj_lib::revset::RevsetExpression; use jj_lib::revset::RevsetModifier; use jj_lib::revset::RevsetParseContext; +use jj_lib::revset::UserRevsetExpression; use jj_lib::store::Store; use once_cell::unsync::OnceCell; @@ -94,7 +94,7 @@ pub struct CommitTemplateLanguage<'repo> { // are contained in RevsetParseContext for example. revset_parse_context: RevsetParseContext<'repo>, id_prefix_context: &'repo IdPrefixContext, - immutable_expression: Rc, + immutable_expression: Rc, build_fn_table: CommitTemplateBuildFnTable<'repo>, keyword_cache: CommitKeywordCache<'repo>, cache_extensions: ExtensionsMap, @@ -109,7 +109,7 @@ impl<'repo> CommitTemplateLanguage<'repo> { workspace_id: &WorkspaceId, revset_parse_context: RevsetParseContext<'repo>, id_prefix_context: &'repo IdPrefixContext, - immutable_expression: Rc, + immutable_expression: Rc, extensions: &[impl AsRef], ) -> Self { let mut build_fn_table = CommitTemplateBuildFnTable::builtin(); @@ -845,7 +845,7 @@ fn expect_fileset_literal( fn evaluate_revset_expression<'repo>( language: &CommitTemplateLanguage<'repo>, span: pest::Span<'_>, - expression: Rc, + expression: Rc, ) -> Result, TemplateParseError> { let symbol_resolver = revset_util::default_symbol_resolver( language.repo, diff --git a/cli/src/movement_util.rs b/cli/src/movement_util.rs index 9d5913a80..6909c2b60 100644 --- a/cli/src/movement_util.rs +++ b/cli/src/movement_util.rs @@ -19,6 +19,7 @@ use itertools::Itertools; use jj_lib::backend::CommitId; use jj_lib::commit::Commit; use jj_lib::repo::Repo; +use jj_lib::revset::ResolvedRevsetExpression; use jj_lib::revset::RevsetExpression; use jj_lib::revset::RevsetFilterPredicate; use jj_lib::revset::RevsetIteratorExt; @@ -117,10 +118,10 @@ impl Direction { fn build_target_revset( &self, - working_revset: &Rc, - start_revset: &Rc, + working_revset: &Rc, + start_revset: &Rc, args: &MovementArgsInternal, - ) -> Result, CommandError> { + ) -> Result, CommandError> { let nth = match (self, args.should_edit) { (Direction::Next, true) => start_revset.descendants_at(args.offset), (Direction::Next, false) => start_revset diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs index 21b8bd69d..5e6a1a106 100644 --- a/cli/src/revset_util.rs +++ b/cli/src/revset_util.rs @@ -36,6 +36,7 @@ use jj_lib::revset::RevsetParseContext; use jj_lib::revset::RevsetParseError; use jj_lib::revset::RevsetResolutionError; use jj_lib::revset::SymbolResolverExtension; +use jj_lib::revset::UserRevsetExpression; use jj_lib::settings::ConfigResultExt as _; use thiserror::Error; @@ -57,12 +58,12 @@ pub enum UserRevsetEvaluationError { Evaluation(RevsetEvaluationError), } -/// Wrapper around `RevsetExpression` to provide convenient methods. +/// Wrapper around `UserRevsetExpression` to provide convenient methods. pub struct RevsetExpressionEvaluator<'repo> { repo: &'repo dyn Repo, extensions: Arc, id_prefix_context: &'repo IdPrefixContext, - expression: Rc, + expression: Rc, } impl<'repo> RevsetExpressionEvaluator<'repo> { @@ -70,7 +71,7 @@ impl<'repo> RevsetExpressionEvaluator<'repo> { repo: &'repo dyn Repo, extensions: Arc, id_prefix_context: &'repo IdPrefixContext, - expression: Rc, + expression: Rc, ) -> Self { RevsetExpressionEvaluator { repo, @@ -81,12 +82,12 @@ impl<'repo> RevsetExpressionEvaluator<'repo> { } /// Returns the underlying expression. - pub fn expression(&self) -> &Rc { + pub fn expression(&self) -> &Rc { &self.expression } /// Intersects the underlying expression with the `other` expression. - pub fn intersect_with(&mut self, other: &Rc) { + pub fn intersect_with(&mut self, other: &Rc) { self.expression = self.expression.intersection(other); } @@ -183,7 +184,7 @@ pub fn load_revset_aliases( pub fn evaluate<'a>( repo: &'a dyn Repo, symbol_resolver: &DefaultSymbolResolver, - expression: Rc, + expression: Rc, ) -> Result, UserRevsetEvaluationError> { let resolved = revset::optimize(expression) .resolve_user_expression(repo, symbol_resolver) @@ -208,7 +209,7 @@ pub fn default_symbol_resolver<'a>( pub fn parse_immutable_heads_expression( diagnostics: &mut RevsetDiagnostics, context: &RevsetParseContext, -) -> Result, RevsetParseError> { +) -> Result, RevsetParseError> { let (_, _, immutable_heads_str) = context .aliases_map() .get_function(USER_IMMUTABLE_HEADS, 0) @@ -278,7 +279,7 @@ pub(super) fn evaluate_revset_to_single_commit<'a>( fn format_multiple_revisions_error( revision_str: &str, - expression: &RevsetExpression, + expression: &UserRevsetExpression, commits: &[Commit], elided: bool, template: &TemplateRenderer<'_, Commit>, diff --git a/lib/src/id_prefix.rs b/lib/src/id_prefix.rs index db42e4501..b41b8dd92 100644 --- a/lib/src/id_prefix.rs +++ b/lib/src/id_prefix.rs @@ -32,10 +32,10 @@ use crate::object_id::PrefixResolution; use crate::repo::Repo; use crate::revset::DefaultSymbolResolver; use crate::revset::RevsetEvaluationError; -use crate::revset::RevsetExpression; use crate::revset::RevsetExtensions; use crate::revset::RevsetResolutionError; use crate::revset::SymbolResolverExtension; +use crate::revset::UserRevsetExpression; #[derive(Debug, Error)] pub enum IdPrefixIndexLoadError { @@ -46,7 +46,7 @@ pub enum IdPrefixIndexLoadError { } struct DisambiguationData { - expression: Rc, + expression: Rc, indexes: OnceCell, } @@ -123,7 +123,7 @@ impl IdPrefixContext { } } - pub fn disambiguate_within(mut self, expression: Rc) -> Self { + pub fn disambiguate_within(mut self, expression: Rc) -> Self { self.disambiguation = Some(DisambiguationData { expression, indexes: OnceCell::new(), diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 24bc15083..9350e899f 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -177,14 +177,51 @@ pub enum RevsetFilterPredicate { Extension(Rc), } +mod private { + /// Defines [`RevsetExpression`] variants depending on resolution state. + pub trait ExpressionState { + type CommitRef: Clone; + type Operation: Clone; + } + + // Not constructible because these state types just define associated types. + #[derive(Debug)] + pub enum UserExpressionState {} + #[derive(Debug)] + pub enum ResolvedExpressionState {} +} + +use private::ExpressionState; +use private::ResolvedExpressionState; +use private::UserExpressionState; + +impl ExpressionState for UserExpressionState { + type CommitRef = RevsetCommitRef; + type Operation = String; +} + +impl ExpressionState for ResolvedExpressionState { + type CommitRef = Infallible; + type Operation = Infallible; +} + +/// [`RevsetExpression`] that may contain unresolved commit refs. +pub type UserRevsetExpression = RevsetExpression; +/// [`RevsetExpression`] that never contains unresolved commit refs. +pub type ResolvedRevsetExpression = RevsetExpression; + +/// Tree of revset expressions describing DAG operations. +/// +/// Use [`UserRevsetExpression`] or [`ResolvedRevsetExpression`] to construct +/// expression of that state. #[derive(Clone, Debug)] -pub enum RevsetExpression { +pub enum RevsetExpression { None, All, VisibleHeads, Root, Commits(Vec), - CommitRef(RevsetCommitRef), + CommitRef(St::CommitRef), Ancestors { heads: Rc, generation: Range, @@ -221,7 +258,7 @@ pub enum RevsetExpression { AsFilter(Rc), /// Resolves symbols and visibility at the specified operation. AtOperation { - operation: String, + operation: St::Operation, candidates: Rc, }, /// Resolves visibility within the specified repo state. @@ -238,8 +275,9 @@ pub enum RevsetExpression { Difference(Rc, Rc), } -// Leaf expression that never contains unresolved commit refs -impl RevsetExpression { +// Leaf expression that never contains unresolved commit refs, which can be +// either user or resolved expression +impl RevsetExpression { pub fn none() -> Rc { Rc::new(Self::None) } @@ -275,7 +313,7 @@ impl RevsetExpression { } // Leaf expression that represents unresolved commit refs -impl RevsetExpression { +impl> RevsetExpression { pub fn working_copy(workspace_id: WorkspaceId) -> Rc { Rc::new(Self::CommitRef(RevsetCommitRef::WorkingCopy(workspace_id))) } @@ -323,7 +361,7 @@ impl RevsetExpression { } // Compound expression -impl RevsetExpression { +impl RevsetExpression { pub fn latest(self: &Rc, count: usize) -> Rc { Rc::new(Self::Latest { candidates: self.clone(), @@ -480,7 +518,7 @@ impl RevsetExpression { } } -impl RevsetExpression { +impl> RevsetExpression { /// Returns symbol string if this expression is of that type. pub fn as_symbol(&self) -> Option<&str> { match self { @@ -488,27 +526,12 @@ impl RevsetExpression { _ => None, } } +} - /// Resolve a programmatically created revset expression. - /// - /// In particular, the expression must not contain any symbols (bookmarks, - /// tags, change/commit prefixes). Callers must not include - /// `RevsetExpression::symbol()` in the expression, and should instead - /// resolve symbols to `CommitId`s and pass them into - /// `RevsetExpression::commits()`. Similarly, the expression must not - /// contain any `RevsetExpression::remote_symbol()` or - /// `RevsetExpression::working_copy()`, unless they're known to be valid. - /// The expression must not contain `RevsetExpression::AtOperation` even if - /// it's known to be valid. It can fail at loading operation data. - pub fn resolve_programmatic(&self, repo: &dyn Repo) -> ResolvedExpression { - let symbol_resolver = FailingSymbolResolver; - resolve_symbols(repo, self, &symbol_resolver) - .map(|expression| resolve_visibility(repo, &expression)) - .unwrap() - } - +impl UserRevsetExpression { /// Resolve a user-provided expression. Symbols will be resolved using the /// provided `SymbolResolver`. + // TODO: return ResolvedRevsetExpression which can still be combined pub fn resolve_user_expression( &self, repo: &dyn Repo, @@ -517,14 +540,18 @@ impl RevsetExpression { resolve_symbols(repo, self, symbol_resolver) .map(|expression| resolve_visibility(repo, &expression)) } +} +impl ResolvedRevsetExpression { /// Resolve a programmatically created revset expression and evaluate it in /// the repo. + // TODO: rename to .evaluate(), and add .evaluate_unoptimized() for + // testing/debugging purpose? pub fn evaluate_programmatic<'index>( self: Rc, repo: &'index dyn Repo, ) -> Result, RevsetEvaluationError> { - optimize(self).resolve_programmatic(repo).evaluate(repo) + resolve_visibility(repo, &optimize(self)).evaluate(repo) } } @@ -548,6 +575,7 @@ pub enum ResolvedPredicateExpression { /// properties. /// /// Use `RevsetExpression` API to build a query programmatically. +// TODO: rename to BackendExpression? #[derive(Clone, Debug)] pub enum ResolvedExpression { Commits(Vec), @@ -603,7 +631,7 @@ pub type RevsetFunction = fn( &mut RevsetDiagnostics, &FunctionCallNode, &RevsetParseContext, -) -> Result, RevsetParseError>; +) -> Result, RevsetParseError>; static BUILTIN_FUNCTION_MAP: Lazy> = Lazy::new(|| { // Not using maplit::hashmap!{} or custom declarative macro here because @@ -973,7 +1001,7 @@ fn parse_remote_bookmarks_arguments( diagnostics: &mut RevsetDiagnostics, function: &FunctionCallNode, remote_ref_state: Option, -) -> Result, RevsetParseError> { +) -> Result, RevsetParseError> { let ([], [bookmark_opt_arg, remote_opt_arg]) = function.expect_named_arguments(&["", "remote"])?; let bookmark_pattern = if let Some(bookmark_arg) = bookmark_opt_arg { @@ -998,7 +1026,7 @@ fn lower_function_call( diagnostics: &mut RevsetDiagnostics, function: &FunctionCallNode, context: &RevsetParseContext, -) -> Result, RevsetParseError> { +) -> Result, RevsetParseError> { let function_map = &context.extensions.function_map; if let Some(func) = function_map.get(function.name) { func(diagnostics, function, context) @@ -1019,7 +1047,7 @@ pub fn lower_expression( diagnostics: &mut RevsetDiagnostics, node: &ExpressionNode, context: &RevsetParseContext, -) -> Result, RevsetParseError> { +) -> Result, RevsetParseError> { match &node.kind { ExpressionKind::Identifier(name) => Ok(RevsetExpression::symbol((*name).to_owned())), ExpressionKind::String(name) => Ok(RevsetExpression::symbol(name.to_owned())), @@ -1106,7 +1134,7 @@ pub fn parse( diagnostics: &mut RevsetDiagnostics, revset_str: &str, context: &RevsetParseContext, -) -> Result, RevsetParseError> { +) -> Result, RevsetParseError> { let node = revset_parser::parse_program(revset_str)?; let node = dsl_util::expand_aliases(node, context.aliases_map)?; lower_expression(diagnostics, &node, context) @@ -1117,7 +1145,7 @@ pub fn parse_with_modifier( diagnostics: &mut RevsetDiagnostics, revset_str: &str, context: &RevsetParseContext, -) -> Result<(Rc, Option), RevsetParseError> { +) -> Result<(Rc, Option), RevsetParseError> { let node = revset_parser::parse_program(revset_str)?; let node = dsl_util::expand_aliases(node, context.aliases_map)?; revset_parser::expect_program_with( @@ -1136,15 +1164,19 @@ pub fn parse_with_modifier( } /// `Some` for rewritten expression, or `None` to reuse the original expression. -type TransformedExpression = Option>; +type TransformedExpression = Option>>; /// Walks `expression` tree and applies `f` recursively from leaf nodes. -fn transform_expression_bottom_up( - expression: &Rc, - mut f: impl FnMut(&Rc) -> TransformedExpression, -) -> TransformedExpression { - try_transform_expression::(expression, |_| Ok(None), |expression| Ok(f(expression))) - .unwrap() +fn transform_expression_bottom_up( + expression: &Rc>, + mut f: impl FnMut(&Rc>) -> TransformedExpression, +) -> TransformedExpression { + try_transform_expression::( + expression, + |_| Ok(None), + |expression| Ok(f(expression)), + ) + .unwrap() } /// Walks `expression` tree and applies transformation recursively. @@ -1159,16 +1191,16 @@ fn transform_expression_bottom_up( /// If no nodes rewritten, this function returns `None`. /// `std::iter::successors()` could be used if the transformation needs to be /// applied repeatedly until converged. -fn try_transform_expression( - expression: &Rc, - mut pre: impl FnMut(&Rc) -> Result, - mut post: impl FnMut(&Rc) -> Result, -) -> Result { - fn transform_child_rec( - expression: &Rc, - pre: &mut impl FnMut(&Rc) -> Result, - post: &mut impl FnMut(&Rc) -> Result, - ) -> Result { +fn try_transform_expression( + expression: &Rc>, + mut pre: impl FnMut(&Rc>) -> Result, E>, + mut post: impl FnMut(&Rc>) -> Result, E>, +) -> Result, E> { + fn transform_child_rec( + expression: &Rc>, + pre: &mut impl FnMut(&Rc>) -> Result, E>, + post: &mut impl FnMut(&Rc>) -> Result, E>, + ) -> Result, E> { Ok(match expression.as_ref() { RevsetExpression::None => None, RevsetExpression::All => None, @@ -1274,11 +1306,11 @@ fn try_transform_expression( } #[allow(clippy::type_complexity)] - fn transform_rec_pair( - (expression1, expression2): (&Rc, &Rc), - pre: &mut impl FnMut(&Rc) -> Result, - post: &mut impl FnMut(&Rc) -> Result, - ) -> Result, Rc)>, E> { + fn transform_rec_pair( + (expression1, expression2): (&Rc>, &Rc>), + pre: &mut impl FnMut(&Rc>) -> Result, E>, + post: &mut impl FnMut(&Rc>) -> Result, E>, + ) -> Result>, Rc>)>, E> { match ( transform_rec(expression1, pre, post)?, transform_rec(expression2, pre, post)?, @@ -1292,11 +1324,11 @@ fn try_transform_expression( } } - fn transform_rec( - expression: &Rc, - pre: &mut impl FnMut(&Rc) -> Result, - post: &mut impl FnMut(&Rc) -> Result, - ) -> Result { + fn transform_rec( + expression: &Rc>, + pre: &mut impl FnMut(&Rc>) -> Result, E>, + post: &mut impl FnMut(&Rc>) -> Result, E>, + ) -> Result, E> { if let Some(new_expression) = pre(expression)? { return Ok(Some(new_expression)); } @@ -1314,41 +1346,42 @@ fn try_transform_expression( /// Visitor-like interface to transform [`RevsetExpression`] state recursively. /// /// This is similar to [`try_transform_expression()`], but is supposed to -/// transform the resolution state. -trait ExpressionStateFolder { +/// transform the resolution state from `InSt` to `OutSt`. +trait ExpressionStateFolder { type Error; /// Transforms the `expression`. By default, inner items are transformed /// recursively. fn fold_expression( &mut self, - expression: &RevsetExpression, - ) -> Result, Self::Error> { + expression: &RevsetExpression, + ) -> Result>, Self::Error> { fold_child_expression_state(self, expression) } /// Transforms commit ref such as symbol. fn fold_commit_ref( &mut self, - commit_ref: &RevsetCommitRef, - ) -> Result, Self::Error>; + commit_ref: &InSt::CommitRef, + ) -> Result>, Self::Error>; /// Transforms `at_operation(operation, candidates)` expression. - #[allow(clippy::ptr_arg)] // TODO: &String will be parameterized fn fold_at_operation( &mut self, - operation: &String, - candidates: &RevsetExpression, - ) -> Result, Self::Error>; + operation: &InSt::Operation, + candidates: &RevsetExpression, + ) -> Result>, Self::Error>; } /// Transforms inner items of the `expression` by using the `folder`. -fn fold_child_expression_state( +fn fold_child_expression_state( folder: &mut F, - expression: &RevsetExpression, -) -> Result, F::Error> + expression: &RevsetExpression, +) -> Result>, F::Error> where - F: ExpressionStateFolder + ?Sized, + InSt: ExpressionState, + OutSt: ExpressionState, + F: ExpressionStateFolder + ?Sized, { let expression: Rc<_> = match expression { RevsetExpression::None => RevsetExpression::None.into(), @@ -1466,22 +1499,25 @@ where /// help further optimization (e.g. combine `file(_)` matchers.) /// c. Wraps union of filter and set (e.g. `author(_) | heads()`), to /// ensure inner filter wouldn't need to evaluate all the input sets. -fn internalize_filter(expression: &Rc) -> TransformedExpression { - fn is_filter(expression: &RevsetExpression) -> bool { +fn internalize_filter( + expression: &Rc>, +) -> TransformedExpression { + fn is_filter(expression: &RevsetExpression) -> bool { matches!( expression, RevsetExpression::Filter(_) | RevsetExpression::AsFilter(_) ) } - fn is_filter_tree(expression: &RevsetExpression) -> bool { + fn is_filter_tree(expression: &RevsetExpression) -> bool { is_filter(expression) || as_filter_intersection(expression).is_some() } // Extracts 'c & f' from intersect_down()-ed node. - fn as_filter_intersection( - expression: &RevsetExpression, - ) -> Option<(&Rc, &Rc)> { + #[allow(clippy::type_complexity)] + fn as_filter_intersection( + expression: &RevsetExpression, + ) -> Option<(&Rc>, &Rc>)> { if let RevsetExpression::Intersection(expression1, expression2) = expression { is_filter(expression2).then_some((expression1, expression2)) } else { @@ -1493,10 +1529,10 @@ fn internalize_filter(expression: &Rc) -> TransformedExpressio // apply the whole bottom-up pass to new intersection node. Instead, just push // new 'c & (d & g)' down-left to '(c & d) & g' while either side is // an intersection of filter node. - fn intersect_down( - expression1: &Rc, - expression2: &Rc, - ) -> TransformedExpression { + fn intersect_down( + expression1: &Rc>, + expression2: &Rc>, + ) -> TransformedExpression { let recurse = |e1, e2| intersect_down(e1, e2).unwrap_or_else(|| e1.intersection(e2)); match (expression1.as_ref(), expression2.as_ref()) { // Don't reorder 'f1 & f2' @@ -1541,7 +1577,9 @@ fn internalize_filter(expression: &Rc) -> TransformedExpressio /// /// This does not rewrite 'x & none()' to 'none()' because 'x' may be an invalid /// symbol. -fn fold_redundant_expression(expression: &Rc) -> TransformedExpression { +fn fold_redundant_expression( + expression: &Rc>, +) -> TransformedExpression { transform_expression_bottom_up(expression, |expression| match expression.as_ref() { RevsetExpression::NotIn(outer) => match outer.as_ref() { RevsetExpression::NotIn(inner) => Some(inner.clone()), @@ -1558,10 +1596,10 @@ fn fold_redundant_expression(expression: &Rc) -> TransformedEx }) } -fn to_difference_range( - expression: &Rc, - complement: &Rc, -) -> TransformedExpression { +fn to_difference_range( + expression: &Rc>, + complement: &Rc>, +) -> TransformedExpression { match (expression.as_ref(), complement.as_ref()) { // ::heads & ~(::roots) -> roots..heads ( @@ -1597,11 +1635,13 @@ fn to_difference_range( /// Transforms negative intersection to difference. Redundant intersections like /// `all() & e` should have been removed. -fn fold_difference(expression: &Rc) -> TransformedExpression { - fn to_difference( - expression: &Rc, - complement: &Rc, - ) -> Rc { +fn fold_difference( + expression: &Rc>, +) -> TransformedExpression { + fn to_difference( + expression: &Rc>, + complement: &Rc>, + ) -> Rc> { to_difference_range(expression, complement).unwrap_or_else(|| expression.minus(complement)) } @@ -1627,7 +1667,9 @@ fn fold_difference(expression: &Rc) -> TransformedExpression { /// /// Since this rule inserts redundant `visible_heads()`, negative intersections /// should have been transformed. -fn fold_not_in_ancestors(expression: &Rc) -> TransformedExpression { +fn fold_not_in_ancestors( + expression: &Rc>, +) -> TransformedExpression { transform_expression_bottom_up(expression, |expression| match expression.as_ref() { RevsetExpression::NotIn(complement) if matches!(complement.as_ref(), RevsetExpression::Ancestors { .. }) => @@ -1644,7 +1686,9 @@ fn fold_not_in_ancestors(expression: &Rc) -> TransformedExpres /// /// For example, `all() ~ e` will become `all() & ~e`, which can be simplified /// further by `fold_redundant_expression()`. -fn unfold_difference(expression: &Rc) -> TransformedExpression { +fn unfold_difference( + expression: &Rc>, +) -> TransformedExpression { transform_expression_bottom_up(expression, |expression| match expression.as_ref() { // roots..heads -> ::heads & ~(::roots) RevsetExpression::Range { @@ -1667,7 +1711,9 @@ fn unfold_difference(expression: &Rc) -> TransformedExpression /// Transforms nested `ancestors()`/`parents()`/`descendants()`/`children()` /// like `h---`/`r+++`. -fn fold_generation(expression: &Rc) -> TransformedExpression { +fn fold_generation( + expression: &Rc>, +) -> TransformedExpression { fn add_generation(generation1: &Range, generation2: &Range) -> Range { // For any (g1, g2) in (generation1, generation2), g1 + g2. if generation1.is_empty() || generation2.is_empty() { @@ -1723,7 +1769,9 @@ fn fold_generation(expression: &Rc) -> TransformedExpression { /// Rewrites the given `expression` tree to reduce evaluation cost. Returns new /// tree. -pub fn optimize(expression: Rc) -> Rc { +pub fn optimize( + expression: Rc>, +) -> Rc> { let expression = unfold_difference(&expression).unwrap_or(expression); let expression = fold_redundant_expression(&expression).unwrap_or(expression); let expression = fold_generation(&expression).unwrap_or(expression); @@ -2141,13 +2189,15 @@ impl<'a> ExpressionSymbolResolver<'a> { } } -impl ExpressionStateFolder for ExpressionSymbolResolver<'_> { +impl ExpressionStateFolder + for ExpressionSymbolResolver<'_> +{ type Error = RevsetResolutionError; fn fold_expression( &mut self, - expression: &RevsetExpression, - ) -> Result, Self::Error> { + expression: &UserRevsetExpression, + ) -> Result, Self::Error> { match expression { // 'present(x)' opens new symbol resolution scope to map error to 'none()' RevsetExpression::Present(candidates) => { @@ -2170,7 +2220,7 @@ impl ExpressionStateFolder for ExpressionSymbolResolver<'_> { fn fold_commit_ref( &mut self, commit_ref: &RevsetCommitRef, - ) -> Result, Self::Error> { + ) -> Result, Self::Error> { let commit_ids = resolve_commit_ref(self.repo(), commit_ref, self.symbol_resolver)?; Ok(RevsetExpression::commits(commit_ids)) } @@ -2178,8 +2228,8 @@ impl ExpressionStateFolder for ExpressionSymbolResolver<'_> { fn fold_at_operation( &mut self, operation: &String, - candidates: &RevsetExpression, - ) -> Result, Self::Error> { + candidates: &UserRevsetExpression, + ) -> Result, Self::Error> { let repo = reload_repo_at_operation(self.repo(), operation)?; self.repo_stack.push(repo); let candidates = self.fold_expression(candidates)?; @@ -2194,9 +2244,9 @@ impl ExpressionStateFolder for ExpressionSymbolResolver<'_> { fn resolve_symbols( repo: &dyn Repo, - expression: &RevsetExpression, + expression: &UserRevsetExpression, symbol_resolver: &dyn SymbolResolver, -) -> Result, RevsetResolutionError> { +) -> Result, RevsetResolutionError> { let mut resolver = ExpressionSymbolResolver::new(repo, symbol_resolver); resolver.fold_expression(expression) } @@ -2210,7 +2260,10 @@ fn resolve_symbols( /// commit ids to make `all()` include hidden-but-specified commits. The /// return type `ResolvedExpression` is stricter than `RevsetExpression`, /// and isn't designed for such transformation. -fn resolve_visibility(repo: &dyn Repo, expression: &RevsetExpression) -> ResolvedExpression { +fn resolve_visibility( + repo: &dyn Repo, + expression: &ResolvedRevsetExpression, +) -> ResolvedExpression { let context = VisibilityResolutionContext { visible_heads: &repo.view().heads().iter().cloned().collect_vec(), root: repo.store().root_commit_id(), @@ -2226,7 +2279,7 @@ struct VisibilityResolutionContext<'a> { impl VisibilityResolutionContext<'_> { /// Resolves expression tree as set. - fn resolve(&self, expression: &RevsetExpression) -> ResolvedExpression { + fn resolve(&self, expression: &ResolvedRevsetExpression) -> ResolvedExpression { match expression { RevsetExpression::None => ResolvedExpression::Commits(vec![]), RevsetExpression::All => self.resolve_all(), @@ -2235,9 +2288,7 @@ impl VisibilityResolutionContext<'_> { RevsetExpression::Commits(commit_ids) => { ResolvedExpression::Commits(commit_ids.clone()) } - RevsetExpression::CommitRef(_) => { - panic!("Expression '{expression:?}' should have been resolved by caller"); - } + RevsetExpression::CommitRef(commit_ref) => match *commit_ref {}, RevsetExpression::Ancestors { heads, generation } => ResolvedExpression::Ancestors { heads: self.resolve(heads).into(), generation: generation.clone(), @@ -2283,9 +2334,7 @@ impl VisibilityResolutionContext<'_> { predicate: self.resolve_predicate(expression), } } - RevsetExpression::AtOperation { .. } => { - panic!("Expression '{expression:?}' should have been resolved by caller"); - } + RevsetExpression::AtOperation { operation, .. } => match *operation {}, RevsetExpression::WithinVisibility { candidates, visible_heads, @@ -2359,7 +2408,10 @@ impl VisibilityResolutionContext<'_> { /// /// For filter expression, this never inserts a hidden `all()` since a /// filter predicate doesn't need to produce revisions to walk. - fn resolve_predicate(&self, expression: &RevsetExpression) -> ResolvedPredicateExpression { + fn resolve_predicate( + &self, + expression: &ResolvedRevsetExpression, + ) -> ResolvedPredicateExpression { match expression { RevsetExpression::None | RevsetExpression::All @@ -2381,9 +2433,7 @@ impl VisibilityResolutionContext<'_> { ResolvedPredicateExpression::Filter(predicate.clone()) } RevsetExpression::AsFilter(candidates) => self.resolve_predicate(candidates), - RevsetExpression::AtOperation { .. } => { - panic!("Expression '{expression:?}' should have been resolved by caller"); - } + RevsetExpression::AtOperation { operation, .. } => match *operation {}, // Filters should be intersected with all() within the at-op repo. RevsetExpression::WithinVisibility { .. } => { ResolvedPredicateExpression::Set(self.resolve(expression).into()) @@ -2599,21 +2649,21 @@ mod tests { use super::*; - fn parse(revset_str: &str) -> Result, RevsetParseError> { + fn parse(revset_str: &str) -> Result, RevsetParseError> { parse_with_aliases(revset_str, [] as [(&str, &str); 0]) } fn parse_with_workspace( revset_str: &str, workspace_id: &WorkspaceId, - ) -> Result, RevsetParseError> { + ) -> Result, RevsetParseError> { parse_with_aliases_and_workspace(revset_str, [] as [(&str, &str); 0], workspace_id) } fn parse_with_aliases( revset_str: &str, aliases: impl IntoIterator, impl Into)>, - ) -> Result, RevsetParseError> { + ) -> Result, RevsetParseError> { let mut aliases_map = RevsetAliasesMap::new(); for (decl, defn) in aliases { aliases_map.insert(decl, defn).unwrap(); @@ -2633,7 +2683,7 @@ mod tests { revset_str: &str, aliases: impl IntoIterator, impl Into)>, workspace_id: &WorkspaceId, - ) -> Result, RevsetParseError> { + ) -> Result, RevsetParseError> { // Set up pseudo context to resolve `workspace_id@` and `file(path)` let path_converter = RepoPathUiConverter::Fs { cwd: PathBuf::from("/"), @@ -2660,14 +2710,14 @@ mod tests { fn parse_with_modifier( revset_str: &str, - ) -> Result<(Rc, Option), RevsetParseError> { + ) -> Result<(Rc, Option), RevsetParseError> { parse_with_aliases_and_modifier(revset_str, [] as [(&str, &str); 0]) } fn parse_with_aliases_and_modifier( revset_str: &str, aliases: impl IntoIterator, impl Into)>, - ) -> Result<(Rc, Option), RevsetParseError> { + ) -> Result<(Rc, Option), RevsetParseError> { let mut aliases_map = RevsetAliasesMap::new(); for (decl, defn) in aliases { aliases_map.insert(decl, defn).unwrap(); @@ -2704,10 +2754,10 @@ mod tests { fn test_revset_expression_building() { let settings = insta_settings(); let _guard = settings.bind_to_scope(); - let current_wc = RevsetExpression::working_copy(WorkspaceId::default()); - let foo_symbol = RevsetExpression::symbol("foo".to_string()); - let bar_symbol = RevsetExpression::symbol("bar".to_string()); - let baz_symbol = RevsetExpression::symbol("baz".to_string()); + let current_wc = UserRevsetExpression::working_copy(WorkspaceId::default()); + let foo_symbol = UserRevsetExpression::symbol("foo".to_string()); + let bar_symbol = UserRevsetExpression::symbol("bar".to_string()); + let baz_symbol = UserRevsetExpression::symbol("baz".to_string()); insta::assert_debug_snapshot!( current_wc, @@ -2779,7 +2829,7 @@ mod tests { ) "###); insta::assert_debug_snapshot!( - RevsetExpression::union_all(&[]), + UserRevsetExpression::union_all(&[]), @"None"); insta::assert_debug_snapshot!( RevsetExpression::union_all(&[current_wc.clone()]), @@ -2841,7 +2891,7 @@ mod tests { ) "###); insta::assert_debug_snapshot!( - RevsetExpression::coalesce(&[]), + UserRevsetExpression::coalesce(&[]), @"None"); insta::assert_debug_snapshot!( RevsetExpression::coalesce(&[current_wc.clone()]), @@ -3445,8 +3495,8 @@ mod tests { #[test] fn test_optimize_unchanged_subtree() { fn unwrap_union( - expression: &RevsetExpression, - ) -> (&Rc, &Rc) { + expression: &UserRevsetExpression, + ) -> (&Rc, &Rc) { match expression { RevsetExpression::Union(left, right) => (left, right), _ => panic!("unexpected expression: {expression:?}"),