luxolus/pyo3
2
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

Merge pull request #1733 from aviramha/list_stuff

Browse source 
Tuple/List get_item to fallible
This commit is contained in:
David Hewitt 2021-08-17 17:13:51 +01:00 committed by GitHub
parent 7a92e2699b c6255e6734
commit fc90092ef9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 214 additions and 86 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -10,11 +10,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### 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)
- Add `PyAny::py()` as a convenience for `PyNativeType::py()`. [#1751](https://github.com/PyO3/pyo3/pull/1751)
### Changed
- Change `PyErr::fetch()` to return `Option<PyErr>`. [#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)
- `PyList` and `PyTuple`'s `get_item` now return `PyResult<&PyAny> instead of panicking. [#1733](https://github.com/PyO3/pyo3/pull/1733)
- Deprecate `PyTuple::split_from`. [#1804](https://github.com/PyO3/pyo3/pull/1804)
### Fixed

18
benches/bench_list.rs
View file

@ -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::<usize>().unwrap();
sum += list.get_item(i).unwrap().extract::<usize>().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::<usize>().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);

18
benches/bench_tuple.rs
View file

@ -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::<usize>().unwrap();
sum += tuple.get_item(i).unwrap().extract::<usize>().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::<usize>().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);

2
pyo3-macros-backend/src/from_pyobject.rs
View file

@ -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));

2
src/conversions/array.rs
View file

@ -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:

4
src/types/dict.rs
View file

