cli, lib: convert revset expressions to use Arc over Rc

We want these to be `Send` and `Sync` so we can send them between
threads in our multi-threaded backend. This requires a bunch of subsequent
(but obvious) changes throughout cli and the tests.

Co-authored-by: Benjamin Brittain <ben@ersc.io>
Signed-off-by: Austin Seipp <austin@ersc.io>
Signed-off-by: Austin Seipp <aseipp@pobox.com>

cli/examples/custom-commit-templater/main.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use std::any::Any;
-use std::rc::Rc;
+use std::sync::Arc;
 
 use jj_cli::cli_util::CliRunner;
 use jj_cli::commit_templater::CommitTemplateBuildFnTable;
@@ -187,10 +187,10 @@ fn even_digits(
     _diagnostics: &mut RevsetDiagnostics,
     function: &FunctionCallNode,
     _context: &LoweringContext,
-) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
+) -> Result<Arc<UserRevsetExpression>, RevsetParseError> {
     function.expect_no_arguments()?;
     Ok(RevsetExpression::filter(RevsetFilterPredicate::Extension(
-        Rc::new(EvenDigitsFilter),
+        Arc::new(EvenDigitsFilter),
     )))
 }
 

cli/src/cli_util.rs

@@ -782,8 +782,8 @@ pub struct WorkspaceCommandEnvironment {
     template_aliases_map: TemplateAliasesMap,
     path_converter: RepoPathUiConverter,
     workspace_name: WorkspaceNameBuf,
-    immutable_heads_expression: Rc<UserRevsetExpression>,
-    short_prefixes_expression: Option<Rc<UserRevsetExpression>>,
+    immutable_heads_expression: Arc<UserRevsetExpression>,
+    short_prefixes_expression: Option<Arc<UserRevsetExpression>>,
     conflict_marker_style: ConflictMarkerStyle,
 }
 
@@ -854,14 +854,14 @@ impl WorkspaceCommandEnvironment {
     }
 
     /// User-configured expression defining the immutable set.
