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

[Alphare (Raphaël Gomès)]

Alphare added a comment.


  In D6593#96417 <https://phab.mercurial-scm.org/D6593#96417>, @yuja wrote:
  
  >>   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> {
  
  I guess I was trying too much to stick to the exact behavior of the Python implementation, but the refcount is indeed not public, that makes sense, thanks.
  
  Regarding your inline comment, my RFC for dirstatemap contains iterator sharing between Rust and Python, I would be very interested in what you would have to say about it.

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