Patchwork D9071: rust-matchers: make `Matcher` trait object-safe

login
register
mail settings
Submitter phabricator
Date Sept. 23, 2020, 8:32 a.m.
Message ID <differential-rev-PHID-DREV-usurpe6a7dcpojdmv2qt-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/47256/
State Superseded
Headers show

Comments

phabricator - Sept. 23, 2020, 8:32 a.m.
Alphare created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Before this patch, it is not possible to create a `Matcher` trait-object (like
  `Box<dyn Matcher>`), because of the use of a generic parameters in some methods,
  namely `impl AsRef<HgPath>`.
  
  While this makes the interface less flexible for callers in theory, it does not
  change anything in the current codebase.
  
  Until something like [1] is implemented, this is a "tradeoff" that we need to
  make anyway.
  
  [1] https://internals.rust-lang.org/t/pre-rfc-expand-object-safety/12693

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D9071

AFFECTED FILES
  rust/hg-core/src/matchers.rs

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-core/src/matchers.rs b/rust/hg-core/src/matchers.rs
--- a/rust/hg-core/src/matchers.rs
+++ b/rust/hg-core/src/matchers.rs
@@ -49,9 +49,9 @@ 
     /// Explicitly listed files
     fn file_set(&self) -> Option<&HashSet<&HgPath>>;
     /// Returns whether `filename` is in `file_set`
-    fn exact_match(&self, filename: impl AsRef<HgPath>) -> bool;
+    fn exact_match(&self, filename: &HgPath) -> bool;
     /// Returns whether `filename` is matched by this matcher
-    fn matches(&self, filename: impl AsRef<HgPath>) -> bool;
+    fn matches(&self, filename: &HgPath) -> bool;
     /// Decides whether a directory should be visited based on whether it
     /// has potential matches in it or one of its subdirectories, and
     /// potentially lists which subdirectories of that directory should be
@@ -89,10 +89,7 @@ 
     /// no files in this dir to investigate (or equivalently that if there are
     /// files to investigate in 'dir' that it will always return
     /// `VisitChildrenSet::This`).
-    fn visit_children_set(
-        &self,
-        directory: impl AsRef<HgPath>,
-    ) -> VisitChildrenSet;
+    fn visit_children_set(&self, directory: &HgPath) -> VisitChildrenSet;
     /// Matcher will match everything and `files_set()` will be empty:
     /// optimization might be possible.
     fn matches_everything(&self) -> bool;
@@ -119,16 +116,13 @@ 
     fn file_set(&self) -> Option<&HashSet<&HgPath>> {
         None
     }
-    fn exact_match(&self, _filename: impl AsRef<HgPath>) -> bool {
+    fn exact_match(&self, _filename: &HgPath) -> bool {
         false
     }
-    fn matches(&self, _filename: impl AsRef<HgPath>) -> bool {
+    fn matches(&self, _filename: &HgPath) -> bool {
         true
     }
-    fn visit_children_set(
-        &self,
-        _directory: impl AsRef<HgPath>,
-    ) -> VisitChildrenSet {
+    fn visit_children_set(&self, _directory: &HgPath) -> VisitChildrenSet {
         VisitChildrenSet::Recursive
     }
     fn matches_everything(&self) -> bool {
@@ -143,9 +137,9 @@ 
 /// patterns.
 ///
 ///```
-/// use hg::{ matchers::{Matcher, FileMatcher}, utils::hg_path::HgPath };
+/// use hg::{ matchers::{Matcher, FileMatcher}, utils::hg_path::{HgPath, HgPathBuf} };
 ///
-/// let files = [HgPath::new(b"a.txt"), HgPath::new(br"re:.*\.c$")];
+/// let files = [HgPathBuf::from_bytes(b"a.txt"), HgPathBuf::from_bytes(br"re:.*\.c$")];
 /// let matcher = FileMatcher::new(&files).unwrap();
 ///
 /// assert_eq!(matcher.matches(HgPath::new(b"a.txt")), true);
@@ -160,15 +154,13 @@ 
 }
 
 impl<'a> FileMatcher<'a> {
-    pub fn new(
-        files: &'a [impl AsRef<HgPath>],
-    ) -> Result<Self, DirstateMapError> {
+    pub fn new(files: &'a [HgPathBuf]) -> Result<Self, DirstateMapError> {
         Ok(Self {
             files: HashSet::from_iter(files.iter().map(AsRef::as_ref)),
             dirs: DirsMultiset::from_manifest(files)?,
         })
     }
-    fn inner_matches(&self, filename: impl AsRef<HgPath>) -> bool {
+    fn inner_matches(&self, filename: &HgPath) -> bool {
         self.files.contains(filename.as_ref())
     }
 }
@@ -177,16 +169,13 @@ 
     fn file_set(&self) -> Option<&HashSet<&HgPath>> {
         Some(&self.files)
     }
-    fn exact_match(&self, filename: impl AsRef<HgPath>) -> bool {
+    fn exact_match(&self, filename: &HgPath) -> bool {
         self.inner_matches(filename)
     }
-    fn matches(&self, filename: impl AsRef<HgPath>) -> bool {
+    fn matches(&self, filename: &HgPath) -> bool {
         self.inner_matches(filename)
     }
-    fn visit_children_set(
-        &self,
-        directory: impl AsRef<HgPath>,
-    ) -> VisitChildrenSet {
+    fn visit_children_set(&self, directory: &HgPath) -> VisitChildrenSet {
         if self.files.is_empty() || !self.dirs.contains(&directory) {
             return VisitChildrenSet::Empty;
         }
@@ -270,18 +259,15 @@ 
         None
     }
 
-    fn exact_match(&self, _filename: impl AsRef<HgPath>) -> bool {
+    fn exact_match(&self, _filename: &HgPath) -> bool {
         false
     }
 
-    fn matches(&self, filename: impl AsRef<HgPath>) -> bool {
+    fn matches(&self, filename: &HgPath) -> bool {
         (self.match_fn)(filename.as_ref())
     }
 
-    fn visit_children_set(
-        &self,
-        directory: impl AsRef<HgPath>,
-    ) -> VisitChildrenSet {
+    fn visit_children_set(&self, directory: &HgPath) -> VisitChildrenSet {
         let dir = directory.as_ref();
         if self.prefix && self.roots.contains(dir) {
             return VisitChildrenSet::Recursive;
@@ -725,7 +711,7 @@ 
     #[test]
     fn test_filematcher_visit_children_set() {
         // Visitchildrenset
-        let files = vec![HgPath::new(b"dir/subdir/foo.txt")];
+        let files = vec![HgPathBuf::from_bytes(b"dir/subdir/foo.txt")];
         let matcher = FileMatcher::new(&files).unwrap();
 
         let mut set = HashSet::new();
@@ -766,11 +752,11 @@ 
     #[test]
     fn test_filematcher_visit_children_set_files_and_dirs() {
         let files = vec![
-            HgPath::new(b"rootfile.txt"),
-            HgPath::new(b"a/file1.txt"),
-            HgPath::new(b"a/b/file2.txt"),
+            HgPathBuf::from_bytes(b"rootfile.txt"),
+            HgPathBuf::from_bytes(b"a/file1.txt"),
+            HgPathBuf::from_bytes(b"a/b/file2.txt"),
             // No file in a/b/c
-            HgPath::new(b"a/b/c/d/file4.txt"),
+            HgPathBuf::from_bytes(b"a/b/c/d/file4.txt"),
         ];
         let matcher = FileMatcher::new(&files).unwrap();