Skip to content

Commit

Permalink
deprecate {PyList, PyTuple}::get_item and implement ::get and ::get_u…
Browse files Browse the repository at this point in the history 
…nchecked
  • Loading branch information
Aviram Hassan committed Jul 22, 2021
1 parent bd0e0d8 commit c598465
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 51 deletions.
4 changes: 2 additions & 2 deletions src/types/dict.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,8 +644,8 @@ mod test {
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(0).unwrap().extract::<i32>().unwrap();
value_sum += tuple.get(1).unwrap().extract::<i32>().unwrap();
}
assert_eq!(7 + 8 + 9, key_sum);
assert_eq!(32 + 42 + 123, value_sum);
Expand Down
144 changes: 99 additions & 45 deletions src/types/list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ impl PyList {
/// Gets the item at the specified index.
///
/// Panics if the index is out of range.
#[deprecated = "use PyList::get, which does not panic"]
pub fn get_item(&self, index: isize) -> &PyAny {
assert!(index >= 0 && index < self.len() as isize);
unsafe {
Expand All @@ -83,6 +84,24 @@ impl PyList {
}
}

/// Gets the list item at the specified index.
pub fn get(&self, index: usize) -> PyResult<&PyAny> {
unsafe {
let item = ffi::PyList_GetItem(self.as_ptr(), index as Py_ssize_t);
self.py().from_borrowed_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_unchecked(&self, index: usize) -> &PyAny {
let item = ffi::PyList_GET_ITEM(self.as_ptr(), index as Py_ssize_t);
self.py().from_borrowed_ptr(item)
}

/// Sets the item at the specified index.
///
/// Panics if the index is out of range.
Expand Down Expand Up @@ -142,16 +161,19 @@ impl PyList {
/// Used by `PyList::iter()`.
pub struct PyListIterator<'a> {
list: &'a PyList,
index: isize,
index: usize,
}

impl<'a> Iterator for PyListIterator<'a> {
type Item = &'a PyAny;

#[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(self.index).expect("tuple.get failed");
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
let item = unsafe { self.list.get_unchecked(self.index) };
self.index += 1;
Some(item)
} else {
Expand Down Expand Up @@ -217,10 +239,10 @@ mod test {
let gil = Python::acquire_gil();
let py = gil.python();
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(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get(2).unwrap().extract::<i32>().unwrap());
assert_eq!(7, list.get(3).unwrap().extract::<i32>().unwrap());
}

#[test]
Expand All @@ -236,19 +258,10 @@ mod test {
let gil = Python::acquire_gil();
let py = gil.python();
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() {
let gil = Python::acquire_gil();
let py = gil.python();
let list = PyList::new(py, &[2, 3, 5, 7]);
list.get_item(-1);
assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get(2).unwrap().extract::<i32>().unwrap());
assert_eq!(7, list.get(3).unwrap().extract::<i32>().unwrap());
}

#[test]
Expand All @@ -257,9 +270,9 @@ mod test {
let py = gil.python();
let list = PyList::new(py, &[2, 3, 5, 7]);
let val = 42i32.to_object(py);
assert_eq!(2, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(2, list.get(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(0).unwrap().extract::<i32>().unwrap());
}

#[test]
Expand Down Expand Up @@ -288,11 +301,11 @@ mod test {
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::<i32>().unwrap());
assert_eq!(2, list.get(0).unwrap().extract::<i32>().unwrap());
list.insert(0, val).unwrap();
assert_eq!(5, list.len());
assert_eq!(42, list.get_item(0).extract::<i32>().unwrap());
assert_eq!(2, list.get_item(1).extract::<i32>().unwrap());
assert_eq!(42, list.get(0).unwrap().extract::<i32>().unwrap());
assert_eq!(2, list.get(1).unwrap().extract::<i32>().unwrap());
}

#[test]
Expand All @@ -318,8 +331,8 @@ mod test {
let py = gil.python();
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(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
}

#[test]
Expand Down Expand Up @@ -397,15 +410,15 @@ mod test {
let py = gil.python();
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(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
assert_eq!(2, list.get(2).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get(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(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get(2).unwrap().extract::<i32>().unwrap());
assert_eq!(7, list.get(3).unwrap().extract::<i32>().unwrap());
}

#[test]
Expand All @@ -414,15 +427,15 @@ mod test {
let py = gil.python();
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(0).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get(1).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get(2).unwrap().extract::<i32>().unwrap());
assert_eq!(7, list.get(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(0).unwrap().extract::<i32>().unwrap());
assert_eq!(5, list.get(1).unwrap().extract::<i32>().unwrap());
assert_eq!(3, list.get(2).unwrap().extract::<i32>().unwrap());
assert_eq!(2, list.get(3).unwrap().extract::<i32>().unwrap());
}

#[test]
Expand All @@ -431,7 +444,48 @@ mod test {
let py = gil.python();
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(0).unwrap().extract::<i32>().unwrap());
assert_eq!(2, list.get(1).unwrap().extract::<i32>().unwrap());
}

#[test]
fn test_list_get_invalid_index() {
pyo3::Python::with_gil(|py| {
let list = PyList::new(py, &[2, 3, 5, 7]);
let obj = list.get(5);
assert!(obj.is_err());
assert_eq!(
obj.unwrap_err().to_string(),
"IndexError: list index out of range"
);
});
}

#[test]
fn test_list_get_sanity() {
pyo3::Python::with_gil(|py| {
let list = PyList::new(py, &[2, 3, 5, 7]);
let obj = list.get(0);
assert_eq!(obj.unwrap().extract::<i32>().unwrap(), 2);
});
}

#[cfg(not(any(Py_LIMITED_API, PyPy)))]
#[test]
fn test_list_get_unchecked_sanity() {
pyo3::Python::with_gil(|py| {
let list = PyList::new(py, &[2, 3, 5, 7]);
let obj = unsafe { list.get_unchecked(0) };
assert_eq!(obj.extract::<i32>().unwrap(), 2);
});
}

#[cfg(not(any(Py_LIMITED_API, PyPy)))]
#[test]
fn test_list_get_unchecked_invalid_index() {
pyo3::Python::with_gil(|py| {
let list = PyList::new(py, &[2, 3, 5, 7]);
unsafe { list.get_unchecked(5) };
});
}
}
68 changes: 64 additions & 4 deletions src/types/tuple.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ impl PyTuple {
/// Gets the tuple item at the specified index.
///
/// Panics if the index is out of range.
#[deprecated = "use PyTuple::get, which does not panic"]
pub fn get_item(&self, index: usize) -> &PyAny {
assert!(index < self.len());
unsafe {
Expand All @@ -88,6 +89,24 @@ impl PyTuple {
}
}

/// Gets the tuple item at the specified index.
pub fn get(&self, index: usize) -> PyResult<&PyAny> {
unsafe {
let item = ffi::PyTuple_GetItem(self.as_ptr(), index as Py_ssize_t);
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_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))))]
Expand Down Expand Up @@ -124,7 +143,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(self.index).expect("tuple.get failed");
#[cfg(not(any(Py_LIMITED_API, PyPy)))]
let item = unsafe { self.tuple.get_unchecked(self.index) };
self.index += 1;
Some(item)
} else {
Expand Down Expand Up @@ -200,9 +222,12 @@ 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($n)?.extract::<$T>()?,)+));

#[cfg(not(any(Py_LIMITED_API, PyPy)))]
unsafe {return Ok(($(t.get_unchecked($n).extract::<$T>()?,)+));}

} else {
Err(wrong_tuple_length(t, $length))
}
Expand Down Expand Up @@ -458,4 +483,39 @@ mod test {
);
})
}

#[test]
fn test_tuple_get_invalid_index() {
pyo3::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(5);
assert!(obj.is_err());
assert_eq!(
obj.unwrap_err().to_string(),
"IndexError: tuple index out of range"
);
});
}

#[test]
fn test_tuple_get_sanity() {
pyo3::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(0);
assert_eq!(obj.unwrap().extract::<i32>().unwrap(), 1);
});
}

#[cfg(not(any(Py_LIMITED_API, PyPy)))]
#[test]
fn test_tuple_get_unchecked_sanity() {
pyo3::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_unchecked(0) };
assert_eq!(obj.extract::<i32>().unwrap(), 1);
});
}
}

0 comments on commit c598465

Please sign in to comment.