@ -639,8 +639,8 @@ mod tests {
let mut value_sum = 0;
for el in dict.items().iter() {
let tuple = el.cast_as::<PyTuple>().unwrap();
key_sum += tuple.get_item(0).extract::<i32>().unwrap();
value_sum += tuple.get_item(1).extract::<i32>().unwrap();
key_sum += tuple.get_item(0).unwrap().extract::<i32>().unwrap();
value_sum += tuple.get_item(1).unwrap().extract::<i32>().unwrap();
}
assert_eq!(7 + 8 + 9, key_sum);
assert_eq!(32 + 42 + 123, value_sum);

162
src/types/list.rs
View file

@ -66,23 +66,39 @@ 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::<i32>().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_XINCREF(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)))]
#[cfg_attr(docsrs, doc(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_XINCREF(item);
self.py().from_owned_ptr(item)
}
/// Takes the slice `self[low:high]` and returns it as a new list.
///
/// Indices must be nonnegative, and out-of-range indices are clipped to
@ -163,7 +179,7 @@ impl PyList {
/// Used by `PyList::iter()`.
pub struct PyListIterator<'a> {
list: &'a PyList,
index: isize,
index: usize,
}
impl<'a> Iterator for PyListIterator<'a> {
@ -171,8 +187,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 {
@ -185,8 +204,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)),
)
}
}
@ -237,10 +256,10 @@ mod tests {
fn test_new() {
Python::with_gil(|py| {
let list = PyList::new(py, &[2, 3, 5, 7]);
assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(5, list.get_item(2).extract::<i32>().unwrap());
assert_eq!(7, list.get_item(3).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get_item(2).unwrap().extract::<i32>().unwrap());
assert_eq!(7, list.get_item(3).unwrap().extract::<i32>().unwrap());
});
}
@ -256,19 +275,10 @@ mod tests {
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::<i32>().unwrap());
assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(5, list.get_item(2).extract::<i32>().unwrap());
assert_eq!(7, list.get_item(3).extract::<i32>().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);
assert_eq!(2, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get_item(2).unwrap().extract::<i32>().unwrap());
assert_eq!(7, list.get_item(3).unwrap().extract::<i32>().unwrap());
});
}
@ -289,9 +299,9 @@ mod tests {
let list = PyList::new(py, &[2, 3, 5, 7]);
let val = 42i32.to_object(py);
let val2 = 42i32.to_object(py);
assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(0).unwrap().extract::<i32>().unwrap());
list.set_item(0, val).unwrap();
assert_eq!(42, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(42, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert!(list.set_item(10, val2).is_err());
});
}
@ -321,13 +331,13 @@ mod tests {
let val = 42i32.to_object(py);
let val2 = 43i32.to_object(py);
assert_eq!(4, list.len());
assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(0).unwrap().extract::<i32>().unwrap());
list.insert(0, val).unwrap();
list.insert(1000, val2).unwrap();
assert_eq!(6, list.len());
assert_eq!(42, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(43, list.get_item(5).extract::<i32>().unwrap());
assert_eq!(42, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(2, list.get_item(1).unwrap().extract::<i32>().unwrap());
assert_eq!(43, list.get_item(5).unwrap().extract::<i32>().unwrap());
});
}
@ -352,8 +362,8 @@ mod tests {
Python::with_gil(|py| {
let list = PyList::new(py, &[2]);
list.append(3).unwrap();
assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).unwrap().extract::<i32>().unwrap());
});
}
@ -430,15 +440,15 @@ mod tests {
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::<i32>().unwrap());
assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(2).extract::<i32>().unwrap());
assert_eq!(5, list.get_item(3).extract::<i32>().unwrap());
assert_eq!(7, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).unwrap().extract::<i32>().unwrap());
assert_eq!(2, list.get_item(2).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get_item(3).unwrap().extract::<i32>().unwrap());
list.sort().unwrap();
assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(5, list.get_item(2).extract::<i32>().unwrap());
assert_eq!(7, list.get_item(3).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get_item(2).unwrap().extract::<i32>().unwrap());
assert_eq!(7, list.get_item(3).unwrap().extract::<i32>().unwrap());
});
}
@ -447,15 +457,15 @@ mod tests {
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::<i32>().unwrap());
assert_eq!(3, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(5, list.get_item(2).extract::<i32>().unwrap());
assert_eq!(7, list.get_item(3).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get_item(1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get_item(2).unwrap().extract::<i32>().unwrap());
assert_eq!(7, list.get_item(3).unwrap().extract::<i32>().unwrap());
list.reverse().unwrap();
assert_eq!(7, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(5, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(3, list.get_item(2).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(3).extract::<i32>().unwrap());
assert_eq!(7, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get_item(1).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get_item(2).unwrap().extract::<i32>().unwrap());
assert_eq!(2, list.get_item(3).unwrap().extract::<i32>().unwrap());
});
}
@ -464,8 +474,40 @@ mod tests {
Python::with_gil(|py| {
let array: PyObject = [1, 2].into_py(py);
let list = <PyList as PyTryFrom>::try_from(array.as_ref(py)).unwrap();
assert_eq!(1, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(1, list.get_item(0).unwrap().extract::<i32>().unwrap());
assert_eq!(2, list.get_item(1).unwrap().extract::<i32>().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::<i32>().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::<i32>().unwrap(), 2);
});
}
}

9
src/types/sequence.rs
View file

@ -99,9 +99,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))
@ -403,11 +403,6 @@ mod tests {
assert_eq!(3, seq.get_item(3).unwrap().extract::<i32>().unwrap());
assert_eq!(5, seq.get_item(4).unwrap().extract::<i32>().unwrap());
assert_eq!(8, seq.get_item(5).unwrap().extract::<i32>().unwrap());
assert_eq!(8, seq.get_item(-1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, seq.get_item(-2).unwrap().extract::<i32>().unwrap());
assert_eq!(3, seq.get_item(-3).unwrap().extract::<i32>().unwrap());
assert_eq!(2, seq.get_item(-4).unwrap().extract::<i32>().unwrap());
assert_eq!(1, seq.get_item(-5).unwrap().extract::<i32>().unwrap());
assert!(seq.get_item(10).is_err());
});
}

82
src/types/tuple.rs
View file

@ -86,20 +86,36 @@ 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 = <PyTuple as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
/// let obj = tuple.get_item(0);
/// assert_eq!(obj.unwrap().extract::<i32>().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 tuple.
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
#[cfg_attr(docsrs, doc(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))))]
@ -136,7 +152,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 {
@ -212,9 +231,11 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
{
let t = <PyTuple as PyTryFrom>::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))
}
@ -481,4 +502,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 = <PyTuple as PyTryFrom>::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 = <PyTuple as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
let obj = tuple.get_item(0);
assert_eq!(obj.unwrap().extract::<i32>().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 = <PyTuple as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
let obj = unsafe { tuple.get_item_unchecked(0) };
assert_eq!(obj.extract::<i32>().unwrap(), 1);
});
}
}