From patchwork Sat Aug 17 12:12:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [10, of, 11] rust-dirstate: split DirsMultiset constructor per input type From: Yuya Nishihara X-Patchwork-Id: 41332 Message-Id: To: mercurial-devel@mercurial-scm.org Date: Sat, 17 Aug 2019 21:12:11 +0900 # HG changeset patch # User Yuya Nishihara # Date 1566033929 -32400 # Sat Aug 17 18:25:29 2019 +0900 # Node ID c0660c41e5b27173f73b8c88da1728b69425aeba # Parent d4c0130723d92bd545e7df45aceead8bd7562156 rust-dirstate: split DirsMultiset constructor per input type Since skip_state only applies to dirstate, it doesn't make sense to unify these constructors and dispatch by enum. diff --git a/rust/hg-core/src/dirstate.rs b/rust/hg-core/src/dirstate.rs --- a/rust/hg-core/src/dirstate.rs +++ b/rust/hg-core/src/dirstate.rs @@ -33,13 +33,6 @@ pub struct DirstateEntry { pub type StateMap = HashMap, DirstateEntry>; pub type CopyMap = HashMap, Vec>; -/// The Python implementation passes either a mapping (dirstate) or a flat -/// iterable (manifest) -pub enum DirsIterable<'a> { - Dirstate(&'a HashMap, DirstateEntry>), - Manifest(&'a Vec>), -} - #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub enum EntryState { Normal, 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 @@ -9,8 +9,7 @@ //! //! Used to counts the references to directories in a manifest or dirstate. use crate::{ - dirstate::EntryState, utils::files, DirsIterable, DirstateEntry, - DirstateMapError, + dirstate::EntryState, utils::files, DirstateEntry, DirstateMapError, }; use std::collections::hash_map::Entry; use std::collections::HashMap; @@ -21,40 +20,44 @@ pub struct DirsMultiset { } impl DirsMultiset { - /// Initializes the multiset from a dirstate or a manifest. + /// Initializes the multiset from a dirstate. /// /// If `skip_state` is provided, skips dirstate entries with equal state. - pub fn new( - iterable: DirsIterable, + pub fn from_dirstate( + vec: &HashMap, DirstateEntry>, skip_state: Option, ) -> Self { let mut multiset = DirsMultiset { inner: HashMap::new(), }; - match iterable { - DirsIterable::Dirstate(vec) => { - for (filename, DirstateEntry { state, .. }) in vec { - // This `if` is optimized out of the loop - if let Some(skip) = skip_state { - if skip != *state { - multiset.add_path(filename); - } - } else { - multiset.add_path(filename); - } - } - } - DirsIterable::Manifest(vec) => { - for filename in vec { + for (filename, DirstateEntry { state, .. }) in vec { + // This `if` is optimized out of the loop + if let Some(skip) = skip_state { + if skip != *state { multiset.add_path(filename); } + } else { + multiset.add_path(filename); } } multiset } + /// Initializes the multiset from a manifest. + pub fn from_manifest(vec: &Vec>) -> Self { + let mut multiset = DirsMultiset { + inner: HashMap::new(), + }; + + for filename in vec { + multiset.add_path(filename); + } + + multiset + } + /// Increases the count of deepest directory contained in the path. /// /// If the directory is not yet in the map, adds its parents. @@ -118,7 +121,7 @@ mod tests { #[test] fn test_delete_path_path_not_found() { - let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None); + let mut map = DirsMultiset::from_manifest(&vec![]); let path = b"doesnotexist/"; assert_eq!( Err(DirstateMapError::PathNotFound(path.to_vec())), @@ -128,8 +131,7 @@ mod tests { #[test] fn test_delete_path_empty_path() { - let mut map = - DirsMultiset::new(DirsIterable::Manifest(&vec![vec![]]), None); + let mut map = DirsMultiset::from_manifest(&vec![vec![]]); let path = b""; assert_eq!(Ok(()), map.delete_path(path)); assert_eq!( @@ -169,7 +171,7 @@ mod tests { #[test] fn test_add_path_empty_path() { - let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None); + let mut map = DirsMultiset::from_manifest(&vec![]); let path = b""; map.add_path(path); @@ -178,7 +180,7 @@ mod tests { #[test] fn test_add_path_successful() { - let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None); + let mut map = DirsMultiset::from_manifest(&vec![]); map.add_path(b"a/"); assert_eq!(1, *map.inner.get(&b"a".to_vec()).unwrap()); @@ -223,15 +225,13 @@ mod tests { #[test] fn test_dirsmultiset_new_empty() { - use DirsIterable::{Dirstate, Manifest}; - - let new = DirsMultiset::new(Manifest(&vec![]), None); + let new = DirsMultiset::from_manifest(&vec![]); let expected = DirsMultiset { inner: HashMap::new(), }; assert_eq!(expected, new); - let new = DirsMultiset::new(Dirstate(&HashMap::new()), None); + let new = DirsMultiset::from_dirstate(&HashMap::new(), None); let expected = DirsMultiset { inner: HashMap::new(), }; @@ -240,8 +240,6 @@ mod tests { #[test] fn test_dirsmultiset_new_no_skip() { - use DirsIterable::{Dirstate, Manifest}; - let input_vec = ["a/", "b/", "a/c", "a/d/"] .iter() .map(|e| e.as_bytes().to_vec()) @@ -251,7 +249,7 @@ mod tests { .map(|(k, v)| (k.as_bytes().to_vec(), *v)) .collect(); - let new = DirsMultiset::new(Manifest(&input_vec), None); + let new = DirsMultiset::from_manifest(&input_vec); let expected = DirsMultiset { inner: expected_inner, }; @@ -276,7 +274,7 @@ mod tests { .map(|(k, v)| (k.as_bytes().to_vec(), *v)) .collect(); - let new = DirsMultiset::new(Dirstate(&input_map), None); + let new = DirsMultiset::from_dirstate(&input_map, None); let expected = DirsMultiset { inner: expected_inner, }; @@ -285,8 +283,6 @@ mod tests { #[test] fn test_dirsmultiset_new_skip() { - use DirsIterable::{Dirstate, Manifest}; - let input_vec = ["a/", "b/", "a/c", "a/d/"] .iter() .map(|e| e.as_bytes().to_vec()) @@ -296,8 +292,9 @@ mod tests { .map(|(k, v)| (k.as_bytes().to_vec(), *v)) .collect(); - let new = - DirsMultiset::new(Manifest(&input_vec), Some(EntryState::Normal)); + // this was + // DirsMultiset::new(Manifest(&input_vec), Some(EntryState::Normal)) + let new = DirsMultiset::from_manifest(&input_vec); let expected = DirsMultiset { inner: expected_inner, }; @@ -331,7 +328,7 @@ mod tests { .collect(); let new = - DirsMultiset::new(Dirstate(&input_map), Some(EntryState::Normal)); + DirsMultiset::from_dirstate(&input_map, Some(EntryState::Normal)); let expected = DirsMultiset { inner: expected_inner, }; diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs --- a/rust/hg-core/src/dirstate/dirstate_map.rs +++ b/rust/hg-core/src/dirstate/dirstate_map.rs @@ -7,9 +7,9 @@ use crate::{ dirstate::{parsers::PARENT_SIZE, EntryState}, - pack_dirstate, parse_dirstate, CopyMap, DirsIterable, DirsMultiset, - DirstateEntry, DirstateError, DirstateMapError, DirstateParents, - DirstateParseError, StateMap, + pack_dirstate, parse_dirstate, CopyMap, DirsMultiset, DirstateEntry, + DirstateError, DirstateMapError, DirstateParents, DirstateParseError, + StateMap, }; use core::borrow::Borrow; use std::collections::{HashMap, HashSet}; @@ -224,17 +224,15 @@ impl DirstateMap { /// good idea. pub fn set_all_dirs(&mut self) { if self.all_dirs.is_none() { - self.all_dirs = Some(DirsMultiset::new( - DirsIterable::Dirstate(&self.state_map), - None, - )); + self.all_dirs = + Some(DirsMultiset::from_dirstate(&self.state_map, None)); } } pub fn set_dirs(&mut self) { if self.dirs.is_none() { - self.dirs = Some(DirsMultiset::new( - DirsIterable::Dirstate(&self.state_map), + self.dirs = Some(DirsMultiset::from_dirstate( + &self.state_map, Some(EntryState::Removed), )); } diff --git a/rust/hg-core/src/lib.rs b/rust/hg-core/src/lib.rs --- a/rust/hg-core/src/lib.rs +++ b/rust/hg-core/src/lib.rs @@ -12,8 +12,7 @@ pub use dirstate::{ dirs_multiset::DirsMultiset, dirstate_map::DirstateMap, parsers::{pack_dirstate, parse_dirstate, PARENT_SIZE}, - CopyMap, DirsIterable, DirstateEntry, DirstateParents, EntryState, - StateMap, + CopyMap, DirstateEntry, DirstateParents, EntryState, StateMap, }; mod filepatterns; pub mod utils; diff --git a/rust/hg-cpython/src/dirstate/dirs_multiset.rs b/rust/hg-cpython/src/dirstate/dirs_multiset.rs --- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs +++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs @@ -17,10 +17,7 @@ use cpython::{ }; use crate::{dirstate::extract_dirstate, ref_sharing::PySharedState}; -use hg::{ - DirsIterable, DirsMultiset, DirstateMapError, DirstateParseError, - EntryState, -}; +use hg::{DirsMultiset, DirstateMapError, DirstateParseError, EntryState}; py_class!(pub class Dirs |py| { data inner: RefCell; @@ -45,19 +42,13 @@ py_class!(pub class Dirs |py| { } let inner = if let Ok(map) = map.cast_as::(py) { let dirstate = extract_dirstate(py, &map)?; - DirsMultiset::new( - DirsIterable::Dirstate(&dirstate), - skip_state, - ) + DirsMultiset::from_dirstate(&dirstate, skip_state) } else { let map: Result>, PyErr> = map .iter(py)? .map(|o| Ok(o?.extract::(py)?.data(py).to_owned())) .collect(); - DirsMultiset::new( - DirsIterable::Manifest(&map?), - skip_state, - ) + DirsMultiset::from_manifest(&map?) }; Self::create_instance( diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs --- a/rust/hg-cpython/src/dirstate/dirstate_map.rs +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs @@ -24,7 +24,7 @@ use crate::{ ref_sharing::PySharedState, }; use hg::{ - DirsIterable, DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap, + DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap, DirstateParents, DirstateParseError, EntryState, PARENT_SIZE, }; @@ -356,8 +356,8 @@ py_class!(pub class DirstateMap |py| { self.inner(py).borrow_mut().set_dirs(); Dirs::from_inner( py, - DirsMultiset::new( - DirsIterable::Dirstate(&self.inner(py).borrow()), + DirsMultiset::from_dirstate( + &self.inner(py).borrow(), Some(EntryState::Removed), ), ) @@ -367,8 +367,8 @@ py_class!(pub class DirstateMap |py| { self.inner(py).borrow_mut().set_all_dirs(); Dirs::from_inner( py, - DirsMultiset::new( - DirsIterable::Dirstate(&self.inner(py).borrow()), + DirsMultiset::from_dirstate( + &self.inner(py).borrow(), None, ), ) diff --git a/rust/hg-cpython/src/parsers.rs b/rust/hg-cpython/src/parsers.rs --- a/rust/hg-cpython/src/parsers.rs +++ b/rust/hg-cpython/src/parsers.rs @@ -15,8 +15,8 @@ use cpython::{ PythonObject, ToPyObject, }; use hg::{ - pack_dirstate, parse_dirstate, DirstateEntry, - DirstatePackError, DirstateParents, DirstateParseError, PARENT_SIZE, + pack_dirstate, parse_dirstate, DirstateEntry, DirstatePackError, + DirstateParents, DirstateParseError, PARENT_SIZE, }; use std::collections::HashMap; use std::convert::TryInto;