-    pub fn immutable_expression(&self) -> Rc<UserRevsetExpression> {
+    pub fn immutable_expression(&self) -> Arc<UserRevsetExpression> {
         // Negated ancestors expression `~::(<heads> | root())` is slightly
         // easier to optimize than negated union `~(::<heads> | root())`.
         self.immutable_heads_expression.ancestors()
     }
 
     /// User-configured expression defining the heads of the immutable set.
-    pub fn immutable_heads_expression(&self) -> &Rc<UserRevsetExpression> {
+    pub fn immutable_heads_expression(&self) -> &Arc<UserRevsetExpression> {
         &self.immutable_heads_expression
     }
 
@@ -873,7 +873,7 @@ impl WorkspaceCommandEnvironment {
     fn load_immutable_heads_expression(
         &self,
         ui: &Ui,
-    ) -> Result<Rc<UserRevsetExpression>, CommandError> {
+    ) -> Result<Arc<UserRevsetExpression>, CommandError> {
         let mut diagnostics = RevsetDiagnostics::new();
         let expression = revset_util::parse_immutable_heads_expression(
             &mut diagnostics,
@@ -887,7 +887,7 @@ impl WorkspaceCommandEnvironment {
     fn load_short_prefixes_expression(
         &self,
         ui: &Ui,
-    ) -> Result<Option<Rc<UserRevsetExpression>>, CommandError> {
+    ) -> Result<Option<Arc<UserRevsetExpression>>, CommandError> {
         let revset_string = self
             .settings
             .get_string("revsets.short-prefixes")
@@ -1641,7 +1641,7 @@ to the current parents may contain changes from multiple commits.
 
     pub fn attach_revset_evaluator(
         &self,
-        expression: Rc<UserRevsetExpression>,
+        expression: Arc<UserRevsetExpression>,
     ) -> RevsetExpressionEvaluator<'_> {
         RevsetExpressionEvaluator::new(
             self.repo().as_ref(),
@@ -2168,7 +2168,7 @@ See https://jj-vcs.github.io/jj/latest/working-copy/#stale-working-copy \
         let added_conflicts_expr = old_heads.range(&new_heads).intersection(&conflicts);
 
         let get_commits =
-            |expr: Rc<ResolvedRevsetExpression>| -> Result<Vec<Commit>, CommandError> {
+            |expr: Arc<ResolvedRevsetExpression>| -> Result<Vec<Commit>, CommandError> {
                 let commits = expr
                     .evaluate(new_repo)?
                     .iter()
@@ -3113,8 +3113,8 @@ pub fn compute_commit_location(
 /// parents of the given commits.
 fn ensure_no_commit_loop(
     repo: &ReadonlyRepo,
-    children_expression: &Rc<ResolvedRevsetExpression>,
-    parents_expression: &Rc<ResolvedRevsetExpression>,
+    children_expression: &Arc<ResolvedRevsetExpression>,
+    parents_expression: &Arc<ResolvedRevsetExpression>,
     commit_type: &str,
 ) -> Result<(), CommandError> {
     if let Some(commit_id) = children_expression

cli/src/commands/bench/revset.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::rc::Rc;
+use std::sync::Arc;
 use std::time::Instant;
 
 use criterion::BatchSize;
@@ -85,7 +85,7 @@ fn bench_revset<M: Measurement>(
         .clone();
     // Time both evaluation and iteration.
     let routine = |workspace_command: &WorkspaceCommandHelper,
-                   expression: Rc<UserRevsetExpression>| {
+                   expression: Arc<UserRevsetExpression>| {
         // Evaluate the expression without parsing/evaluating short-prefixes.
         let repo = workspace_command.repo().as_ref();
         let symbol_resolver =

cli/src/commit_templater.rs

@@ -22,6 +22,7 @@ use std::fmt;
 use std::fmt::Display;
 use std::io;
 use std::rc::Rc;
+use std::sync::Arc;
 
 use bstr::BString;
 use futures::StreamExt as _;
@@ -130,7 +131,7 @@ pub struct CommitTemplateLanguage<'repo> {
     // WorkspaceName are contained in RevsetParseContext for example.
     revset_parse_context: RevsetParseContext<'repo>,
     id_prefix_context: &'repo IdPrefixContext,
-    immutable_expression: Rc<UserRevsetExpression>,
+    immutable_expression: Arc<UserRevsetExpression>,
     conflict_marker_style: ConflictMarkerStyle,
     build_fn_table: CommitTemplateBuildFnTable<'repo>,
     keyword_cache: CommitKeywordCache<'repo>,
@@ -147,7 +148,7 @@ impl<'repo> CommitTemplateLanguage<'repo> {
         workspace_name: &WorkspaceName,
         revset_parse_context: RevsetParseContext<'repo>,
         id_prefix_context: &'repo IdPrefixContext,
-        immutable_expression: Rc<UserRevsetExpression>,
+        immutable_expression: Arc<UserRevsetExpression>,
         conflict_marker_style: ConflictMarkerStyle,
         extensions: &[impl AsRef<dyn CommitTemplateLanguageExtension>],
     ) -> Self {
@@ -2688,7 +2689,7 @@ mod tests {
         id_prefix_context: IdPrefixContext,
         revset_aliases_map: RevsetAliasesMap,
         template_aliases_map: TemplateAliasesMap,
-        immutable_expression: Rc<UserRevsetExpression>,
+        immutable_expression: Arc<UserRevsetExpression>,
         extra_functions: HashMap<&'static str, BuildFunctionFn>,
     }
 

cli/src/movement_util.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use std::io::Write as _;
-use std::rc::Rc;
+use std::sync::Arc;
 
 use itertools::Itertools as _;
 use jj_lib::backend::CommitId;
@@ -118,10 +118,10 @@ impl Direction {
 
     fn build_target_revset(
         &self,
-        working_revset: &Rc<ResolvedRevsetExpression>,
-        start_revset: &Rc<ResolvedRevsetExpression>,
+        working_revset: &Arc<ResolvedRevsetExpression>,
+        start_revset: &Arc<ResolvedRevsetExpression>,
         args: &MovementArgsInternal,
-    ) -> Result<Rc<ResolvedRevsetExpression>, CommandError> {
+    ) -> Result<Arc<ResolvedRevsetExpression>, CommandError> {
         let nth = match (self, args.should_edit) {
             (Self::Next, true) => start_revset.descendants_at(args.offset),
             (Self::Next, false) => start_revset

cli/src/revset_util.rs

@@ -15,7 +15,6 @@
 //! Utility for parsing and evaluating user-provided revset expressions.
 
 use std::io;
-use std::rc::Rc;
 use std::sync::Arc;
 
 use itertools::Itertools as _;
@@ -66,7 +65,7 @@ pub struct RevsetExpressionEvaluator<'repo> {
     repo: &'repo dyn Repo,
     extensions: Arc<RevsetExtensions>,
     id_prefix_context: &'repo IdPrefixContext,
-    expression: Rc<UserRevsetExpression>,
+    expression: Arc<UserRevsetExpression>,
 }
 
 impl<'repo> RevsetExpressionEvaluator<'repo> {
@@ -74,7 +73,7 @@ impl<'repo> RevsetExpressionEvaluator<'repo> {
         repo: &'repo dyn Repo,
         extensions: Arc<RevsetExtensions>,
         id_prefix_context: &'repo IdPrefixContext,
-        expression: Rc<UserRevsetExpression>,
+        expression: Arc<UserRevsetExpression>,
     ) -> Self {
         Self {
             repo,
@@ -85,17 +84,17 @@ impl<'repo> RevsetExpressionEvaluator<'repo> {
     }
 
     /// Returns the underlying expression.
-    pub fn expression(&self) -> &Rc<UserRevsetExpression> {
+    pub fn expression(&self) -> &Arc<UserRevsetExpression> {
         &self.expression
     }
 
     /// Intersects the underlying expression with the `other` expression.
-    pub fn intersect_with(&mut self, other: &Rc<UserRevsetExpression>) {
+    pub fn intersect_with(&mut self, other: &Arc<UserRevsetExpression>) {
         self.expression = self.expression.intersection(other);
     }
 
     /// Resolves user symbols in the expression, returns new expression.
-    pub fn resolve(&self) -> Result<Rc<ResolvedRevsetExpression>, RevsetResolutionError> {
+    pub fn resolve(&self) -> Result<Arc<ResolvedRevsetExpression>, RevsetResolutionError> {
         let symbol_resolver = default_symbol_resolver(
             self.repo,
             self.extensions.symbol_resolvers(),
@@ -218,7 +217,7 @@ pub fn default_symbol_resolver<'a>(
 pub fn parse_immutable_heads_expression(
     diagnostics: &mut RevsetDiagnostics,
     context: &RevsetParseContext,
-) -> Result<Rc<UserRevsetExpression>, RevsetParseError> {
+) -> Result<Arc<UserRevsetExpression>, RevsetParseError> {
     let (_, _, immutable_heads_str) = context
         .aliases_map
         .get_function(USER_IMMUTABLE_HEADS, 0)

lib/src/absorb.rs

@@ -18,7 +18,7 @@
 use std::cmp;
 use std::collections::HashMap;
 use std::ops::Range;
-use std::rc::Rc;
+use std::sync::Arc;
 
 use bstr::BString;
 use futures::StreamExt as _;
@@ -92,7 +92,7 @@ pub struct SelectedTrees {
 pub async fn split_hunks_to_trees(
     repo: &dyn Repo,
     source: &AbsorbSource,
-    destinations: &Rc<ResolvedRevsetExpression>,
+    destinations: &Arc<ResolvedRevsetExpression>,
     matcher: &dyn Matcher,
 ) -> Result<SelectedTrees, AbsorbError> {
     let mut selected_trees = SelectedTrees::default();

lib/src/annotate.rs

@@ -22,7 +22,7 @@ use std::collections::HashMap;
 use std::collections::hash_map;
 use std::iter;
 use std::ops::Range;
-use std::rc::Rc;
+use std::sync::Arc;
 
 use bstr::BStr;
 use bstr::BString;
@@ -212,7 +212,7 @@ impl FileAnnotator {
     pub fn compute(
         &mut self,
         repo: &dyn Repo,
-        domain: &Rc<ResolvedRevsetExpression>,
+        domain: &Arc<ResolvedRevsetExpression>,
     ) -> Result<(), RevsetEvaluationError> {
         process_commits(repo, &mut self.state, domain, &self.file_path)
     }
@@ -292,7 +292,7 @@ pub struct LineOrigin {
 fn process_commits(
     repo: &dyn Repo,
     state: &mut AnnotationState,
-    domain: &Rc<ResolvedRevsetExpression>,
+    domain: &Arc<ResolvedRevsetExpression>,
     file_name: &RepoPath,
 ) -> Result<(), RevsetEvaluationError> {
     let predicate = RevsetFilterPredicate::File(FilesetExpression::file_path(file_name.to_owned()));

lib/src/bisect.rs

@@ -15,7 +15,7 @@
 //! Bisect a range of commits.
 
 use std::collections::HashSet;
-use std::rc::Rc;
+use std::sync::Arc;
 
 use itertools::Itertools as _;
 use thiserror::Error;
@@ -51,7 +51,7 @@ pub enum Evaluation {
 /// Performs bisection to find the first bad commit in a range.
 pub struct Bisector<'repo> {
     repo: &'repo dyn Repo,
-    input_range: Rc<ResolvedRevsetExpression>,
+    input_range: Arc<ResolvedRevsetExpression>,
     good_commits: HashSet<CommitId>,
     bad_commits: HashSet<CommitId>,
     skipped_commits: HashSet<CommitId>,
@@ -82,7 +82,7 @@ impl<'repo> Bisector<'repo> {
     /// Parents of the range's roots are assumed to be good.
     pub fn new(
         repo: &'repo dyn Repo,
-        input_range: Rc<ResolvedRevsetExpression>,
+        input_range: Arc<ResolvedRevsetExpression>,
     ) -> Result<Self, BisectionError> {
         let bad_commits = input_range.heads().evaluate(repo)?.iter().try_collect()?;
         Ok(Self {

lib/src/id_prefix.rs

@@ -16,11 +16,10 @@
 
 use std::iter;
 use std::marker::PhantomData;
-use std::rc::Rc;
 use std::sync::Arc;
 
 use itertools::Itertools as _;
-use once_cell::unsync::OnceCell;
+use once_cell::sync::OnceCell;
 use thiserror::Error;
 
 use crate::backend::ChangeId;
@@ -47,7 +46,7 @@ pub enum IdPrefixIndexLoadError {
 }
 
 struct DisambiguationData {
-    expression: Rc<UserRevsetExpression>,
+    expression: Arc<UserRevsetExpression>,
     indexes: OnceCell<Indexes>,
 }
 
@@ -61,7 +60,7 @@ impl DisambiguationData {
     fn indexes(
         &self,
         repo: &dyn Repo,
-        extensions: &[impl AsRef<dyn SymbolResolverExtension>],
+        extensions: &[Box<dyn SymbolResolverExtension>],
     ) -> Result<&Indexes, IdPrefixIndexLoadError> {
         self.indexes.get_or_try_init(|| {
             let symbol_resolver = SymbolResolver::new(repo, extensions);
@@ -124,7 +123,7 @@ impl IdPrefixContext {
         }
     }
 
-    pub fn disambiguate_within(mut self, expression: Rc<UserRevsetExpression>) -> Self {
+    pub fn disambiguate_within(mut self, expression: Arc<UserRevsetExpression>) -> Self {
         self.disambiguation = Some(DisambiguationData {
             expression,
             indexes: OnceCell::new(),

lib/tests/test_annotate.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 
 use std::fmt::Write as _;
-use std::rc::Rc;
+use std::sync::Arc;
 
 use itertools::Itertools as _;
 use jj_lib::annotate::FileAnnotation;
@@ -67,7 +67,7 @@ fn annotate(repo: &dyn Repo, commit: &Commit, file_path: &RepoPath) -> String {
 fn annotate_within(
     repo: &dyn Repo,
     commit: &Commit,
-    domain: &Rc<ResolvedRevsetExpression>,
+    domain: &Arc<ResolvedRevsetExpression>,
     file_path: &RepoPath,
 ) -> String {
     let mut annotator = FileAnnotator::from_commit(commit, file_path).unwrap();

lib/tests/test_bisect.rs

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::rc::Rc;
+use std::sync::Arc;
 
 use assert_matches::assert_matches;
 use jj_lib::backend::CommitId;
@@ -28,7 +28,7 @@ use testutils::write_random_commit_with_parents;
 
 fn test_bisection<'a>(
     repo: &dyn Repo,
-    input_range: &Rc<ResolvedRevsetExpression>,
+    input_range: &Arc<ResolvedRevsetExpression>,
     results: impl IntoIterator<Item = (&'a CommitId, Evaluation)>,
 ) -> BisectionResult {
     let mut bisector = Bisector::new(repo, input_range.clone()).unwrap();

lib/tests/test_revset_optimized.rs

@@ -19,7 +19,7 @@
 //! The default is `256`, which might be too small to catch edge-case bugs.
 //! <https://proptest-rs.github.io/proptest/proptest/tutorial/config.html>
 
-use std::rc::Rc;
+use std::sync::Arc;
 
 use itertools::Itertools as _;
 use jj_lib::backend::CommitId;
@@ -74,7 +74,7 @@ fn rebase_descendants(repo: &mut MutableRepo) -> Vec<Commit> {
 fn arb_expression(
     known_commits: Vec<CommitId>,
     visible_heads: Vec<Vec<CommitId>>,
-) -> impl Strategy<Value = Rc<ResolvedRevsetExpression>> {
+) -> impl Strategy<Value = Arc<ResolvedRevsetExpression>> {
     // https://proptest-rs.github.io/proptest/proptest/tutorial/recursive.html
     let max_commits = known_commits.len();
     let leaf_expr = prop_oneof![
@@ -132,7 +132,7 @@ fn arb_expression(
                     expr.clone(),
                     proptest::sample::select(visible_heads.clone())
                 )
-                    .prop_map(|(candidates, visible_heads)| Rc::new(
+                    .prop_map(|(candidates, visible_heads)| Arc::new(
                         RevsetExpression::WithinVisibility {
                             candidates,
                             visible_heads
@@ -152,7 +152,7 @@ fn arb_expression(
 
 fn verify_optimized(
     repo: &dyn Repo,
-    expression: &Rc<ResolvedRevsetExpression>,
+    expression: &Arc<ResolvedRevsetExpression>,
 ) -> Result<(), TestCaseError> {
     let optimized_revset = expression.clone().evaluate(repo).unwrap();
     let unoptimized_revset = expression.clone().evaluate_unoptimized(repo).unwrap();