From 4ce3e9649f23a6bb2b798e5c8633d0c63f52411d Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Thu, 3 Aug 2023 19:54:59 +0200 Subject: [PATCH 1/2] Implement DoubleEndedIterator for PyTupleIterator --- newsfragments/3366.added.md | 1 + newsfragments/3366.fixed.md | 1 + src/buffer.rs | 2 +- src/types/mod.rs | 1 + src/types/tuple.rs | 60 +++++++++++++++++++++++++++++++++---- 5 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 newsfragments/3366.added.md create mode 100644 newsfragments/3366.fixed.md diff --git a/newsfragments/3366.added.md b/newsfragments/3366.added.md new file mode 100644 index 00000000000..ffc4eff8186 --- /dev/null +++ b/newsfragments/3366.added.md @@ -0,0 +1 @@ +Add implementation `DoubleEndedIterator` for `PyTupleIterator`. diff --git a/newsfragments/3366.fixed.md b/newsfragments/3366.fixed.md new file mode 100644 index 00000000000..093f863db5d --- /dev/null +++ b/newsfragments/3366.fixed.md @@ -0,0 +1 @@ +The `PyTupleIterator` type returned by `PyTuple::iter` is now public and hence can be named by downstream crates. diff --git a/src/buffer.rs b/src/buffer.rs index eb48a7564c6..f9dca60e541 100644 --- a/src/buffer.rs +++ b/src/buffer.rs @@ -495,7 +495,7 @@ impl PyBuffer { err::error_on_minusone(py, unsafe { ffi::PyBuffer_ToContiguous( - target.as_ptr() as *mut raw::c_void, + target.as_mut_ptr().cast(), #[cfg(Py_3_11)] &*self.0, #[cfg(not(Py_3_11))] diff --git a/src/types/mod.rs b/src/types/mod.rs index 6cf52593669..eb862655b30 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -75,6 +75,7 @@ pub mod iter { pub use super::dict::PyDictIterator; pub use super::frozenset::PyFrozenSetIterator; pub use super::set::PySetIterator; + pub use super::tuple::PyTupleIterator; } // Implementations core to all native types diff --git a/src/types/tuple.rs b/src/types/tuple.rs index 687acae03f6..ddaec4238af 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -1,4 +1,5 @@ use std::convert::TryInto; +use std::iter::FusedIterator; use crate::ffi::{self, Py_ssize_t}; #[cfg(feature = "experimental-inspect")] @@ -232,16 +233,23 @@ pub struct PyTupleIterator<'a> { length: usize, } +impl<'a> PyTupleIterator<'a> { + unsafe fn get_item(&self, index: usize) -> &'a PyAny { + #[cfg(any(Py_LIMITED_API, PyPy))] + let item = self.tuple.get_item(index).expect("tuple.get failed"); + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + let item = self.tuple.get_item_unchecked(index); + item + } +} + impl<'a> Iterator for PyTupleIterator<'a> { type Item = &'a PyAny; #[inline] - fn next(&mut self) -> Option<&'a PyAny> { + fn next(&mut self) -> Option { if self.index < self.length { - #[cfg(any(Py_LIMITED_API, PyPy))] - let item = self.tuple.get_item(self.index).expect("tuple.get failed"); - #[cfg(not(any(Py_LIMITED_API, PyPy)))] - let item = unsafe { self.tuple.get_item_unchecked(self.index) }; + let item = unsafe { self.get_item(self.index) }; self.index += 1; Some(item) } else { @@ -256,12 +264,27 @@ impl<'a> Iterator for PyTupleIterator<'a> { } } +impl<'a> DoubleEndedIterator for PyTupleIterator<'a> { + #[inline] + fn next_back(&mut self) -> Option { + if self.index < self.length { + let item = unsafe { self.get_item(self.length - 1) }; + self.length -= 1; + Some(item) + } else { + None + } + } +} + impl<'a> ExactSizeIterator for PyTupleIterator<'a> { fn len(&self) -> usize { self.length.saturating_sub(self.index) } } +impl FusedIterator for PyTupleIterator<'_> {} + impl<'a> IntoIterator for &'a PyTuple { type Item = &'a PyAny; type IntoIter = PyTupleIterator<'a>; @@ -510,6 +533,33 @@ mod tests { assert_eq!(3_i32, iter.next().unwrap().extract::<'_, i32>().unwrap()); assert_eq!(iter.size_hint(), (0, Some(0))); + + assert!(iter.next().is_none()); + assert!(iter.next().is_none()); + }); + } + + #[test] + fn test_iter_rev() { + Python::with_gil(|py| { + let ob = (1, 2, 3).to_object(py); + let tuple: &PyTuple = ob.downcast(py).unwrap(); + assert_eq!(3, tuple.len()); + let mut iter = tuple.iter().rev(); + + assert_eq!(iter.size_hint(), (3, Some(3))); + + assert_eq!(3_i32, iter.next().unwrap().extract::<'_, i32>().unwrap()); + assert_eq!(iter.size_hint(), (2, Some(2))); + + assert_eq!(2_i32, iter.next().unwrap().extract::<'_, i32>().unwrap()); + assert_eq!(iter.size_hint(), (1, Some(1))); + + assert_eq!(1_i32, iter.next().unwrap().extract::<'_, i32>().unwrap()); + assert_eq!(iter.size_hint(), (0, Some(0))); + + assert!(iter.next().is_none()); + assert!(iter.next().is_none()); }); } From a371fbe4f8612c1b579835fdf300abfa61098bd3 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Mon, 7 Aug 2023 14:51:46 +0200 Subject: [PATCH 2/2] Implement DoubleEndedIterator for PyListIterator by caching the length while still validating it before access. --- newsfragments/3366.added.md | 2 +- src/types/list.rs | 75 ++++++++++++++++++++++++++++++++----- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/newsfragments/3366.added.md b/newsfragments/3366.added.md index ffc4eff8186..9a6a0799e22 100644 --- a/newsfragments/3366.added.md +++ b/newsfragments/3366.added.md @@ -1 +1 @@ -Add implementation `DoubleEndedIterator` for `PyTupleIterator`. +Add implementations `DoubleEndedIterator` for `PyTupleIterator` and `PyListIterator`. diff --git a/src/types/list.rs b/src/types/list.rs index 8f3a1672f6d..b5bbf70fade 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -1,4 +1,5 @@ use std::convert::TryInto; +use std::iter::FusedIterator; use crate::err::{self, PyResult}; use crate::ffi::{self, Py_ssize_t}; @@ -264,6 +265,7 @@ impl PyList { PyListIterator { list: self, index: 0, + length: self.len(), } } @@ -291,18 +293,28 @@ index_impls!(PyList, "list", PyList::len, PyList::get_slice); pub struct PyListIterator<'a> { list: &'a PyList, index: usize, + length: usize, +} + +impl<'a> PyListIterator<'a> { + unsafe fn get_item(&self, index: usize) -> &'a PyAny { + #[cfg(any(Py_LIMITED_API, PyPy))] + let item = self.list.get_item(index).expect("list.get failed"); + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + let item = self.list.get_item_unchecked(index); + item + } } impl<'a> Iterator for PyListIterator<'a> { type Item = &'a PyAny; #[inline] - fn next(&mut self) -> Option<&'a PyAny> { - if self.index < self.list.len() { - #[cfg(any(Py_LIMITED_API, PyPy))] - let item = self.list.get_item(self.index).expect("list.get failed"); - #[cfg(not(any(Py_LIMITED_API, PyPy)))] - let item = unsafe { self.list.get_item_unchecked(self.index) }; + fn next(&mut self) -> Option { + let length = self.length.min(self.list.len()); + + if self.index < length { + let item = unsafe { self.get_item(self.index) }; self.index += 1; Some(item) } else { @@ -317,13 +329,30 @@ impl<'a> Iterator for PyListIterator<'a> { } } +impl<'a> DoubleEndedIterator for PyListIterator<'a> { + #[inline] + fn next_back(&mut self) -> Option { + let length = self.length.min(self.list.len()); + + if self.index < length { + let item = unsafe { self.get_item(length - 1) }; + self.length = length - 1; + Some(item) + } else { + None + } + } +} + impl<'a> ExactSizeIterator for PyListIterator<'a> { fn len(&self) -> usize { - self.list.len().saturating_sub(self.index) + self.length.saturating_sub(self.index) } } -impl<'a> std::iter::IntoIterator for &'a PyList { +impl FusedIterator for PyListIterator<'_> {} + +impl<'a> IntoIterator for &'a PyList { type Item = &'a PyAny; type IntoIter = PyListIterator<'a>; @@ -494,13 +523,41 @@ mod tests { iter.next(); assert_eq!(iter.size_hint(), (v.len() - 1, Some(v.len() - 1))); - // Exhust iterator. + // Exhaust iterator. for _ in &mut iter {} assert_eq!(iter.size_hint(), (0, Some(0))); }); } + #[test] + fn test_iter_rev() { + Python::with_gil(|py| { + let v = vec![2, 3, 5, 7]; + let ob = v.to_object(py); + let list: &PyList = ob.downcast(py).unwrap(); + + let mut iter = list.iter().rev(); + + assert_eq!(iter.size_hint(), (4, Some(4))); + + assert_eq!(iter.next().unwrap().extract::().unwrap(), 7); + assert_eq!(iter.size_hint(), (3, Some(3))); + + assert_eq!(iter.next().unwrap().extract::().unwrap(), 5); + assert_eq!(iter.size_hint(), (2, Some(2))); + + assert_eq!(iter.next().unwrap().extract::().unwrap(), 3); + assert_eq!(iter.size_hint(), (1, Some(1))); + + assert_eq!(iter.next().unwrap().extract::().unwrap(), 2); + assert_eq!(iter.size_hint(), (0, Some(0))); + + assert!(iter.next().is_none()); + assert!(iter.next().is_none()); + }); + } + #[test] fn test_into_iter() { Python::with_gil(|py| {