This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7524

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

Authored by Alphare on Nov 29 2019, 12:56 PM.

Details

Reviewers
kevincox
pulkit
Group Reviewers
hg-reviewers
Commits
rHG088ba9d94079: rust-dirs-multiset: use `AsRef` instead of concrete types when possible
Summary

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

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare created this revision.Nov 29 2019, 12:56 PM
kevincox accepted this revision.Dec 2 2019, 8:06 AM
pulkit accepted this revision.Dec 10 2019, 10:11 AM
This revision is now accepted and ready to land.Dec 10 2019, 10:11 AM
pulkit added a comment.Dec 10 2019, 10:16 AM

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

Alphare updated this revision to Diff 18566.Dec 10 2019, 10:26 AM

Revision Contents
Changeset List

Diff 18582

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

inner: FastHashMap<HgPathBuf, u32>, inner: FastHashMap<HgPathBuf, u32>,
} }
impl DirsMultiset { impl DirsMultiset {
/// Initializes the multiset from a dirstate. /// Initializes the multiset from a dirstate.
/// ///
/// If `skip_state` is provided, skips dirstate entries with equal state. /// If `skip_state` is provided, skips dirstate entries with equal state.
pub fn from_dirstate( pub fn from_dirstate(
vec: &FastHashMap<HgPathBuf, DirstateEntry>, dirstate: &FastHashMap<HgPathBuf, DirstateEntry>,
skip_state: Option<EntryState>, skip_state: Option<EntryState>,
) -> Self { ) -> Self {
let mut multiset = DirsMultiset { let mut multiset = DirsMultiset {
inner: FastHashMap::default(), inner: FastHashMap::default(),
}; };
for (filename, DirstateEntry { state, .. }) in vec { for (filename, DirstateEntry { state, .. }) in dirstate {
// This `if` is optimized out of the loop // This `if` is optimized out of the loop
if let Some(skip) = skip_state { if let Some(skip) = skip_state {
if skip != *state { if skip != *state {
multiset.add_path(filename); multiset.add_path(filename);
} }
} else { } else {
multiset.add_path(filename); multiset.add_path(filename);
} }
} }
multiset multiset
} }
/// Initializes the multiset from a manifest. /// 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 { let mut multiset = DirsMultiset {
inner: FastHashMap::default(), inner: FastHashMap::default(),
}; };
for filename in vec { for filename in manifest {
multiset.add_path(filename); multiset.add_path(filename.as_ref());
} }
multiset multiset
} }
/// Increases the count of deepest directory contained in the path. /// Increases the count of deepest directory contained in the path.
/// ///
/// If the directory is not yet in the map, adds its parents. /// If the directory is not yet in the map, adds its parents.
pub fn add_path(&mut self, path: &HgPath) -> Result<(), DirstateMapError> { pub fn add_path(
for subpath in files::find_dirs(path) { &mut self,
path: impl AsRef<HgPath>,
) -> Result<(), DirstateMapError> {
for subpath in files::find_dirs(path.as_ref()) {
if subpath.as_bytes().last() == Some(&b'/') { if subpath.as_bytes().last() == Some(&b'/') {
// TODO Remove this once PathAuditor is certified // TODO Remove this once PathAuditor is certified
// as the only entrypoint for path data // as the only entrypoint for path data
return Err(DirstateMapError::ConsecutiveSlashes); return Err(DirstateMapError::ConsecutiveSlashes);
} }
if let Some(val) = self.inner.get_mut(subpath) { if let Some(val) = self.inner.get_mut(subpath) {
*val += 1; *val += 1;
break; break;
} }
self.inner.insert(subpath.to_owned(), 1); self.inner.insert(subpath.to_owned(), 1);
} }
Ok(()) Ok(())
} }
/// Decreases the count of deepest directory contained in the path. /// Decreases the count of deepest directory contained in the path.
/// ///
/// If it is the only reference, decreases all parents until one is /// If it is the only reference, decreases all parents until one is
/// removed. /// removed.
/// If the directory is not in the map, something horrible has happened. /// If the directory is not in the map, something horrible has happened.
pub fn delete_path( pub fn delete_path(
&mut self, &mut self,
path: &HgPath, path: impl AsRef<HgPath>,
) -> Result<(), DirstateMapError> { ) -> 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()) { match self.inner.entry(subpath.to_owned()) {
Entry::Occupied(mut entry) => { Entry::Occupied(mut entry) => {
let val = entry.get().clone(); let val = entry.get().clone();
if val > 1 { if val > 1 {
entry.insert(val - 1); entry.insert(val - 1);
break; break;
} }
entry.remove(); entry.remove();
} }
Entry::Vacant(_) => { Entry::Vacant(_) => {
return Err(DirstateMapError::PathNotFound( return Err(DirstateMapError::PathNotFound(
path.to_owned(), path.as_ref().to_owned(),
)) ))
} }
}; };
} }
Ok(()) Ok(())
} }
pub fn contains(&self, key: &HgPath) -> bool { pub fn contains(&self, key: impl AsRef<HgPath>) -> bool {
self.inner.contains_key(key) self.inner.contains_key(key.as_ref())
} }
pub fn iter(&self) -> DirsMultisetIter { pub fn iter(&self) -> DirsMultisetIter {
self.inner.keys() self.inner.keys()
} }
pub fn len(&self) -> usize { pub fn len(&self) -> usize {
self.inner.len() self.inner.len()
} }
} }
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
#[test] #[test]
fn test_delete_path_path_not_found() { 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/"); let path = HgPathBuf::from_bytes(b"doesnotexist/");
assert_eq!( assert_eq!(
Err(DirstateMapError::PathNotFound(path.to_owned())), Err(DirstateMapError::PathNotFound(path.to_owned())),
map.delete_path(&path) map.delete_path(&path)
); );
} }
#[test] #[test]
b"a/c/" b"a/c/"
))), ))),
map.delete_path(HgPath::new(b"a/c/")) map.delete_path(HgPath::new(b"a/c/"))
); );
} }
#[test] #[test]
fn test_add_path_empty_path() { 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""); let path = HgPath::new(b"");
map.add_path(path); map.add_path(path);
assert_eq!(1, map.len()); assert_eq!(1, map.len());
} }
#[test] #[test]
fn test_add_path_successful() { 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/")); map.add_path(HgPath::new(b"a/"));
assert_eq!(1, *map.inner.get(HgPath::new(b"a")).unwrap()); assert_eq!(1, *map.inner.get(HgPath::new(b"a")).unwrap());
assert_eq!(1, *map.inner.get(HgPath::new(b"")).unwrap()); assert_eq!(1, *map.inner.get(HgPath::new(b"")).unwrap());
assert_eq!(2, map.len()); assert_eq!(2, map.len());
// Non directory should be ignored // Non directory should be ignored
map.add_path(HgPath::new(b"a")); map.add_path(HgPath::new(b"a"));
.map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v)) .map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
.collect(), .collect(),
}; };
assert_eq!(map, expected); assert_eq!(map, expected);
} }
#[test] #[test]
fn test_dirsmultiset_new_empty() { 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 { let expected = DirsMultiset {
inner: FastHashMap::default(), inner: FastHashMap::default(),
}; };
assert_eq!(expected, new); assert_eq!(expected, new);
let new = DirsMultiset::from_dirstate(&FastHashMap::default(), None); let new = DirsMultiset::from_dirstate(&FastHashMap::default(), None);
let expected = DirsMultiset { let expected = DirsMultiset {
inner: FastHashMap::default(), inner: FastHashMap::default(),
}; };
assert_eq!(expected, new); assert_eq!(expected, new);
} }
#[test] #[test]
fn test_dirsmultiset_new_no_skip() { 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() .iter()
.map(|e| HgPathBuf::from_bytes(e.as_bytes())) .map(|e| HgPathBuf::from_bytes(e.as_bytes()))
.collect(); .collect();
let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)] let expected_inner = [("", 2), ("a", 3), ("b", 1), ("a/d", 1)]
.iter() .iter()
.map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v)) .map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
.collect(); .collect();