Patchwork D6690: rust-dirstate: improve API of `DirsMultiset`

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

Comments

phabricator - July 24, 2019, 3:24 p.m.
Alphare created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  - Use opaque `Iterator` type instead of implementation-specific one from `HashMap`
  - Make `DirsMultiset` behave like a set both in Rust and from Python

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS




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

Patch

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
@@ -12,7 +12,7 @@ 
 
 use cpython::{
     exc, ObjectProtocol, PyBytes, PyDict, PyErr, PyObject, PyResult,
-    ToPyObject,
+    PythonObject, ToPyObject,
 };
 
 use crate::dirstate::extract_dirstate;
@@ -93,26 +93,21 @@ 
     // of having it work to continue working on the rest of the module
     // hopefully bypassing Python entirely pretty soon.
     def __iter__(&self) -> PyResult<PyObject> {
-        let dict = PyDict::new(py);
-
-        for (key, value) in self.dirs_map(py).borrow().iter() {
-            dict.set_item(
-                py,
-                PyBytes::new(py, &key[..]),
-                value.to_py_object(py),
-            )?;
-        }
-
-        let locals = PyDict::new(py);
-        locals.set_item(py, "obj", dict)?;
-
-        py.eval("iter(obj)", None, Some(&locals))
+        let dirs = self.dirs_map(py).borrow();
+        let dirs: Vec<_> = dirs
+            .iter()
+            .map(|d| PyBytes::new(py, d))
+            .collect();
+        dirs.to_py_object(py)
+            .into_object()
+            .iter(py)
+            .map(|o| o.into_object())
     }
 
     def __contains__(&self, item: PyObject) -> PyResult<bool> {
         Ok(self
             .dirs_map(py)
             .borrow()
-            .contains_key(item.extract::<PyBytes>(py)?.data(py).as_ref()))
+            .contains(item.extract::<PyBytes>(py)?.data(py).as_ref()))
     }
 });
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
@@ -12,7 +12,7 @@ 
     dirstate::EntryState, utils::files, DirsIterable, DirstateEntry,
     DirstateMapError,
 };
-use std::collections::hash_map::{Entry, Iter};
+use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 
 #[derive(PartialEq, Debug)]
@@ -98,12 +98,12 @@ 
         Ok(())
     }
 
-    pub fn contains_key(&self, key: &[u8]) -> bool {
+    pub fn contains(&self, key: &[u8]) -> bool {
         self.inner.contains_key(key)
     }
 
-    pub fn iter(&self) -> Iter<Vec<u8>, u32> {
-        self.inner.iter()
+    pub fn iter(&self) -> impl Iterator<Item = &Vec<u8>> {
+        self.inner.keys()
     }
 
     pub fn len(&self) -> usize {