changeset 53140:4a4c5e65eb5e

rust-matchers: generalize consumers of the Matcher api somewhat In the interest of making it easier to use the matchers API, be more generous when producing matchers, and be more permissive when consuming them. - Add the `Send` trait everywhere where we're returning a Boxed matcher, so the caller can be free to wrap it into an `Arc` to share between threads. - For all matcher consumers, accept an &impl instead of a dyn box so the caller is free to pass anything they want.
author Arseniy Alekseyev <aalekseyev@janestreet.com>
date Thu, 20 Mar 2025 14:35:11 +0000
parents 3659c13ac2aa
children 2ae3cd112399
files rust/hg-core/src/dirstate/dirstate_map.rs rust/hg-core/src/dirstate/status.rs rust/hg-core/src/narrow.rs rust/hg-core/src/operations/list_tracked_files.rs rust/hg-core/src/operations/status_rev_rev.rs rust/hg-core/src/sparse.rs rust/hg-pyo3/src/dirstate/status.rs rust/rhg/src/commands/status.rs
diffstat 8 files changed, 38 insertions(+), 31 deletions(-) [+]
line wrap: on
line diff
--- a/rust/hg-core/src/dirstate/dirstate_map.rs	Thu Mar 20 12:10:08 2025 +0000
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs	Thu Mar 20 14:35:11 2025 +0000
@@ -1383,7 +1383,7 @@
     /// we need to borrow from `Self`.
     pub fn with_status<R>(
         &mut self,
-        matcher: &(dyn Matcher + Sync),
+        matcher: &(impl Matcher + Sync),
         root_dir: PathBuf,
         ignore_files: Vec<PathBuf>,
         options: StatusOptions,
--- a/rust/hg-core/src/dirstate/status.rs	Thu Mar 20 12:10:08 2025 +0000
+++ b/rust/hg-core/src/dirstate/status.rs	Thu Mar 20 14:35:11 2025 +0000
@@ -73,7 +73,7 @@
 /// `Box<dyn Trait>` is syntactic sugar for `Box<dyn Trait + 'static>`, so add
 /// an explicit lifetime here to not fight `'static` bounds "out of nowhere".
 pub type IgnoreFnType<'a> =
-    Box<dyn for<'r> Fn(&'r HgPath) -> bool + Sync + 'a>;
+    Box<dyn for<'r> Fn(&'r HgPath) -> bool + Sync + Send + 'a>;
 
 /// We have a good mix of owned (from directory traversal) and borrowed (from
 /// the dirstate/explicit) paths, this comes up a lot.
@@ -180,7 +180,7 @@
 #[tracing::instrument(level = "debug", skip_all)]
 pub fn status<'dirstate>(
     dmap: &'dirstate mut DirstateMap,
-    matcher: &(dyn Matcher + Sync),
+    matcher: &(impl Matcher + Sync),
     root_dir: PathBuf,
     ignore_files: Vec<PathBuf>,
     options: StatusOptions,
