Patchwork D10746: rust: Return owned instead of borrowed DirstateEntry in DirstateMap APIs

login
register
mail settings
Submitter phabricator
Date May 19, 2021, 4:38 p.m.
Message ID <differential-rev-PHID-DREV-n32ggobtw4fcznccqrt7-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49070/
State Superseded
Headers show

Comments

phabricator - May 19, 2021, 4:38 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This will enable the tree-based DirstateMap to not always have an actual
  DirstateEntry in memory for all nodes, but construct it on demand.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  rust/hg-core/src/dirstate.rs
  rust/hg-core/src/dirstate/dirs_multiset.rs
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dirstate_map.rs
  rust/hg-core/src/dirstate_tree/dispatch.rs
  rust/hg-cpython/src/dirstate/dirs_multiset.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/dirstate/dispatch.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-cpython/src/dirstate/dispatch.rs b/rust/hg-cpython/src/dirstate/dispatch.rs
--- a/rust/hg-cpython/src/dirstate/dispatch.rs
+++ b/rust/hg-cpython/src/dirstate/dispatch.rs
@@ -173,7 +173,7 @@ 
         self.get().contains_key(key)
     }
 
-    fn get(&self, key: &HgPath) -> Option<&DirstateEntry> {
+    fn get(&self, key: &HgPath) -> Option<DirstateEntry> {
         self.get().get(key)
     }
 
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
@@ -92,7 +92,7 @@ 
         let key = key.extract::<PyBytes>(py)?;
         match self.inner(py).borrow().get(HgPath::new(key.data(py))) {
             Some(entry) => {
-                Ok(Some(make_dirstate_tuple(py, entry)?))
+                Ok(Some(make_dirstate_tuple(py, &entry)?))
             },
             None => Ok(default)
         }
@@ -348,7 +348,7 @@ 
         let key = HgPath::new(key.data(py));
         match self.inner(py).borrow().get(key) {
             Some(entry) => {
-                Ok(make_dirstate_tuple(py, entry)?)
+                Ok(make_dirstate_tuple(py, &entry)?)
             },
             None => Err(PyErr::new::<exc::KeyError, _>(
                 py,
@@ -525,13 +525,13 @@ 
     }
     fn translate_key(
         py: Python,
-        res: (&HgPath, &DirstateEntry),
+        res: (&HgPath, DirstateEntry),
     ) -> PyResult<Option<PyBytes>> {
         Ok(Some(PyBytes::new(py, res.0.as_bytes())))
     }
     fn translate_key_value(
         py: Python,
-        res: (&HgPath, &DirstateEntry),
+        res: (&HgPath, DirstateEntry),
     ) -> PyResult<Option<(PyBytes, PyObject)>> {
         let (f, entry) = res;
         Ok(Some((
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
@@ -45,7 +45,8 @@ 
         }
         let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
             let dirstate = extract_dirstate(py, &map)?;
-            DirsMultiset::from_dirstate(&dirstate, skip_state)
+            let dirstate = dirstate.iter().map(|(k, v)| (k, *v));
+            DirsMultiset::from_dirstate(dirstate, skip_state)
                 .map_err(|e: DirstateMapError| {
                     PyErr::new::<exc::ValueError, _>(py, e.to_string())
                 })?
diff --git a/rust/hg-core/src/dirstate_tree/dispatch.rs b/rust/hg-core/src/dirstate_tree/dispatch.rs
--- a/rust/hg-core/src/dirstate_tree/dispatch.rs
+++ b/rust/hg-core/src/dirstate_tree/dispatch.rs
@@ -117,7 +117,7 @@ 
 
     fn contains_key(&self, key: &HgPath) -> bool;
 
-    fn get(&self, key: &HgPath) -> Option<&DirstateEntry>;
+    fn get(&self, key: &HgPath) -> Option<DirstateEntry>;
 
     fn iter(&self) -> StateMapIter<'_>;
 }
@@ -290,11 +290,11 @@ 
         (&**self).contains_key(key)
     }
 
-    fn get(&self, key: &HgPath) -> Option<&DirstateEntry> {
-        (&**self).get(key)
+    fn get(&self, key: &HgPath) -> Option<DirstateEntry> {
+        (&**self).get(key).cloned()
     }
 
     fn iter(&self) -> StateMapIter<'_> {
-        Box::new((&**self).iter().map(|(key, value)| (&**key, value)))
+        Box::new((&**self).iter().map(|(key, value)| (&**key, *value)))
     }
 }
diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -627,13 +627,13 @@ 
         self.get(key).is_some()
     }
 
-    fn get(&self, key: &HgPath) -> Option<&DirstateEntry> {
-        self.get_node(key)?.entry.as_ref()
+    fn get(&self, key: &HgPath) -> Option<DirstateEntry> {
+        self.get_node(key)?.entry
     }
 
     fn iter(&self) -> StateMapIter<'_> {
         Box::new(self.iter_nodes().filter_map(|(path, node)| {
-            node.entry.as_ref().map(|entry| (&**path, entry))
+            node.entry.map(|entry| (&**path, entry))
         }))
     }
 }
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
@@ -249,7 +249,7 @@ 
     pub fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> {
         if self.all_dirs.is_none() {
             self.all_dirs = Some(DirsMultiset::from_dirstate(
-                self.state_map.iter(),
+                self.state_map.iter().map(|(k, v)| (k, *v)),
                 None,
             )?);
         }
@@ -259,7 +259,7 @@ 
     pub fn set_dirs(&mut self) -> Result<(), DirstateMapError> {
         if self.dirs.is_none() {
             self.dirs = Some(DirsMultiset::from_dirstate(
-                self.state_map.iter(),
+                self.state_map.iter().map(|(k, v)| (k, *v)),
                 Some(EntryState::Removed),
             )?);
         }
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
@@ -30,12 +30,12 @@ 
     /// Initializes the multiset from a dirstate.
     ///
     /// If `skip_state` is provided, skips dirstate entries with equal state.
-    pub fn from_dirstate<'a, I, P>(
+    pub fn from_dirstate<I, P>(
         dirstate: I,
         skip_state: Option<EntryState>,
     ) -> Result<Self, DirstateMapError>
     where
-        I: IntoIterator<Item = (P, &'a DirstateEntry)>,
+        I: IntoIterator<Item = (P, DirstateEntry)>,
         P: AsRef<HgPath>,
     {
         let mut multiset = DirsMultiset {
@@ -338,7 +338,7 @@ 
         assert_eq!(expected, new);
 
         let new =
-            DirsMultiset::from_dirstate(&StateMap::default(), None).unwrap();
+            DirsMultiset::from_dirstate(StateMap::default(), None).unwrap();
         let expected = DirsMultiset {
             inner: FastHashMap::default(),
         };
@@ -381,7 +381,7 @@ 
             .map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
             .collect();
 
-        let new = DirsMultiset::from_dirstate(&input_map, None).unwrap();
+        let new = DirsMultiset::from_dirstate(input_map, None).unwrap();
         let expected = DirsMultiset {
             inner: expected_inner,
         };
@@ -417,7 +417,7 @@ 
             .collect();
 
         let new =
-            DirsMultiset::from_dirstate(&input_map, Some(EntryState::Normal))
+            DirsMultiset::from_dirstate(input_map, Some(EntryState::Normal))
                 .unwrap();
         let expected = DirsMultiset {
             inner: expected_inner,
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
@@ -77,7 +77,7 @@ 
 
 pub type StateMap = FastHashMap<HgPathBuf, DirstateEntry>;
 pub type StateMapIter<'a> =
-    Box<dyn Iterator<Item = (&'a HgPath, &'a DirstateEntry)> + Send + 'a>;
+    Box<dyn Iterator<Item = (&'a HgPath, DirstateEntry)> + Send + 'a>;
 
 pub type CopyMap = FastHashMap<HgPathBuf, HgPathBuf>;
 pub type CopyMapIter<'a> =