Patchwork D10363: dirstate-tree: Abstract "non-normal" and "other parent" sets

login
register
mail settings
Submitter phabricator
Date April 12, 2021, 12:24 p.m.
Message ID <differential-rev-PHID-DREV-3spb67wgbe7o7znqgect-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48680/
State Superseded
Headers show

Comments

phabricator - April 12, 2021, 12:24 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Instead of exposing `HashSet`s directly, have slightly higher-level
  methods for the operations that Python bindings need on them.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/dirstate.py
  rust/hg-core/src/dirstate_tree/dispatch.rs
  rust/hg-cpython/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/dirstate/non_normal_entries.rs

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-cpython/src/dirstate/non_normal_entries.rs b/rust/hg-cpython/src/dirstate/non_normal_entries.rs
--- a/rust/hg-cpython/src/dirstate/non_normal_entries.rs
+++ b/rust/hg-cpython/src/dirstate/non_normal_entries.rs
@@ -7,14 +7,13 @@ 
 
 use cpython::{
     exc::NotImplementedError, CompareOp, ObjectProtocol, PyBytes, PyClone,
-    PyErr, PyList, PyObject, PyResult, PyString, Python, PythonObject,
-    ToPyObject, UnsafePyLeaked,
+    PyErr, PyObject, PyResult, PyString, Python, PythonObject, ToPyObject,
+    UnsafePyLeaked,
 };
 
 use crate::dirstate::DirstateMap;
 use hg::utils::hg_path::HgPathBuf;
 use std::cell::RefCell;
