Patchwork D6593: rust-minor-fixes: remove Deref in favor of explicit methods

login
register
mail settings
Submitter Yuya Nishihara
Date July 5, 2019, 2:21 p.m.
Message ID <20190705232124.81cfe6f90b1851c2c26b4921@tcha.org>
Download mbox | patch
Permalink /patch/40792/
State New
Headers show

Comments

Yuya Nishihara - July 5, 2019, 2:21 p.m.
>   Using `keys()` will change the behavior, as I actually need to expose an `Iter<Vec<u8>, u32>`. Is your issue with the naming?

No. My point is that the refcount value is an implementation detail.
Do you have some plan to use the refcount value?

```

```
phabricator - July 5, 2019, 2:23 p.m.
yuja added a comment.


  >   Using `keys()` will change the behavior, as I actually need to expose an `Iter<Vec<u8>, u32>`. Is your issue with the naming?
  
  No. My point is that the refcount value is an implementation detail.
  Do you have some plan to use the refcount value?
  
    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
    @@ -8,7 +8,7 @@
     //! A multiset of directory names.
     //!
     //! Used to counts the references to directories in a manifest or dirstate.
    -use std::collections::hash_map::{Entry, Iter};
    +use std::collections::hash_map::{Entry, Keys};
     use std::collections::HashMap;
     use {DirsIterable, DirstateEntry, DirstateMapError};
     
    @@ -128,8 +128,8 @@ impl DirsMultiset {
             self.inner.contains_key(key)
         }
     
    -    pub fn iter(&self) -> Iter<Vec<u8>, u32> {
    -        self.inner.iter()
    +    pub fn iter(&self) -> Keys<Vec<u8>, u32> {
    +        self.inner.keys()
         }
     
         pub fn len(&self) -> usize {
    diff --git a/rust/hg-cpython/src/dirstate.rs b/rust/hg-cpython/src/dirstate.rs
    --- a/rust/hg-cpython/src/dirstate.rs
    +++ b/rust/hg-cpython/src/dirstate.rs
    @@ -271,20 +271,9 @@ py_class!(pub class Dirs |py| {
         // 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))
    +        // maybe there would be a better way to cast objects back and forth?
    +        let dirs: Vec<_> = self.dirs_map(py).borrow().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> {

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6593/new/

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

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

Patch

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
@@ -8,7 +8,7 @@ 
 //! A multiset of directory names.
 //!
 //! Used to counts the references to directories in a manifest or dirstate.
-use std::collections::hash_map::{Entry, Iter};
+use std::collections::hash_map::{Entry, Keys};
 use std::collections::HashMap;
 use {DirsIterable, DirstateEntry, DirstateMapError};
 
@@ -128,8 +128,8 @@  impl DirsMultiset {
         self.inner.contains_key(key)
     }
 
-    pub fn iter(&self) -> Iter<Vec<u8>, u32> {
-        self.inner.iter()
+    pub fn iter(&self) -> Keys<Vec<u8>, u32> {
+        self.inner.keys()
     }
 
     pub fn len(&self) -> usize {
diff --git a/rust/hg-cpython/src/dirstate.rs b/rust/hg-cpython/src/dirstate.rs
--- a/rust/hg-cpython/src/dirstate.rs
+++ b/rust/hg-cpython/src/dirstate.rs
@@ -271,20 +271,9 @@  py_class!(pub class Dirs |py| {
     // 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))
+        // maybe there would be a better way to cast objects back and forth?
+        let dirs: Vec<_> = self.dirs_map(py).borrow().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> {