Merge #2914
2914: correct ffi definition of PyIter_Check r=davidhewitt a=davidhewitt Closes #2913 It looks like what is happening is that PyO3 was relying on an outdated macro form of `PyIter_Check` which is now a CPython implementation detail, which would explain why it was behaving inconsistently on different platforms (likely due to differences in linkers / implementations). The test I've pushed succeeds, but fails to compile due to a hygiene bug! I'm done for tonight so I'll take a look at that soon and then rebase this after. Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
This commit is contained in:
commit
806eed5b07
|
@ -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`.
|
|
@ -0,0 +1 @@
|
|||
Fix downcast to `PyIterator` succeeding for Python classes which did not implement `__next__`.
|
|
@ -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;
|
||||
|
||||
|
|
|
@ -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.
|
||||
|
@ -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 {
|
||||
|
@ -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();
|
||||
|
@ -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();
|
||||
|
@ -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())
|
||||
"#
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue