Skip to content

Commit

Permalink
Try #2914:
Browse files Browse the repository at this point in the history
  • Loading branch information
bors[bot] committed Feb 3, 2023
2 parents 141cbea + 5bab0e9 commit bbecea2
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 14 deletions.
1 change: 1 addition & 0 deletions newsfragments/2914.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FFI definition `PyIter_Check` on CPython 3.7 now does the equivalent for `hasattr(type(obj), "__next__")`, which works correctly on all platforms and adds support for `abi3`.
1 change: 1 addition & 0 deletions newsfragments/2914.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix downcast to `PyIterator` succeeding for Python classes which did not implement `__next__`.
21 changes: 11 additions & 10 deletions pyo3-ffi/src/abstract_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,21 +91,22 @@ extern "C" {
pub fn PyObject_GetIter(arg1: *mut PyObject) -> *mut PyObject;
}

// Defined as this macro in Python limited API, but relies on
// non-limited PyTypeObject. Don't expose this since it cannot be used.
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
// Before 3.8 PyIter_Check was defined in CPython as a macro,
// but the implementation of that in PyO3 did not work, see
// https://github.com/PyO3/pyo3/pull/2914
//
// This is a slow implementation which should function equivalently.
#[cfg(not(any(Py_3_8, PyPy)))]
#[inline]
pub unsafe fn PyIter_Check(o: *mut PyObject) -> c_int {
(match (*crate::Py_TYPE(o)).tp_iternext {
Some(tp_iternext) => {
tp_iternext as *const std::os::raw::c_void != crate::_PyObject_NextNotImplemented as _
}
None => false,
}) as c_int
crate::PyObject_HasAttrString(
crate::Py_TYPE(o).cast(),
"__next__\0".as_ptr() as *const c_char,
)
}

extern "C" {
#[cfg(any(all(Py_3_8, Py_LIMITED_API), PyPy))]
#[cfg(any(Py_3_8, PyPy))]
#[cfg_attr(PyPy, link_name = "PyPyIter_Check")]
pub fn PyIter_Check(obj: *mut PyObject) -> c_int;

Expand Down
77 changes: 73 additions & 4 deletions src/types/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython

use crate::{ffi, AsPyPointer, IntoPyPointer, Py, PyAny, PyErr, PyNativeType, PyResult, Python};
#[cfg(any(not(Py_LIMITED_API), Py_3_8))]
use crate::{PyDowncastError, PyTryFrom};

/// A Python iterator object.
Expand All @@ -29,7 +28,6 @@ use crate::{PyDowncastError, PyTryFrom};
#[repr(transparent)]
pub struct PyIterator(PyAny);
pyobject_native_type_named!(PyIterator);
#[cfg(any(not(Py_LIMITED_API), Py_3_8))]
pyobject_native_type_extract!(PyIterator);

impl PyIterator {
Expand Down Expand Up @@ -64,7 +62,6 @@ impl<'p> Iterator for &'p PyIterator {
}

// PyIter_Check does not exist in the limited API until 3.8
#[cfg(any(not(Py_LIMITED_API), Py_3_8))]
impl<'v> PyTryFrom<'v> for PyIterator {
fn try_from<V: Into<&'v PyAny>>(value: V) -> Result<&'v PyIterator, PyDowncastError<'v>> {
let value = value.into();
Expand Down Expand Up @@ -218,7 +215,7 @@ def fibonacci(target):
}

#[test]
#[cfg(any(not(Py_LIMITED_API), Py_3_8))]

fn iterator_try_from() {
Python::with_gil(|py| {
let obj: Py<PyAny> = vec![10, 20].to_object(py).as_ref(py).iter().unwrap().into();
Expand Down Expand Up @@ -248,4 +245,76 @@ def fibonacci(target):
assert_eq!(iter_ref.get_refcnt(), 2);
})
}

#[test]
#[cfg(feature = "macros")]
fn python_class_not_iterator() {
use crate::PyErr;

#[crate::pyclass(crate = "crate")]
struct Downcaster {
failed: Option<PyErr>,
}

#[crate::pymethods(crate = "crate")]
impl Downcaster {
fn downcast_iterator(&mut self, obj: &PyAny) {
self.failed = Some(obj.downcast::<PyIterator>().unwrap_err().into());
}
}

// Regression test for 2913
Python::with_gil(|py| {
let downcaster = Py::new(py, Downcaster { failed: None }).unwrap();
crate::py_run!(
py,
downcaster,
r#"
from collections.abc import Sequence
class MySequence(Sequence):
def __init__(self):
self._data = [1, 2, 3]
def __getitem__(self, index):
return self._data[index]
def __len__(self):
return len(self._data)
downcaster.downcast_iterator(MySequence())
"#
);

assert_eq!(
downcaster.borrow_mut(py).failed.take().unwrap().to_string(),
"TypeError: 'MySequence' object cannot be converted to 'Iterator'"
);
});
}

#[test]
#[cfg(feature = "macros")]
fn python_class_iterator() {
#[crate::pyfunction(crate = "crate")]
fn assert_iterator(obj: &PyAny) {
assert!(obj.downcast::<PyIterator>().is_ok())
}

// Regression test for 2913
Python::with_gil(|py| {
let assert_iterator = crate::wrap_pyfunction!(assert_iterator, py).unwrap();
crate::py_run!(
py,
assert_iterator,
r#"
class MyIter:
def __next__(self):
raise StopIteration
assert_iterator(MyIter())
"#
);
});
}
}

0 comments on commit bbecea2

Please sign in to comment.