Skip to content

Commit

Permalink
Change PyIterator::from_object` to return underlying TypeError
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Jul 18, 2020
1 parent 41b35b8 commit a62f37b
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Rename `PyString::to_string` to `to_str`, change return type `Cow<str>` to `&str`. [#1023](https://github.com/PyO3/pyo3/pull/1023)
- Correct FFI definition `_PyLong_AsByteArray` `*mut c_uchar` argument instead of `*const c_uchar`. [#1029](https://github.com/PyO3/pyo3/pull/1029)
- `PyType::as_type_ptr` is no longer `unsafe`. [#1047](https://github.com/PyO3/pyo3/pull/1047)
- Change `PyIterator::from_object` to return `PyResult<PyIterator>` instead of `Result<PyIterator, PyDowncastError>`. [#1051](https://github.com/PyO3/pyo3/pull/1051)

### Removed
- Remove `PyString::as_bytes`. [#1023](https://github.com/PyO3/pyo3/pull/1023)
Expand Down
2 changes: 1 addition & 1 deletion src/types/any.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ impl PyAny {
/// This is typically a new iterator but if the argument is an iterator,
/// this returns itself.
pub fn iter(&self) -> PyResult<PyIterator> {
Ok(PyIterator::from_object(self.py(), self)?)
PyIterator::from_object(self.py(), self)
}

/// Returns the Python type object for this object's type.
Expand Down
35 changes: 13 additions & 22 deletions src/types/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython

use crate::{ffi, AsPyPointer, PyAny, PyDowncastError, PyErr, PyNativeType, PyResult, Python};
use crate::{ffi, AsPyPointer, PyAny, PyErr, PyNativeType, PyResult, Python};

/// A Python iterator object.
///
Expand All @@ -29,30 +29,21 @@ pub struct PyIterator<'p>(&'p PyAny);

impl<'p> PyIterator<'p> {
/// Constructs a `PyIterator` from a Python iterator object.
pub fn from_object<T>(py: Python<'p>, obj: &T) -> Result<PyIterator<'p>, PyDowncastError>
pub fn from_object<T>(py: Python<'p>, obj: &T) -> PyResult<PyIterator<'p>>
where
T: AsPyPointer,
{
unsafe {
let ptr = ffi::PyObject_GetIter(obj.as_ptr());
// Returns NULL if an object cannot be iterated.
if ptr.is_null() {
PyErr::fetch(py);
return Err(PyDowncastError);
}

if ffi::PyIter_Check(ptr) != 0 {
// 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 instead of from_owned_ptr so that the
// GILPool does not take ownership of the reference.
Ok(PyIterator(py.from_borrowed_ptr(ptr)))
} else {
Err(PyDowncastError)
}
}
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))
}
}

Expand Down

0 comments on commit a62f37b

Please sign in to comment.