From d7b4d2e797d499e5d71feefbd2a74248404d82ed Mon Sep 17 00:00:00 2001 From: Georg Brandl Date: Fri, 11 Sep 2020 19:03:01 +0200 Subject: [PATCH] implement PyIterator without additional lifetime This lets us treat it no different from other types like PySequence. --- src/types/any.rs | 2 +- src/types/iterator.rs | 59 +++++++++++++++++++++++-------------------- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/types/any.rs b/src/types/any.rs index cd973b0011d..e5f4ae84625 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -362,7 +362,7 @@ impl PyAny { /// /// This is typically a new iterator but if the argument is an iterator, /// this returns itself. - pub fn iter(&self) -> PyResult { + pub fn iter(&self) -> PyResult<&PyIterator> { PyIterator::from_object(self.py(), self) } diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 4dac1f08c91..5d9b440845a 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -2,17 +2,11 @@ // // based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython -use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python}; +use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python, + PyTryFrom, PyDowncastError}; /// A Python iterator object. /// -/// Unlike other Python objects, this class includes a `Python<'p>` token -/// so that `PyIterator` can implement the Rust `Iterator` trait. -/// -/// This means that you can't use `PyIterator` in many places where other -/// types like `PyList` can automatically be extracted from objects, such -/// as function arguments. -/// /// # Example /// /// ```rust @@ -30,31 +24,26 @@ use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python}; /// # } /// ``` #[derive(Debug)] -pub struct PyIterator<'p>(&'p PyAny); +#[repr(transparent)] +pub struct PyIterator(PyAny); +pyobject_native_type_named!(PyIterator); +pyobject_native_type_extract!(PyIterator); -impl<'p> PyIterator<'p> { +impl PyIterator { /// Constructs a `PyIterator` from a Python iterable object. /// /// Equivalent to Python's built-in `iter` function. - pub fn from_object(py: Python<'p>, obj: &T) -> PyResult> + pub fn from_object<'p, T>(py: Python<'p>, obj: &T) -> PyResult<&'p PyIterator> where T: AsPyPointer, { - let iter = unsafe { - // This looks suspicious, but is actually correct. Even though ptr is an owned - // reference, PyIterator takes ownership of the reference and decreases the count - // in its Drop implementation. - // - // Therefore we must use from_borrowed_ptr_or_err instead of from_owned_ptr_or_err so - // that the GILPool does not take ownership of the reference. - py.from_borrowed_ptr_or_err(ffi::PyObject_GetIter(obj.as_ptr()))? - }; - - Ok(PyIterator(iter)) + unsafe { + py.from_owned_ptr_or_err(ffi::PyObject_GetIter(obj.as_ptr())) + } } } -impl<'p> Iterator for PyIterator<'p> { +impl<'p> Iterator for &'p PyIterator { type Item = PyResult<&'p PyAny>; /// Retrieves the next item from an iterator. @@ -79,10 +68,26 @@ impl<'p> Iterator for PyIterator<'p> { } } -/// Dropping a `PyIterator` instance decrements the reference count on the object by 1. -impl<'p> Drop for PyIterator<'p> { - fn drop(&mut self) { - unsafe { ffi::Py_DECREF(self.0.as_ptr()) } +impl<'v> PyTryFrom<'v> for PyIterator { + fn try_from>(value: V) -> Result<&'v PyIterator, PyDowncastError<'v>> { + let value = value.into(); + unsafe { + if ffi::PyIter_Check(value.as_ptr()) != 0 { + Ok(::try_from_unchecked(value)) + } else { + Err(PyDowncastError::new(value, "Iterator")) + } + } + } + + fn try_from_exact>(value: V) -> Result<&'v PyIterator, PyDowncastError<'v>> { + ::try_from(value) + } + + #[inline] + unsafe fn try_from_unchecked>(value: V) -> &'v PyIterator { + let ptr = value.into() as *const _ as *const PyIterator; + &*ptr } }