--- a/rust/hg-core/src/narrow.rs	Thu Mar 20 12:10:08 2025 +0000
+++ b/rust/hg-core/src/narrow.rs	Thu Mar 20 14:35:11 2025 +0000
@@ -30,7 +30,10 @@
 /// warnings to display.
 pub fn matcher(
     repo: &Repo,
-) -> Result<(Box<dyn Matcher + Sync>, Vec<SparseWarning>), SparseConfigError> {
+) -> Result<
+    (Box<dyn Matcher + Sync + Send>, Vec<SparseWarning>),
+    SparseConfigError,
+> {
     let mut warnings = vec![];
     if !repo.requirements().contains(NARROW_REQUIREMENT) {
         return Ok((Box::new(AlwaysMatcher), warnings));
@@ -78,7 +81,7 @@
     )?;
     warnings.extend(subwarnings.into_iter().map(From::from));
 
-    let mut m: Box<dyn Matcher + Sync> =
+    let mut m: Box<dyn Matcher + Sync + Send> =
         Box::new(IncludeMatcher::new(patterns)?);
 
     let (patterns, subwarnings) = parse_pattern_file_contents(
--- a/rust/hg-core/src/operations/list_tracked_files.rs	Thu Mar 20 12:10:08 2025 +0000
+++ b/rust/hg-core/src/operations/list_tracked_files.rs	Thu Mar 20 14:35:11 2025 +0000
@@ -17,11 +17,11 @@
 use crate::{Node, UncheckedRevision};
 
 /// List files under Mercurial control at a given revset.
-pub fn list_revset_tracked_files(
+pub fn list_revset_tracked_files<M: Matcher>(
     repo: &Repo,
     revset: &str,
-    narrow_matcher: Box<dyn Matcher + Sync>,
-) -> Result<FilesForRev, RevlogError> {
+    narrow_matcher: M,
+) -> Result<FilesForRev<M>, RevlogError> {
     match crate::revset::resolve_single(revset, repo)?.exclude_wdir() {
         Some(rev) => list_rev_tracked_files(repo, rev.into(), narrow_matcher),
         None => {
@@ -31,11 +31,11 @@
 }
 
 /// List files under Mercurial control at a given revision.
-pub fn list_rev_tracked_files(
+pub fn list_rev_tracked_files<M: Matcher>(
     repo: &Repo,
     rev: UncheckedRevision,
-    narrow_matcher: Box<dyn Matcher + Sync>,
-) -> Result<FilesForRev, RevlogError> {
+    narrow_matcher: M,
+) -> Result<FilesForRev<M>, RevlogError> {
     // TODO move this to the repo itself
     // This implies storing the narrow matcher in the repo, bubbling up the
     // errors and warnings, so it's a bit of churn. In the meantime, the repo
@@ -64,16 +64,16 @@
     })
 }
 
-pub struct FilesForRev {
+pub struct FilesForRev<M> {
     manifest: Manifest,
-    narrow_matcher: Box<dyn Matcher + Sync>,
+    narrow_matcher: M,
 }
 
 /// Like [`crate::revlog::manifest::ManifestEntry`], but with the `Node`
 /// already checked.
 pub type ExpandedManifestEntry<'a> = (&'a HgPath, Node, Option<NonZeroU8>);
 
-impl FilesForRev {
+impl<M: Matcher> FilesForRev<M> {
     pub fn iter(
         &self,
     ) -> impl Iterator<Item = Result<ExpandedManifestEntry, HgError>> {
--- a/rust/hg-core/src/operations/status_rev_rev.rs	Thu Mar 20 12:10:08 2025 +0000
+++ b/rust/hg-core/src/operations/status_rev_rev.rs	Thu Mar 20 14:35:11 2025 +0000
@@ -37,10 +37,10 @@
     // TODO: For --rev --rev --copies use a precomputed copy map
 }
 
-pub struct StatusRevRev<'a> {
+pub struct StatusRevRev<'a, M> {
     manifest1: Manifest,
     manifest2: Manifest,
-    narrow_matcher: Box<dyn Matcher>,
+    narrow_matcher: M,
     copies: Option<(ListCopies, CopyStrategy<'a>)>,
 }
 
@@ -53,12 +53,12 @@
     })
 }
 
-pub fn status_rev_rev_no_copies(
+pub fn status_rev_rev_no_copies<M: Matcher>(
     repo: &Repo,
     rev1: Revision,
     rev2: Revision,
-    narrow_matcher: Box<dyn Matcher>,
-) -> Result<StatusRevRev, HgError> {
+    narrow_matcher: M,
+) -> Result<StatusRevRev<M>, HgError> {
     Ok(StatusRevRev {
         manifest1: manifest_for_rev(repo, rev1)?,
         manifest2: manifest_for_rev(repo, rev2)?,
@@ -68,12 +68,12 @@
 }
 
 /// Computes the status of `rev` against its first parent.
-pub fn status_change(
+pub fn status_change<M: Matcher>(
     repo: &Repo,
     rev: Revision,
-    narrow_matcher: Box<dyn Matcher>,
+    narrow_matcher: M,
     list_copies: Option<ListCopies>,
-) -> Result<StatusRevRev, HgError> {
+) -> Result<StatusRevRev<M>, HgError> {
     let parent = repo.changelog()?.revlog.get_entry(rev)?.p1();
     let parent = parent.unwrap_or(NULL_REVISION);
     Ok(StatusRevRev {
@@ -84,7 +84,7 @@
     })
 }
 
-impl StatusRevRev<'_> {
+impl<M: Matcher> StatusRevRev<'_, M> {
     pub fn iter(
         &self,
     ) -> impl Iterator<Item = Result<(StatusPath<'_>, DiffStatus), HgError>>
--- a/rust/hg-core/src/sparse.rs	Thu Mar 20 12:10:08 2025 +0000
+++ b/rust/hg-core/src/sparse.rs	Thu Mar 20 14:35:11 2025 +0000
@@ -325,7 +325,10 @@
 /// Obtain a matcher for sparse working directories.
 pub fn matcher(
     repo: &Repo,
-) -> Result<(Box<dyn Matcher + Sync>, Vec<SparseWarning>), SparseConfigError> {
+) -> Result<
+    (Box<dyn Matcher + Sync + Send>, Vec<SparseWarning>),
+    SparseConfigError,
+> {
     let mut warnings = vec![];
     if !repo.requirements().contains(SPARSE_REQUIREMENT) {
         return Ok((Box::new(AlwaysMatcher), warnings));
@@ -361,7 +364,8 @@
         let config = patterns_for_rev(repo, *rev);
         if let Ok(Some(config)) = config {
             warnings.extend(config.warnings);
-            let mut m: Box<dyn Matcher + Sync> = Box::new(AlwaysMatcher);
+            let mut m: Box<dyn Matcher + Sync + Send> =
+                Box::new(AlwaysMatcher);
             if !config.includes.is_empty() {
                 let (patterns, subwarnings) = parse_pattern_file_contents(
                     &config.includes,
@@ -390,7 +394,7 @@
             matchers.push(m);
         }
     }
-    let result: Box<dyn Matcher + Sync> = match matchers.len() {
+    let result: Box<dyn Matcher + Sync + Send> = match matchers.len() {
         0 => Box::new(AlwaysMatcher),
         1 => matchers.pop().expect("1 is equal to 0"),
         _ => Box::new(UnionMatcher::new(matchers)),
@@ -404,9 +408,9 @@
 /// Returns a matcher that returns true for any of the forced includes before
 /// testing against the actual matcher
 fn force_include_matcher(
-    result: Box<dyn Matcher + Sync>,
+    result: Box<dyn Matcher + Sync + Send>,
     temp_includes: &[Vec<u8>],
-) -> Result<Box<dyn Matcher + Sync>, PatternError> {
+) -> Result<Box<dyn Matcher + Sync + Send>, PatternError> {
     if temp_includes.is_empty() {
         return Ok(result);
     }
--- a/rust/hg-pyo3/src/dirstate/status.rs	Thu Mar 20 12:10:08 2025 +0000
+++ b/rust/hg-pyo3/src/dirstate/status.rs	Thu Mar 20 14:35:11 2025 +0000
@@ -97,7 +97,7 @@
 
 fn extract_matcher(
     matcher: &Bound<'_, PyAny>,
-) -> PyResult<Box<dyn Matcher + Sync>> {
+) -> PyResult<Box<dyn Matcher + Sync + Send>> {
     let py = matcher.py();
     let tampered = matcher
         .call_method0(intern!(py, "was_tampered_with_nonrec"))?
@@ -209,7 +209,7 @@
     let matcher = extract_matcher(matcher)?;
     DirstateMap::with_inner_write(dmap, |_dm_ref, mut inner| {
         inner.with_status(
-            &*matcher,
+            &matcher,
             root_dir.to_path_buf(),
             ignore_files,
             StatusOptions {
--- a/rust/rhg/src/commands/status.rs	Thu Mar 20 12:10:08 2025 +0000
+++ b/rust/rhg/src/commands/status.rs	Thu Mar 20 14:35:11 2025 +0000
@@ -566,7 +566,7 @@
 
     let (fixup, mut dirstate_write_needed, filesystem_time_at_status_start) =
         dmap.with_status(
-            matcher.as_ref(),
+            &matcher,
             repo.working_directory_path().to_owned(),
             ignore_files(repo, config),
             options,