-use std::collections::hash_set;
 
 py_class!(pub class NonNormalEntries |py| {
     data dmap: DirstateMap;
@@ -25,9 +24,6 @@ 
     def remove(&self, key: PyObject) -> PyResult<PyObject> {
         self.dmap(py).non_normal_entries_remove(py, key)
     }
-    def union(&self, other: PyObject) -> PyResult<PyList> {
-        self.dmap(py).non_normal_entries_union(py, other)
-    }
     def __richcmp__(&self, other: PyObject, op: CompareOp) -> PyResult<bool> {
         match op {
             CompareOp::Eq => self.is_equal_to(py, other),
@@ -66,7 +62,8 @@ 
     }
 }
 
-type NonNormalEntriesIter<'a> = hash_set::Iter<'a, HgPathBuf>;
+type NonNormalEntriesIter<'a> =
+    Box<dyn Iterator<Item = &'a HgPathBuf> + Send + 'a>;
 
 py_shared_iterator!(
     NonNormalEntriesIterator,
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
@@ -173,11 +173,8 @@ 
 
     def other_parent_entries(&self) -> PyResult<PyObject> {
         let mut inner_shared = self.inner(py).borrow_mut();
-        let (_, other_parent) =
-            inner_shared.get_non_normal_other_parent_entries();
-
         let set = PySet::empty(py)?;
-        for path in other_parent.iter() {
+        for path in inner_shared.iter_other_parent_paths() {
             set.add(py, PyBytes::new(py, path.as_bytes()))?;
         }
         Ok(set.into_object())
@@ -192,8 +189,7 @@ 
         Ok(self
             .inner(py)
             .borrow_mut()
-            .get_non_normal_other_parent_entries().0
-            .contains(HgPath::new(key.data(py))))
+            .non_normal_entries_contains(HgPath::new(key.data(py))))
     }
 
     def non_normal_entries_display(&self) -> PyResult<PyString> {
@@ -201,14 +197,17 @@ 
             PyString::new(
                 py,
                 &format!(
-                    "NonNormalEntries: {:?}",
-                    self
-                        .inner(py)
-                        .borrow_mut()
-                        .get_non_normal_other_parent_entries().0
-                        .iter().map(|o| o))
+                    "NonNormalEntries: {}",
+                    hg::utils::join_display(
+                        self
+                            .inner(py)
+                            .borrow_mut()
+                            .iter_non_normal_paths(),
+                        ", "
+                    )
                 )
             )
+        )
     }
 
     def non_normal_entries_remove(&self, key: PyObject) -> PyResult<PyObject> {
@@ -220,22 +219,11 @@ 
         Ok(py.None())
     }
 
-    def non_normal_entries_union(&self, other: PyObject) -> PyResult<PyList> {
-        let other: PyResult<_> = other.iter(py)?
-                    .map(|f| {
-                        Ok(HgPathBuf::from_bytes(
-                            f?.extract::<PyBytes>(py)?.data(py),
-                        ))
-                    })
-                    .collect();
-
-        let res = self
-            .inner(py)
-            .borrow_mut()
-            .non_normal_entries_union(other?);
+    def non_normal_or_other_parent_paths(&self) -> PyResult<PyList> {
+        let mut inner = self.inner(py).borrow_mut();
 
         let ret = PyList::new(py, &[]);
-        for filename in res.iter() {
+        for filename in inner.non_normal_or_other_parent_paths() {
             let as_pystring = PyBytes::new(py, filename.as_bytes());
             ret.append(py, as_pystring.into_object());
         }
@@ -253,7 +241,7 @@ 
 
         NonNormalEntriesIterator::from_inner(py, unsafe {
             leaked_ref.map(py, |o| {
-                o.get_non_normal_other_parent_entries_panic().0.iter()
+                o.iter_non_normal_paths_panic()
             })
         })
     }
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
@@ -1,4 +1,3 @@ 
-use std::collections::HashSet;
 use std::path::PathBuf;
 use std::time::Duration;
 
@@ -44,22 +43,27 @@ 
 
     fn clear_ambiguous_times(&mut self, filenames: Vec<HgPathBuf>, now: i32);
 
+    fn non_normal_entries_contains(&mut self, key: &HgPath) -> bool;
+
     fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool;
 
-    fn non_normal_entries_union(
+    fn non_normal_or_other_parent_paths(
         &mut self,
-        other: HashSet<HgPathBuf>,
-    ) -> Vec<HgPathBuf>;
+    ) -> Box<dyn Iterator<Item = &HgPathBuf> + '_>;
 
     fn set_non_normal_other_parent_entries(&mut self, force: bool);
 
-    fn get_non_normal_other_parent_entries_panic(
-        &self,
-    ) -> (&HashSet<HgPathBuf>, &HashSet<HgPathBuf>);
+    fn iter_non_normal_paths(
+        &mut self,
+    ) -> Box<dyn Iterator<Item = &HgPathBuf> + Send + '_>;
 
-    fn get_non_normal_other_parent_entries(
+    fn iter_non_normal_paths_panic(
+        &self,
+    ) -> Box<dyn Iterator<Item = &HgPathBuf> + Send + '_>;
+
+    fn iter_other_parent_paths(
         &mut self,
-    ) -> (&mut HashSet<HgPathBuf>, &mut HashSet<HgPathBuf>);
+    ) -> Box<dyn Iterator<Item = &HgPathBuf> + Send + '_>;
 
     fn has_tracked_dir(
         &mut self,
@@ -169,31 +173,50 @@ 
         self.clear_ambiguous_times(filenames, now)
     }
 
+    fn non_normal_entries_contains(&mut self, key: &HgPath) -> bool {
+        let (non_normal, _other_parent) =
+            self.get_non_normal_other_parent_entries();
+        non_normal.contains(key)
+    }
+
     fn non_normal_entries_remove(&mut self, key: &HgPath) -> bool {
         self.non_normal_entries_remove(key)
     }
 
-    fn non_normal_entries_union(
+    fn non_normal_or_other_parent_paths(
         &mut self,
-        other: HashSet<HgPathBuf>,
-    ) -> Vec<HgPathBuf> {
-        self.non_normal_entries_union(other)
+    ) -> Box<dyn Iterator<Item = &HgPathBuf> + '_> {
+        let (non_normal, other_parent) =
+            self.get_non_normal_other_parent_entries();
+        Box::new(non_normal.union(other_parent))
     }
 
     fn set_non_normal_other_parent_entries(&mut self, force: bool) {
         self.set_non_normal_other_parent_entries(force)
     }
 
-    fn get_non_normal_other_parent_entries_panic(
-        &self,
-    ) -> (&HashSet<HgPathBuf>, &HashSet<HgPathBuf>) {
-        self.get_non_normal_other_parent_entries_panic()
+    fn iter_non_normal_paths(
+        &mut self,
+    ) -> Box<dyn Iterator<Item = &HgPathBuf> + Send + '_> {
+        let (non_normal, _other_parent) =
+            self.get_non_normal_other_parent_entries();
+        Box::new(non_normal.iter())
     }
 
-    fn get_non_normal_other_parent_entries(
+    fn iter_non_normal_paths_panic(
+        &self,
+    ) -> Box<dyn Iterator<Item = &HgPathBuf> + Send + '_> {
+        let (non_normal, _other_parent) =
+            self.get_non_normal_other_parent_entries_panic();
+        Box::new(non_normal.iter())
+    }
+
+    fn iter_other_parent_paths(
         &mut self,
-    ) -> (&mut HashSet<HgPathBuf>, &mut HashSet<HgPathBuf>) {
-        self.get_non_normal_other_parent_entries()
+    ) -> Box<dyn Iterator<Item = &HgPathBuf> + Send + '_> {
+        let (_non_normal, other_parent) =
+            self.get_non_normal_other_parent_entries();
+        Box::new(other_parent.iter())
     }
 
     fn has_tracked_dir(
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -336,9 +336,7 @@ 
         self._map.setparents(p1, p2)
         copies = {}
         if oldp2 != nullid and p2 == nullid:
-            candidatefiles = self._map.nonnormalset.union(
-                self._map.otherparentset
-            )
+            candidatefiles = self._map.non_normal_or_other_parent_paths()
             for f in candidatefiles:
                 s = self._map.get(f)
                 if s is None:
@@ -1718,6 +1716,9 @@ 
         self.nonnormalset = nonnorm
         return otherparents
 
+    def non_normal_or_other_parent_paths(self):
+        return self.nonnormalset.union(self.otherparentset)
+
     @propertycache
     def identity(self):
         self._map
@@ -1930,6 +1931,9 @@ 
             otherparents = self._rustmap.other_parent_entries()
             return otherparents
 
+        def non_normal_or_other_parent_paths(self):
+            return self._rustmap.non_normal_or_other_parent_paths()
+
         @propertycache
         def dirfoldmap(self):
             f = {}