From 8cbf9f9f567dcb4302dfd7f63f3d1764ec4d5839 Mon Sep 17 00:00:00 2001 From: Aviram Hassan Date: Thu, 22 Jul 2021 20:22:38 +0300 Subject: [PATCH] - `PyList`, `PyTuple` and `PySequence`'s `get_item` now accepts only `usize` indices instead of `isize`. - `PyList` and `PyTuple`'s `get_item` now always uses the safe API. See `get_item_unchecked` for retrieving index without checks. --- CHANGELOG.md | 6 + benches/bench_list.rs | 18 +- benches/bench_tuple.rs | 18 +- pyo3-macros-backend/src/from_pyobject.rs | 2 +- src/conversions/array.rs | 2 +- src/types/dict.rs | 34 ++-- src/types/list.rs | 208 ++++++++++++++--------- src/types/sequence.rs | 9 +- src/types/tuple.rs | 81 +++++++-- 9 files changed, 258 insertions(+), 120 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b1219cd09e..192fbf62d41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ## [Unreleased] +### Added +- Add `PyList::get_item_unchecked()` and `PyTuple::get_item_unchecked()` to get items without bounds checks. [#1733](https://github.com/PyO3/pyo3/pull/1733) + ### Changed - Change `PyErr::fetch()` to return `Option`. [#1717](https://github.com/PyO3/pyo3/pull/1717) +- `PyList`, `PyTuple` and `PySequence`'s `get_item` now accepts only `usize` indices instead of `isize`. [#1733](https://github.com/PyO3/pyo3/pull/1733) ### Fixed @@ -39,6 +43,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Fix compiler warning: deny trailing semicolons in expression macro. [#1762](https://github.com/PyO3/pyo3/pull/1762) - Fix incorrect FFI definition of `Py_DecodeLocale`. The 2nd argument is now `*mut Py_ssize_t` instead of `Py_ssize_t`. [#1766](https://github.com/PyO3/pyo3/pull/1766) +### Changed +- `PyList` and `PyTuple`'s `get_item` now return `PyResult<&PyAny> instead of panicking. [#1733](https://github.com/PyO3/pyo3/pull/1733) ## [0.14.1] - 2021-07-04 ### Added diff --git a/benches/bench_list.rs b/benches/bench_list.rs index a778a0de262..6bc1911a967 100644 --- a/benches/bench_list.rs +++ b/benches/bench_list.rs @@ -32,7 +32,22 @@ fn list_get_item(b: &mut Bencher) { let mut sum = 0; b.iter(|| { for i in 0..LEN { - sum += list.get_item(i as isize).extract::().unwrap(); + sum += list.get_item(i).unwrap().extract::().unwrap(); + } + }); +} + +fn list_get_item_unchecked(b: &mut Bencher) { + let gil = Python::acquire_gil(); + let py = gil.python(); + const LEN: usize = 50_000; + let list = PyList::new(py, 0..LEN); + let mut sum = 0; + b.iter(|| { + for i in 0..LEN { + unsafe { + sum += list.get_item_unchecked(i).extract::().unwrap(); + } } }); } @@ -41,6 +56,7 @@ fn criterion_benchmark(c: &mut Criterion) { c.bench_function("iter_list", iter_list); c.bench_function("list_new", list_new); c.bench_function("list_get_item", list_get_item); + c.bench_function("list_get_item_unchecked", list_get_item_unchecked); } criterion_group!(benches, criterion_benchmark); diff --git a/benches/bench_tuple.rs b/benches/bench_tuple.rs index ee5caf3260d..e6359e8eb58 100644 --- a/benches/bench_tuple.rs +++ b/benches/bench_tuple.rs @@ -32,7 +32,22 @@ fn tuple_get_item(b: &mut Bencher) { let mut sum = 0; b.iter(|| { for i in 0..LEN { - sum += tuple.get_item(i).extract::().unwrap(); + sum += tuple.get_item(i).unwrap().extract::().unwrap(); + } + }); +} + +fn tuple_get_item_unchecked(b: &mut Bencher) { + let gil = Python::acquire_gil(); + let py = gil.python(); + const LEN: usize = 50_000; + let tuple = PyTuple::new(py, 0..LEN); + let mut sum = 0; + b.iter(|| { + for i in 0..LEN { + unsafe { + sum += tuple.get_item_unchecked(i).extract::().unwrap(); + } } }); } @@ -41,6 +56,7 @@ fn criterion_benchmark(c: &mut Criterion) { c.bench_function("iter_tuple", iter_tuple); c.bench_function("tuple_new", tuple_new); c.bench_function("tuple_get_item", tuple_get_item); + c.bench_function("tuple_get_item_unchecked", tuple_get_item_unchecked); } criterion_group!(benches, criterion_benchmark); diff --git a/pyo3-macros-backend/src/from_pyobject.rs b/pyo3-macros-backend/src/from_pyobject.rs index da1144253c1..460349dfc9f 100644 --- a/pyo3-macros-backend/src/from_pyobject.rs +++ b/pyo3-macros-backend/src/from_pyobject.rs @@ -243,7 +243,7 @@ impl<'a> Container<'a> { for i in 0..len { let error_msg = format!("failed to extract field {}.{}", quote!(#self_ty), i); fields.push(quote!( - s.get_item(#i).extract().map_err(|inner| { + s.get_item(#i).and_then(PyAny::extract).map_err(|inner| { let py = pyo3::PyNativeType::py(obj); let new_err = pyo3::exceptions::PyTypeError::new_err(#error_msg); new_err.set_cause(py, Some(inner)); diff --git a/src/conversions/array.rs b/src/conversions/array.rs index 31cde938242..e6d10347caa 100644 --- a/src/conversions/array.rs +++ b/src/conversions/array.rs @@ -60,7 +60,7 @@ mod min_const_generics { if seq_len != N { return Err(invalid_sequence_length(N, seq_len)); } - array_try_from_fn(|idx| seq.get_item(idx as isize).and_then(PyAny::extract)) + array_try_from_fn(|idx| seq.get_item(idx).and_then(PyAny::extract)) } // TODO use std::array::try_from_fn, if that stabilises: diff --git a/src/types/dict.rs b/src/types/dict.rs index f410adc88ce..0ec8101aed3 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -628,23 +628,23 @@ mod tests { #[test] fn test_items() { Python::with_gil(|py| { - let mut v = HashMap::new(); - v.insert(7, 32); - v.insert(8, 42); - v.insert(9, 123); - let ob = v.to_object(py); - let dict = ::try_from(ob.as_ref(py)).unwrap(); - // Can't just compare against a vector of tuples since we don't have a guaranteed ordering. - let mut key_sum = 0; - let mut value_sum = 0; - for el in dict.items().iter() { - let tuple = el.cast_as::().unwrap(); - key_sum += tuple.get_item(0).extract::().unwrap(); - value_sum += tuple.get_item(1).extract::().unwrap(); - } - assert_eq!(7 + 8 + 9, key_sum); - assert_eq!(32 + 42 + 123, value_sum); - }); + let mut v = HashMap::new(); + v.insert(7, 32); + v.insert(8, 42); + v.insert(9, 123); + let ob = v.to_object(py); + let dict = ::try_from(ob.as_ref(py)).unwrap(); + // Can't just compare against a vector of tuples since we don't have a guaranteed ordering. + let mut key_sum = 0; + let mut value_sum = 0; + for el in dict.items().iter() { + let tuple = el.cast_as::().unwrap(); + key_sum += tuple.get_item(0).unwrap().extract::().unwrap(); + value_sum += tuple.get_item(1).unwrap().extract::().unwrap(); + } + assert_eq!(7 + 8 + 9, key_sum); + assert_eq!(32 + 42 + 123, value_sum); + }); } #[test] diff --git a/src/types/list.rs b/src/types/list.rs index 14e9f63b68d..464b11ca981 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -66,23 +66,38 @@ impl PyList { self.len() == 0 } - /// Gets the item at the specified index. - /// - /// Panics if the index is out of range. - pub fn get_item(&self, index: isize) -> &PyAny { - assert!(index >= 0 && index < self.len() as isize); + /// Gets the list item at the specified index. + /// # Example + /// ``` + /// use pyo3::{prelude::*, types::PyList}; + /// Python::with_gil(|py| { + /// let list = PyList::new(py, &[2, 3, 5, 7]); + /// let obj = list.get_item(0); + /// assert_eq!(obj.unwrap().extract::().unwrap(), 2); + /// }); + /// ``` + pub fn get_item(&self, index: usize) -> PyResult<&PyAny> { unsafe { - #[cfg(not(Py_LIMITED_API))] - let ptr = ffi::PyList_GET_ITEM(self.as_ptr(), index as Py_ssize_t); - #[cfg(Py_LIMITED_API)] - let ptr = ffi::PyList_GetItem(self.as_ptr(), index as Py_ssize_t); - + let item = ffi::PyList_GetItem(self.as_ptr(), index as Py_ssize_t); // PyList_GetItem return borrowed ptr; must make owned for safety (see #890). - ffi::Py_INCREF(ptr); - self.py().from_owned_ptr(ptr) + ffi::Py_INCREF(item); + self.py().from_owned_ptr_or_err(item) } } + /// Gets the list item at the specified index. Undefined behavior on bad index. Use with caution. + /// + /// # Safety + /// + /// Caller must verify that the index is within the bounds of the list. + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + pub unsafe fn get_item_unchecked(&self, index: usize) -> &PyAny { + let item = ffi::PyList_GET_ITEM(self.as_ptr(), index as Py_ssize_t); + // PyList_GET_ITEM return borrowed ptr; must make owned for safety (see #890). + ffi::Py_INCREF(item); + self.py().from_owned_ptr(item) + } + /// Sets the item at the specified index. /// /// Panics if the index is out of range. @@ -142,7 +157,7 @@ impl PyList { /// Used by `PyList::iter()`. pub struct PyListIterator<'a> { list: &'a PyList, - index: isize, + index: usize, } impl<'a> Iterator for PyListIterator<'a> { @@ -150,8 +165,11 @@ impl<'a> Iterator for PyListIterator<'a> { #[inline] fn next(&mut self) -> Option<&'a PyAny> { - if self.index < self.list.len() as isize { - let item = self.list.get_item(self.index); + if self.index < self.list.len() { + #[cfg(any(Py_LIMITED_API, PyPy))] + let item = self.list.get_item(self.index).expect("tuple.get failed"); + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + let item = unsafe { self.list.get_item_unchecked(self.index) }; self.index += 1; Some(item) } else { @@ -164,8 +182,8 @@ impl<'a> Iterator for PyListIterator<'a> { let len = self.list.len(); ( - len.saturating_sub(self.index as usize), - Some(len.saturating_sub(self.index as usize)), + len.saturating_sub(self.index), + Some(len.saturating_sub(self.index)), ) } } @@ -215,11 +233,11 @@ mod tests { #[test] fn test_new() { Python::with_gil(|py| { - let list = PyList::new(py, &[2, 3, 5, 7]); - assert_eq!(2, list.get_item(0).extract::().unwrap()); - assert_eq!(3, list.get_item(1).extract::().unwrap()); - assert_eq!(5, list.get_item(2).extract::().unwrap()); - assert_eq!(7, list.get_item(3).extract::().unwrap()); + let list = PyList::new(py, &[2, 3, 5, 7]); + assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(3, list.get_item(1).unwrap().extract::().unwrap()); + assert_eq!(5, list.get_item(2).unwrap().extract::().unwrap()); + assert_eq!(7, list.get_item(3).unwrap().extract::().unwrap()); }); } @@ -234,31 +252,22 @@ mod tests { #[test] fn test_get_item() { Python::with_gil(|py| { - let list = PyList::new(py, &[2, 3, 5, 7]); - assert_eq!(2, list.get_item(0).extract::().unwrap()); - assert_eq!(3, list.get_item(1).extract::().unwrap()); - assert_eq!(5, list.get_item(2).extract::().unwrap()); - assert_eq!(7, list.get_item(3).extract::().unwrap()); - }); - } - - #[test] - #[should_panic] - fn test_get_item_invalid() { - Python::with_gil(|py| { - let list = PyList::new(py, &[2, 3, 5, 7]); - list.get_item(-1); + let list = PyList::new(py, &[2, 3, 5, 7]); + assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(3, list.get_item(1).unwrap().extract::().unwrap()); + assert_eq!(5, list.get_item(2).unwrap().extract::().unwrap()); + assert_eq!(7, list.get_item(3).unwrap().extract::().unwrap()); }); } #[test] fn test_set_item() { Python::with_gil(|py| { - let list = PyList::new(py, &[2, 3, 5, 7]); - let val = 42i32.to_object(py); - assert_eq!(2, list.get_item(0).extract::().unwrap()); - list.set_item(0, val).unwrap(); - assert_eq!(42, list.get_item(0).extract::().unwrap()); + let list = PyList::new(py, &[2, 3, 5, 7]); + let val = 42i32.to_object(py); + assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); + list.set_item(0, val).unwrap(); + assert_eq!(42, list.get_item(0).unwrap().extract::().unwrap()); }); } @@ -283,14 +292,14 @@ mod tests { #[test] fn test_insert() { Python::with_gil(|py| { - let list = PyList::new(py, &[2, 3, 5, 7]); - let val = 42i32.to_object(py); - assert_eq!(4, list.len()); - assert_eq!(2, list.get_item(0).extract::().unwrap()); - list.insert(0, val).unwrap(); - assert_eq!(5, list.len()); - assert_eq!(42, list.get_item(0).extract::().unwrap()); - assert_eq!(2, list.get_item(1).extract::().unwrap()); + let list = PyList::new(py, &[2, 3, 5, 7]); + let val = 42i32.to_object(py); + assert_eq!(4, list.len()); + assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); + list.insert(0, val).unwrap(); + assert_eq!(5, list.len()); + assert_eq!(42, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(2, list.get_item(1).unwrap().extract::().unwrap()); }); } @@ -313,10 +322,10 @@ mod tests { #[test] fn test_append() { Python::with_gil(|py| { - let list = PyList::new(py, &[2]); - list.append(3).unwrap(); - assert_eq!(2, list.get_item(0).extract::().unwrap()); - assert_eq!(3, list.get_item(1).extract::().unwrap()); + let list = PyList::new(py, &[2]); + list.append(3).unwrap(); + assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(3, list.get_item(1).unwrap().extract::().unwrap()); }); } @@ -391,44 +400,85 @@ mod tests { #[test] fn test_sort() { Python::with_gil(|py| { - let v = vec![7, 3, 2, 5]; - let list = PyList::new(py, &v); - assert_eq!(7, list.get_item(0).extract::().unwrap()); - assert_eq!(3, list.get_item(1).extract::().unwrap()); - assert_eq!(2, list.get_item(2).extract::().unwrap()); - assert_eq!(5, list.get_item(3).extract::().unwrap()); - list.sort().unwrap(); - assert_eq!(2, list.get_item(0).extract::().unwrap()); - assert_eq!(3, list.get_item(1).extract::().unwrap()); - assert_eq!(5, list.get_item(2).extract::().unwrap()); - assert_eq!(7, list.get_item(3).extract::().unwrap()); + let v = vec![7, 3, 2, 5]; + let list = PyList::new(py, &v); + assert_eq!(7, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(3, list.get_item(1).unwrap().extract::().unwrap()); + assert_eq!(2, list.get_item(2).unwrap().extract::().unwrap()); + assert_eq!(5, list.get_item(3).unwrap().extract::().unwrap()); + list.sort().unwrap(); + assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(3, list.get_item(1).unwrap().extract::().unwrap()); + assert_eq!(5, list.get_item(2).unwrap().extract::().unwrap()); + assert_eq!(7, list.get_item(3).unwrap().extract::().unwrap()); }); } #[test] fn test_reverse() { Python::with_gil(|py| { - let v = vec![2, 3, 5, 7]; - let list = PyList::new(py, &v); - assert_eq!(2, list.get_item(0).extract::().unwrap()); - assert_eq!(3, list.get_item(1).extract::().unwrap()); - assert_eq!(5, list.get_item(2).extract::().unwrap()); - assert_eq!(7, list.get_item(3).extract::().unwrap()); - list.reverse().unwrap(); - assert_eq!(7, list.get_item(0).extract::().unwrap()); - assert_eq!(5, list.get_item(1).extract::().unwrap()); - assert_eq!(3, list.get_item(2).extract::().unwrap()); - assert_eq!(2, list.get_item(3).extract::().unwrap()); + let v = vec![2, 3, 5, 7]; + let list = PyList::new(py, &v); + assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(3, list.get_item(1).unwrap().extract::().unwrap()); + assert_eq!(5, list.get_item(2).unwrap().extract::().unwrap()); + assert_eq!(7, list.get_item(3).unwrap().extract::().unwrap()); + list.reverse().unwrap(); + assert_eq!(7, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(5, list.get_item(1).unwrap().extract::().unwrap()); + assert_eq!(3, list.get_item(2).unwrap().extract::().unwrap()); + assert_eq!(2, list.get_item(3).unwrap().extract::().unwrap()); }); } #[test] fn test_array_into_py() { Python::with_gil(|py| { - let array: PyObject = [1, 2].into_py(py); - let list = ::try_from(array.as_ref(py)).unwrap(); - assert_eq!(1, list.get_item(0).extract::().unwrap()); - assert_eq!(2, list.get_item(1).extract::().unwrap()); + let array: PyObject = [1, 2].into_py(py); + let list = ::try_from(array.as_ref(py)).unwrap(); + assert_eq!(1, list.get_item(0).unwrap().extract::().unwrap()); + assert_eq!(2, list.get_item(1).unwrap().extract::().unwrap()); + }); + } + + #[test] + fn test_list_get_item_invalid_index() { + Python::with_gil(|py| { + let list = PyList::new(py, &[2, 3, 5, 7]); + let obj = list.get_item(5); + assert!(obj.is_err()); + assert_eq!( + obj.unwrap_err().to_string(), + "IndexError: list index out of range" + ); + }); + } + + #[test] + fn test_list_get_item_sanity() { + Python::with_gil(|py| { + let list = PyList::new(py, &[2, 3, 5, 7]); + let obj = list.get_item(0); + assert_eq!(obj.unwrap().extract::().unwrap(), 2); + }); + } + + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + #[test] + fn test_list_get_item_unchecked_sanity() { + Python::with_gil(|py| { + let list = PyList::new(py, &[2, 3, 5, 7]); + let obj = unsafe { list.get_item_unchecked(0) }; + assert_eq!(obj.extract::().unwrap(), 2); + }); + } + + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + #[test] + fn test_list_get_item_unchecked_invalid_index() { + Python::with_gil(|py| { + let list = PyList::new(py, &[2, 3, 5, 7]); + unsafe { list.get_item_unchecked(5) }; }); } } diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 1407b0e6128..cd6cb48af8d 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -100,9 +100,9 @@ impl PySequence { /// Returns the `index`th element of the Sequence. /// - /// This is equivalent to the Python expression `self[index]`. + /// This is equivalent to the Python expression `self[index]` without support of negative indices. #[inline] - pub fn get_item(&self, index: isize) -> PyResult<&PyAny> { + pub fn get_item(&self, index: usize) -> PyResult<&PyAny> { unsafe { self.py() .from_owned_ptr_or_err(ffi::PySequence_GetItem(self.as_ptr(), index as Py_ssize_t)) @@ -404,11 +404,6 @@ mod tests { assert_eq!(3, seq.get_item(3).unwrap().extract::().unwrap()); assert_eq!(5, seq.get_item(4).unwrap().extract::().unwrap()); assert_eq!(8, seq.get_item(5).unwrap().extract::().unwrap()); - assert_eq!(8, seq.get_item(-1).unwrap().extract::().unwrap()); - assert_eq!(5, seq.get_item(-2).unwrap().extract::().unwrap()); - assert_eq!(3, seq.get_item(-3).unwrap().extract::().unwrap()); - assert_eq!(2, seq.get_item(-4).unwrap().extract::().unwrap()); - assert_eq!(1, seq.get_item(-5).unwrap().extract::().unwrap()); assert!(seq.get_item(10).is_err()); }); } diff --git a/src/types/tuple.rs b/src/types/tuple.rs index b34422b93d9..ef1876ce17f 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -74,20 +74,35 @@ impl PyTuple { } /// Gets the tuple item at the specified index. - /// - /// Panics if the index is out of range. - pub fn get_item(&self, index: usize) -> &PyAny { - assert!(index < self.len()); + /// # Example + /// ``` + /// use pyo3::{prelude::*, types::PyTuple}; + /// Python::with_gil(|py| -> PyResult<()> { + /// let ob = (1, 2, 3).to_object(py); + /// let tuple = ::try_from(ob.as_ref(py)).unwrap(); + /// let obj = tuple.get_item(0); + /// assert_eq!(obj.unwrap().extract::().unwrap(), 1); + /// Ok(()) + /// }); + /// ``` + pub fn get_item(&self, index: usize) -> PyResult<&PyAny> { unsafe { - #[cfg(not(any(Py_LIMITED_API, PyPy)))] - let item = ffi::PyTuple_GET_ITEM(self.as_ptr(), index as Py_ssize_t); - #[cfg(any(Py_LIMITED_API, PyPy))] let item = ffi::PyTuple_GetItem(self.as_ptr(), index as Py_ssize_t); - - self.py().from_borrowed_ptr(item) + self.py().from_borrowed_ptr_or_err(item) } } + /// Gets the tuple item at the specified index. Undefined behavior on bad index. Use with caution. + /// + /// # Safety + /// + /// Caller must verify that the index is within the bounds of the list. + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + pub unsafe fn get_item_unchecked(&self, index: usize) -> &PyAny { + let item = ffi::PyTuple_GET_ITEM(self.as_ptr(), index as Py_ssize_t); + self.py().from_borrowed_ptr(item) + } + /// Returns `self` as a slice of objects. #[cfg(not(Py_LIMITED_API))] #[cfg_attr(docsrs, doc(cfg(not(Py_LIMITED_API))))] @@ -124,7 +139,10 @@ impl<'a> Iterator for PyTupleIterator<'a> { #[inline] fn next(&mut self) -> Option<&'a PyAny> { if self.index < self.length { - let item = self.tuple.get_item(self.index); + #[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) }; self.index += 1; Some(item) } else { @@ -200,9 +218,11 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ { let t = ::try_from(obj)?; if t.len() == $length { - Ok(( - $(t.get_item($n).extract::<$T>()?,)+ - )) + #[cfg(any(Py_LIMITED_API, PyPy))] + return Ok(($(t.get_item($n)?.extract::<$T>()?,)+)); + + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + unsafe {return Ok(($(t.get_item_unchecked($n).extract::<$T>()?,)+));} } else { Err(wrong_tuple_length(t, $length)) } @@ -458,4 +478,39 @@ mod tests { ); }) } + + #[test] + fn test_tuple_get_item_invalid_index() { + Python::with_gil(|py| { + let ob = (1, 2, 3).to_object(py); + let tuple = ::try_from(ob.as_ref(py)).unwrap(); + let obj = tuple.get_item(5); + assert!(obj.is_err()); + assert_eq!( + obj.unwrap_err().to_string(), + "IndexError: tuple index out of range" + ); + }); + } + + #[test] + fn test_tuple_get_item_sanity() { + Python::with_gil(|py| { + let ob = (1, 2, 3).to_object(py); + let tuple = ::try_from(ob.as_ref(py)).unwrap(); + let obj = tuple.get_item(0); + assert_eq!(obj.unwrap().extract::().unwrap(), 1); + }); + } + + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + #[test] + fn test_tuple_get_item_unchecked_sanity() { + Python::with_gil(|py| { + let ob = (1, 2, 3).to_object(py); + let tuple = ::try_from(ob.as_ref(py)).unwrap(); + let obj = unsafe { tuple.get_item_unchecked(0) }; + assert_eq!(obj.extract::().unwrap(), 1); + }); + } }