From 41292755cd60d7f426e984991d93665b674e9636 Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Wed, 5 Nov 2025 11:04:06 +0100 Subject: [PATCH 1/4] Handle errors on `PyIterator` when calling `size_hint` --- newsfragments/5604.fixed.md | 1 + src/types/iterator.rs | 46 +++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 newsfragments/5604.fixed.md diff --git a/newsfragments/5604.fixed.md b/newsfragments/5604.fixed.md new file mode 100644 index 00000000000..6bc52a41a55 --- /dev/null +++ b/newsfragments/5604.fixed.md @@ -0,0 +1 @@ +Handle errors on `PyIterator` when calling `size_hint` diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 62ab3bac742..3890fd0231d 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -1,3 +1,5 @@ +#[cfg(not(Py_LIMITED_API))] +use crate::exceptions::PyNotImplementedError; use crate::ffi_ptr_ext::FfiPtrExt; use crate::instance::Borrowed; use crate::py_result_ext::PyResultExt; @@ -108,8 +110,19 @@ impl<'py> Iterator for Bound<'py, PyIterator> { #[cfg(not(Py_LIMITED_API))] fn size_hint(&self) -> (usize, Option) { + // SAFETY: `self` is a valid iterator object let hint = unsafe { ffi::PyObject_LengthHint(self.as_ptr(), 0) }; - (hint.max(0) as usize, None) + if hint < 0 { + let py = self.py(); + let err = PyErr::fetch(py); + if !err.is_instance_of::(py) { + // Write unraisable error only if it's not NotImplementedError + err.write_unraisable(py, Some(self)); + } + (0, None) + } else { + (hint as usize, None) + } } } @@ -144,7 +157,7 @@ mod tests { #[cfg(all(not(PyPy), Py_3_10))] use crate::types::PyNone; use crate::types::{PyAnyMethods, PyDict, PyList, PyListMethods}; - use crate::{IntoPyObject, PyTypeInfo, Python}; + use crate::{IntoPyObject, PyErr, PyTypeInfo, Python}; #[test] fn vec_iter() { @@ -392,6 +405,35 @@ def fibonacci(target): }); } + #[test] + #[cfg(all(feature = "macros", not(Py_LIMITED_API)))] + fn length_hint_not_implemented() { + #[crate::pyfunction(crate = "crate")] + fn test_size_hint(obj: &crate::Bound<'_, crate::PyAny>) { + let iter = obj.cast::().unwrap(); + assert_eq!((0, None), iter.size_hint()); + assert!(PyErr::take(obj.py()).is_none()); + } + + Python::attach(|py| { + let test_size_hint = crate::wrap_pyfunction!(test_size_hint, py).unwrap(); + crate::py_run!( + py, + test_size_hint, + r#" + class MyIter: + def __next__(self): + raise StopIteration + + def __length_hint__(self): + raise NotImplementedError + + test_size_hint(MyIter()) + "# + ); + }); + } + #[test] fn test_type_object() { Python::attach(|py| { From ae20ab2e81a91bc7639b3f8ac5abdf4f22ff12f0 Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Wed, 5 Nov 2025 11:13:58 +0100 Subject: [PATCH 2/4] Test `PyNotImplemented` return value as well --- src/types/iterator.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 3890fd0231d..5b2a130c1df 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -1,5 +1,3 @@ -#[cfg(not(Py_LIMITED_API))] -use crate::exceptions::PyNotImplementedError; use crate::ffi_ptr_ext::FfiPtrExt; use crate::instance::Borrowed; use crate::py_result_ext::PyResultExt; @@ -114,11 +112,7 @@ impl<'py> Iterator for Bound<'py, PyIterator> { let hint = unsafe { ffi::PyObject_LengthHint(self.as_ptr(), 0) }; if hint < 0 { let py = self.py(); - let err = PyErr::fetch(py); - if !err.is_instance_of::(py) { - // Write unraisable error only if it's not NotImplementedError - err.write_unraisable(py, Some(self)); - } + PyErr::fetch(py).write_unraisable(py, Some(self)); (0, None) } else { (hint as usize, None) @@ -157,7 +151,9 @@ mod tests { #[cfg(all(not(PyPy), Py_3_10))] use crate::types::PyNone; use crate::types::{PyAnyMethods, PyDict, PyList, PyListMethods}; - use crate::{IntoPyObject, PyErr, PyTypeInfo, Python}; + #[cfg(all(feature = "macros", not(Py_LIMITED_API)))] + use crate::PyErr; + use crate::{IntoPyObject, PyTypeInfo, Python}; #[test] fn vec_iter() { @@ -407,7 +403,7 @@ def fibonacci(target): #[test] #[cfg(all(feature = "macros", not(Py_LIMITED_API)))] - fn length_hint_not_implemented() { + fn length_hint_error() { #[crate::pyfunction(crate = "crate")] fn test_size_hint(obj: &crate::Bound<'_, crate::PyAny>) { let iter = obj.cast::().unwrap(); @@ -421,14 +417,22 @@ def fibonacci(target): py, test_size_hint, r#" - class MyIter: + class NoHintIter: + def __next__(self): + raise StopIteration + + def __length_hint__(self): + return NotImplemented + + class ErrorHintIter: def __next__(self): raise StopIteration def __length_hint__(self): - raise NotImplementedError + raise ValueError("bad hint impl") - test_size_hint(MyIter()) + test_size_hint(NoHintIter()) + test_size_hint(ErrorHintIter()) "# ); }); From d9de53e38977cdf9b937a7d12299282ff3c07045 Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Mon, 10 Nov 2025 13:42:10 +0100 Subject: [PATCH 3/4] Capture unraisable error in test --- src/types/iterator.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 5b2a130c1df..87cf260dc88 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -405,9 +405,12 @@ def fibonacci(target): #[cfg(all(feature = "macros", not(Py_LIMITED_API)))] fn length_hint_error() { #[crate::pyfunction(crate = "crate")] - fn test_size_hint(obj: &crate::Bound<'_, crate::PyAny>) { + fn test_size_hint(obj: &crate::Bound<'_, crate::PyAny>, should_error: bool) { let iter = obj.cast::().unwrap(); - assert_eq!((0, None), iter.size_hint()); + crate::test_utils::UnraisableCapture::enter(obj.py(), |capture| { + assert_eq!((0, None), iter.size_hint()); + assert_eq!(should_error, capture.take_capture().is_some()); + }); assert!(PyErr::take(obj.py()).is_none()); } @@ -431,8 +434,8 @@ def fibonacci(target): def __length_hint__(self): raise ValueError("bad hint impl") - test_size_hint(NoHintIter()) - test_size_hint(ErrorHintIter()) + test_size_hint(NoHintIter(), False) + test_size_hint(ErrorHintIter(), True) "# ); }); From a5576aa2991380374f34026ec045cdfbe0816c3d Mon Sep 17 00:00:00 2001 From: Bas Schoenmaeckers Date: Tue, 11 Nov 2025 10:58:02 +0100 Subject: [PATCH 4/4] disable test on `3.7` --- src/types/iterator.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 87cf260dc88..2ab1bda1722 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -151,7 +151,7 @@ mod tests { #[cfg(all(not(PyPy), Py_3_10))] use crate::types::PyNone; use crate::types::{PyAnyMethods, PyDict, PyList, PyListMethods}; - #[cfg(all(feature = "macros", not(Py_LIMITED_API)))] + #[cfg(all(feature = "macros", Py_3_8, not(Py_LIMITED_API)))] use crate::PyErr; use crate::{IntoPyObject, PyTypeInfo, Python}; @@ -402,7 +402,7 @@ def fibonacci(target): } #[test] - #[cfg(all(feature = "macros", not(Py_LIMITED_API)))] + #[cfg(all(feature = "macros", Py_3_8, not(Py_LIMITED_API)))] fn length_hint_error() { #[crate::pyfunction(crate = "crate")] fn test_size_hint(obj: &crate::Bound<'_, crate::PyAny>, should_error: bool) {