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

[Raphaël Gomès]

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

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,
+dirstate: &FastHashMap,
 skip_state: Option,
 ) -> 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) -> Self {
+pub fn from_manifest(manifest: &[impl AsRef]) -> 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) {
+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,
 ) -> 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) -> 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 = 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 = 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 = 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 = 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 = ["a/", "b/", "a/c", "a/d/"]
 .iter()
 .map(|e| HgPathBuf::from_bytes(e.as_bytes()))
 .collect();



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

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

[pulkit (Pulkit Goyal)]

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
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

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

[Raphaël Gomès]

Alphare updated this revision to Diff 18566.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7524?vs=18400&id=18566

BRANCH
  default

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

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

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

CHANGE DETAILS

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,
+dirstate: &FastHashMap,
 skip_state: Option,
 ) -> 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) -> Self {
+pub fn from_manifest(manifest: &[impl AsRef]) -> 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,11 @@
 /// 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) -> Result<(), DirstateMapError> {
-for subpath in files::find_dirs(path) {
+pub fn add_path(
+&mut self,
+path: impl AsRef,
+) -> Result<(), DirstateMapError> {
+for subpath in files::find_dirs(path.as_ref()) {
 if subpath.as_bytes().last() == Some(&b'/') {
 // TODO Remove this once PathAuditor is certified
 // as the only entrypoint for path data
@@ -88,9 +91,9 @@
 /// If the directory is not in the map, something horrible has happened.
 pub fn delete_path(
 &mut self,
-path: &HgPath,
+path: impl AsRef,
 ) -> 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();
@@ -102,7 +105,7 @@
 }
 Entry::Vacant(_) => {
 return Err(DirstateMapError::PathNotFound(
-path.to_owned(),
+path.as_ref().to_owned(),
 ))
 }
 };
@@ -111,8 +114,8 @@
 Ok(())
 }
 
-pub fn contains(&self, key: &HgPath) -> bool {
-self.inner.contains_key(key)
+pub fn contains(&self, key: impl AsRef) -> bool {
+self.inner.contains_key(key.as_ref())
 }
 
 pub fn iter(&self) -> DirsMultisetIter {
@@ -130,7 +133,8 @@
 
 #[test]
 fn test_delete_path_path_not_found() {
-let mut map = DirsMultiset::from_manifest(&vec![]);
+let manifest: Vec = vec![];
+let mut map = DirsMultiset::from_manifest(&manifest);
 let path = HgPathBuf::from_bytes(b"doesnotexist/");
 assert_eq!(
 Err(DirstateMapError::PathNotFound(path.to_owned())),
@@ -186,7 +190,8 @@
 
 #[test]
 fn test_add_path_empty_path() {
-let mut map = DirsMultiset::from_manifest(&vec![]);
+let manifest: Vec = vec![];
+let mut map = DirsMultiset::from_manifest(&manifest);
 let path = HgPath::new(b"");
 map.add_path(path);
 
@@ -195,7 +200,8 @@
 
 #[test]
 fn test_add_path_successful() {
-let mut map = DirsMultiset::from_manifest(&vec![]);
+let manifest: Vec = 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());
@@ -240,7 +246,8 @@
 
 #[test]
 fn test_dirsmultiset_new_empty() {
-let new = DirsMultiset::from_manifest(&vec![]);
+let manifest: Vec = vec![];
+let new = DirsMultiset::from_manifest(&manifest);
 let expected = DirsMultiset {
 inner: FastHashMap::default(),
 };
@@ -255,7 +262,7 @@
 
 #[test]
 fn test_dirsmultiset_new_no_skip() {
-let input_vec = ["a/", "b/", "a/c", "a/d/"]
+let input_vec: Vec = ["a/", "b/", "a/c", "a/d/"]
 .iter()
 .map(|e| HgPa

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

[Raphaël Gomès]

Closed by commit rHG088ba9d94079: rust-dirs-multiset: use `AsRef` instead of 
concrete types when possible (authored by Alphare).
This revision was automatically updated to reflect the committed changes.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7524?vs=18566&id=18582

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

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

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

CHANGE DETAILS

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,
+dirstate: &FastHashMap,
 skip_state: Option,
 ) -> 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) -> Self {
+pub fn from_manifest(manifest: &[impl AsRef]) -> 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,11 @@
 /// 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) -> Result<(), DirstateMapError> {
-for subpath in files::find_dirs(path) {
+pub fn add_path(
+&mut self,
+path: impl AsRef,
+) -> Result<(), DirstateMapError> {
+for subpath in files::find_dirs(path.as_ref()) {
 if subpath.as_bytes().last() == Some(&b'/') {
 // TODO Remove this once PathAuditor is certified
 // as the only entrypoint for path data
@@ -88,9 +91,9 @@
 /// If the directory is not in the map, something horrible has happened.
 pub fn delete_path(
 &mut self,
-path: &HgPath,
+path: impl AsRef,
 ) -> 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();
@@ -102,7 +105,7 @@
 }
 Entry::Vacant(_) => {
 return Err(DirstateMapError::PathNotFound(
-path.to_owned(),
+path.as_ref().to_owned(),
 ))
 }
 };
@@ -111,8 +114,8 @@
 Ok(())
 }
 
-pub fn contains(&self, key: &HgPath) -> bool {
-self.inner.contains_key(key)
+pub fn contains(&self, key: impl AsRef) -> bool {
+self.inner.contains_key(key.as_ref())
 }
 
 pub fn iter(&self) -> DirsMultisetIter {
@@ -130,7 +133,8 @@
 
 #[test]
 fn test_delete_path_path_not_found() {
-let mut map = DirsMultiset::from_manifest(&vec![]);
+let manifest: Vec = vec![];
+let mut map = DirsMultiset::from_manifest(&manifest);
 let path = HgPathBuf::from_bytes(b"doesnotexist/");
 assert_eq!(
 Err(DirstateMapError::PathNotFound(path.to_owned())),
@@ -186,7 +190,8 @@
 
 #[test]
 fn test_add_path_empty_path() {
-let mut map = DirsMultiset::from_manifest(&vec![]);
+let manifest: Vec = vec![];
+let mut map = DirsMultiset::from_manifest(&manifest);
 let path = HgPath::new(b"");
 map.add_path(path);
 
@@ -195,7 +200,8 @@
 
 #[test]
 fn test_add_path_successful() {
-let mut map = DirsMultiset::from_manifest(&vec![]);
+let manifest: Vec = 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());
@@ -240,7 +246,8 @@
 
 #[test]
 fn test_dirsmultiset_new_empty() {
-let new = DirsMultiset::from_manifest(&vec![]);
+let manifest: Vec = vec![];
+let new = DirsMultiset::from_manifest(&manifest);
 let expected = DirsMultiset {
 inner: FastHashMap::default(),
 };
@@ -255,7 +262,7 @@
 
 #[test]
 fn test_dirsmultiset_new_no_skip() {
-let inpu