From 3b01b8f6ba726733803362f674c8e25471f2f400 Mon Sep 17 00:00:00 2001
From: kngwyu <yuji.kngw.80s.revive@gmail.com>
Date: Tue, 13 Nov 2018 00:53:06 +0900
Subject: [PATCH] Review fixes

---
 examples/rustapi_module/setup.py              |  2 +-
 examples/rustapi_module/src/dict_iter.rs      | 15 +++++++-------
 .../rustapi_module/tests/test_dict_iter.py    |  2 +-
 src/types/dict.rs                             | 20 ++++++++++++-------
 4 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/examples/rustapi_module/setup.py b/examples/rustapi_module/setup.py
index 1b9ed7f6..99baa66c 100644
--- a/examples/rustapi_module/setup.py
+++ b/examples/rustapi_module/setup.py
@@ -61,7 +61,7 @@ setup(
             rustc_flags=get_py_version_cfgs(),
         ),
         RustExtension(
-            "rustapi_module._test_dict",
+            "rustapi_module.test_dict",
             "Cargo.toml",
             rustc_flags=get_py_version_cfgs(),
         ),
diff --git a/examples/rustapi_module/src/dict_iter.rs b/examples/rustapi_module/src/dict_iter.rs
index 7394b156..a5cfaad6 100644
--- a/examples/rustapi_module/src/dict_iter.rs
+++ b/examples/rustapi_module/src/dict_iter.rs
@@ -1,9 +1,9 @@
 use pyo3::prelude::*;
 
-use pyo3::exceptions as exc;
+use pyo3::exceptions::RuntimeError;
 use pyo3::types::PyDict;
 
-#[pymodinit(_test_dict)]
+#[pymodinit(test_dict)]
 fn test_dict(_py: Python, m: &PyModule) -> PyResult<()> {
     m.add_class::<DictSize>()?;
     Ok(())
@@ -18,7 +18,7 @@ pub struct DictSize {
 impl DictSize {
     #[new]
     fn __new__(obj: &PyRawObject, expected: u32) -> PyResult<()> {
-        obj.init(|_t| DictSize { expected })
+        obj.init(|| DictSize { expected })
     }
 
     fn iter_dict(&mut self, _py: Python, dict: &PyDict) -> PyResult<u32> {
@@ -31,12 +31,13 @@ impl DictSize {
             );
         }
 
-        match seen == self.expected {
-            true => Ok(seen),
-            _ => Err(PyErr::new::<exc::TypeError, _>(format!(
+        if seen == self.expected {
+            Ok(seen)
+        } else {
+            Err(PyErr::new::<RuntimeError, _>(format!(
                 "Expected {} iterations - performed {}",
                 self.expected, seen
-            ))),
+            )))
         }
     }
 }
diff --git a/examples/rustapi_module/tests/test_dict_iter.py b/examples/rustapi_module/tests/test_dict_iter.py
index b5878867..c0af56a0 100644
--- a/examples/rustapi_module/tests/test_dict_iter.py
+++ b/examples/rustapi_module/tests/test_dict_iter.py
@@ -7,6 +7,6 @@ import pytest
 )
 def test_size(size):
     d = {}
-    for i in range(0,size):
+    for i in range(size):
         d[i] = str(i)
     assert DictSize(len(d)).iter_dict(d) == size
diff --git a/src/types/dict.rs b/src/types/dict.rs
index 22d4da23..4ecd1fe0 100644
--- a/src/types/dict.rs
+++ b/src/types/dict.rs
@@ -8,7 +8,7 @@ use crate::object::PyObject;
 use crate::python::{IntoPyPointer, Python, ToPyPointer};
 use crate::types::{PyList, PyObjectRef};
 use std;
-use std::{cmp, collections, hash, marker::PhantomData, mem};
+use std::{cmp, collections, hash, mem, ops::Drop};
 
 /// Represents a Python `dict`.
 #[repr(transparent)]
@@ -152,19 +152,19 @@ impl PyDict {
         PyDictIterator {
             dict: self.to_object(py),
             pos: 0,
-            __marker: PhantomData,
+            py,
         }
     }
 }
 
-pub struct PyDictIterator<'dict> {
+pub struct PyDictIterator<'py> {
     dict: PyObject,
     pos: isize,
-    __marker: PhantomData<&'dict ()>,
+    py: Python<'py>,
 }
 
-impl<'a> Iterator for PyDictIterator<'a> {
-    type Item = (&'a PyObjectRef, &'a PyObjectRef);
+impl<'py> Iterator for PyDictIterator<'py> {
+    type Item = (&'py PyObjectRef, &'py PyObjectRef);
 
     #[inline]
     fn next(&mut self) -> Option<Self::Item> {
@@ -172,7 +172,7 @@ impl<'a> Iterator for PyDictIterator<'a> {
             let mut key: *mut ffi::PyObject = mem::uninitialized();
             let mut value: *mut ffi::PyObject = mem::uninitialized();
             if ffi::PyDict_Next(self.dict.as_ptr(), &mut self.pos, &mut key, &mut value) != 0 {
-                let py = Python::assume_gil_acquired();
+                let py = self.py;
                 Some((py.from_borrowed_ptr(key), py.from_borrowed_ptr(value)))
             } else {
                 None
@@ -181,6 +181,12 @@ impl<'a> Iterator for PyDictIterator<'a> {
     }
 }
 
+impl<'py> Drop for PyDictIterator<'py> {
+    fn drop(&mut self) {
+        unsafe { ffi::Py_DECREF(self.dict.as_ptr()) }
+    }
+}
+
 impl<'a> std::iter::IntoIterator for &'a PyDict {
     type Item = (&'a PyObjectRef, &'a PyObjectRef);
     type IntoIter = PyDictIterator<'a>;