Patchwork D7524: rust-dirs-multiset: use `AsRef` instead of concrete types when possible

login
register
mail settings
Submitter phabricator
Date Nov. 29, 2019, 5:57 p.m.
Message ID <differential-rev-PHID-DREV-m2ch6kh223bb37h4tgku-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/43530/
State Superseded
Headers show

Comments

phabricator - Nov. 29, 2019, 5:57 p.m.
Alphare created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I also renamed `vec` to `dirstate`, because it was not a great name.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/dirstate/dirs_multiset.rs

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
phabricator - Dec. 10, 2019, 3:16 p.m.
pulkit added a comment.


  This one fails to apply on default. Skipping pushing this and it's accepted children.

REPOSITORY
  rHG Mercurial

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7524/new/

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

To: Alphare, #hg-reviewers, kevincox, pulkit
Cc: durin42, kevincox, mercurial-devel

Patch

diff --git a/rust/hg-core/src/dirstate/dirs_multiset.rs b/rust/hg-core/src/dirstate/dirs_multiset.rs
--- a/rust/hg-core/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-core/src/dirstate/dirs_multiset.rs
@@ -28,14 +28,14 @@ 
     ///
     /// If `skip_state` is provided, skips dirstate entries with equal state.
     pub fn from_dirstate(
-        vec: &FastHashMap<HgPathBuf, DirstateEntry>,
+        dirstate: &FastHashMap<HgPathBuf, DirstateEntry>,
         skip_state: Option<EntryState>,
     ) -> Self {
         let mut multiset = DirsMultiset {
             inner: FastHashMap::default(),
         };
 
-        for (filename, DirstateEntry { state, .. }) in vec {
+        for (filename, DirstateEntry { state, .. }) in dirstate {
             // This `if` is optimized out of the loop
             if let Some(skip) = skip_state {
                 if skip != *state {
@@ -50,13 +50,13 @@ 
     }
 
     /// Initializes the multiset from a manifest.
-    pub fn from_manifest(vec: &Vec<HgPathBuf>) -> Self {
+    pub fn from_manifest(manifest: &[impl AsRef<HgPath>]) -> Self {
         let mut multiset = DirsMultiset {
             inner: FastHashMap::default(),
         };
 
-        for filename in vec {
-            multiset.add_path(filename);
+        for filename in manifest {
+            multiset.add_path(filename.as_ref());
         }
 
         multiset
@@ -65,8 +65,8 @@ 
     /// Increases the count of deepest directory contained in the path.
     ///
     /// If the directory is not yet in the map, adds its parents.
-    pub fn add_path(&mut self, path: &HgPath) {
-        for subpath in files::find_dirs(path) {
+    pub fn add_path(&mut self, path: impl AsRef<HgPath>) {
+        for subpath in files::find_dirs(path.as_ref()) {
             if let Some(val) = self.inner.get_mut(subpath) {
                 *val += 1;
                 break;
@@ -82,9 +82,9 @@ 
     /// If the directory is not in the map, something horrible has happened.
     pub fn delete_path(
         &mut self,
-        path: &HgPath,
+        path: impl AsRef<HgPath>,
     ) -> Result<(), DirstateMapError> {
-        for subpath in files::find_dirs(path) {
+        for subpath in files::find_dirs(path.as_ref()) {
             match self.inner.entry(subpath.to_owned()) {
                 Entry::Occupied(mut entry) => {
                     let val = entry.get().clone();
@@ -96,7 +96,7 @@ 
                 }
                 Entry::Vacant(_) => {
                     return Err(DirstateMapError::PathNotFound(
-                        path.to_owned(),
+                        path.as_ref().to_owned(),
                     ))
                 }
             };
@@ -105,8 +105,8 @@ 
         Ok(())
     }
 
-    pub fn contains(&self, key: &HgPath) -> bool {
-        self.inner.contains_key(key)
+    pub fn contains(&self, key: impl AsRef<HgPath>) -> bool {
+        self.inner.contains_key(key.as_ref())
     }
 
     pub fn iter(&self) -> DirsMultisetIter {
@@ -124,7 +124,8 @@ 
 
     #[test]
     fn test_delete_path_path_not_found() {
-        let mut map = DirsMultiset::from_manifest(&vec![]);
+        let manifest: Vec<HgPathBuf> = vec![];
+        let mut map = DirsMultiset::from_manifest(&manifest);
         let path = HgPathBuf::from_bytes(b"doesnotexist/");
         assert_eq!(
             Err(DirstateMapError::PathNotFound(path.to_owned())),
@@ -180,7 +181,8 @@ 
 
     #[test]
     fn test_add_path_empty_path() {
-        let mut map = DirsMultiset::from_manifest(&vec![]);
+        let manifest: Vec<HgPathBuf> = vec![];
+        let mut map = DirsMultiset::from_manifest(&manifest);
         let path = HgPath::new(b"");
         map.add_path(path);
 
@@ -189,7 +191,8 @@ 
 
     #[test]
     fn test_add_path_successful() {
-        let mut map = DirsMultiset::from_manifest(&vec![]);
+        let manifest: Vec<HgPathBuf> = vec![];
+        let mut map = DirsMultiset::from_manifest(&manifest);
 
         map.add_path(HgPath::new(b"a/"));
         assert_eq!(1, *map.inner.get(HgPath::new(b"a")).unwrap());
@@ -234,7 +237,8 @@ 
 
     #[test]
     fn test_dirsmultiset_new_empty() {
-        let new = DirsMultiset::from_manifest(&vec![]);
+        let manifest: Vec<HgPathBuf> = vec![];
+        let new = DirsMultiset::from_manifest(&manifest);
         let expected = DirsMultiset {
             inner: FastHashMap::default(),
         };
@@ -249,7 +253,7 @@ 
 
     #[test]
     fn test_dirsmultiset_new_no_skip() {
-        let input_vec = ["a/", "b/", "a/c", "a/d/"]
+        let input_vec: Vec<HgPathBuf> = ["a/", "b/", "a/c", "a/d/"]
             .iter()
             .map(|e| HgPathBuf::from_bytes(e.as_bytes()))
             .collect();