Skip to content

Commit 9880cb4

Browse files
author
Aviram Hassan
committed
- 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.
1 parent 388c255 commit 9880cb4

File tree

9 files changed

+259
-120
lines changed

9 files changed

+259
-120
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
3939
- Fix compiler warning: deny trailing semicolons in expression macro. [#1762](https://github.com/PyO3/pyo3/pull/1762)
4040
- 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)
4141

42+
### Changed
43+
- `PyList`, `PyTuple` and `PySequence`'s `get_item` now accepts only `usize` indices instead of `isize`.
44+
### Added
45+
- Add `PyList::get_item_unchecked()` and `PyTuple::get_item_unchecked()` to get items without bounds checks. [#1733](https://github.com/PyO3/pyo3/pull/1733)
46+
47+
### Changed
48+
- `PyList` and `PyTuple`'s `get_item` now return `PyResult<&PyAny> instead of panicking. [#1733](https://github.com/PyO3/pyo3/pull/1733)
4249
## [0.14.1] - 2021-07-04
4350

4451
### Added

benches/bench_list.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,22 @@ fn list_get_item(b: &mut Bencher) {
3232
let mut sum = 0;
3333
b.iter(|| {
3434
for i in 0..LEN {
35-
sum += list.get_item(i as isize).extract::<usize>().unwrap();
35+
sum += list.get_item(i).unwrap().extract::<usize>().unwrap();
36+
}
37+
});
38+
}
39+
40+
fn list_get_item_unchecked(b: &mut Bencher) {
41+
let gil = Python::acquire_gil();
42+
let py = gil.python();
43+
const LEN: usize = 50_000;
44+
let list = PyList::new(py, 0..LEN);
45+
let mut sum = 0;
46+
b.iter(|| {
47+
for i in 0..LEN {
48+
unsafe {
49+
sum += list.get_item_unchecked(i).extract::<usize>().unwrap();
50+
}
3651
}
3752
});
3853
}
@@ -41,6 +56,7 @@ fn criterion_benchmark(c: &mut Criterion) {
4156
c.bench_function("iter_list", iter_list);
4257
c.bench_function("list_new", list_new);
4358
c.bench_function("list_get_item", list_get_item);
59+
c.bench_function("list_get_item_unchecked", list_get_item_unchecked);
4460
}
4561

4662
criterion_group!(benches, criterion_benchmark);

benches/bench_tuple.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,22 @@ fn tuple_get_item(b: &mut Bencher) {
3232
let mut sum = 0;
3333
b.iter(|| {
3434
for i in 0..LEN {
35-
sum += tuple.get_item(i).extract::<usize>().unwrap();
35+
sum += tuple.get_item(i).unwrap().extract::<usize>().unwrap();
36+
}
37+
});
38+
}
39+
40+
fn tuple_get_item_unchecked(b: &mut Bencher) {
41+
let gil = Python::acquire_gil();
42+
let py = gil.python();
43+
const LEN: usize = 50_000;
44+
let tuple = PyTuple::new(py, 0..LEN);
45+
let mut sum = 0;
46+
b.iter(|| {
47+
for i in 0..LEN {
48+
unsafe {
49+
sum += tuple.get_item_unchecked(i).extract::<usize>().unwrap();
50+
}
3651
}
3752
});
3853
}
@@ -41,6 +56,7 @@ fn criterion_benchmark(c: &mut Criterion) {
4156
c.bench_function("iter_tuple", iter_tuple);
4257
c.bench_function("tuple_new", tuple_new);
4358
c.bench_function("tuple_get_item", tuple_get_item);
59+
c.bench_function("tuple_get_item_unchecked", tuple_get_item_unchecked);
4460
}
4561

4662
criterion_group!(benches, criterion_benchmark);

pyo3-macros-backend/src/from_pyobject.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ impl<'a> Container<'a> {
243243
for i in 0..len {
244244
let error_msg = format!("failed to extract field {}.{}", quote!(#self_ty), i);
245245
fields.push(quote!(
246-
s.get_item(#i).extract().map_err(|inner| {
246+
s.get_item(#i).and_then(PyAny::extract).map_err(|inner| {
247247
let py = pyo3::PyNativeType::py(obj);
248248
let new_err = pyo3::exceptions::PyTypeError::new_err(#error_msg);
249249
new_err.set_cause(py, Some(inner));

src/conversions/array.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ mod min_const_generics {
6060
if seq_len != N {
6161
return Err(invalid_sequence_length(N, seq_len));
6262
}
63-
array_try_from_fn(|idx| seq.get_item(idx as isize).and_then(PyAny::extract))
63+
array_try_from_fn(|idx| seq.get_item(idx).and_then(PyAny::extract))
6464
}
6565

6666
// TODO use std::array::try_from_fn, if that stabilises:

src/types/dict.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -628,23 +628,23 @@ mod tests {
628628
#[test]
629629
fn test_items() {
630630
Python::with_gil(|py| {
631-
let mut v = HashMap::new();
632-
v.insert(7, 32);
633-
v.insert(8, 42);
634-
v.insert(9, 123);
635-
let ob = v.to_object(py);
636-
let dict = <PyDict as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
637-
// Can't just compare against a vector of tuples since we don't have a guaranteed ordering.
638-
let mut key_sum = 0;
639-
let mut value_sum = 0;
640-
for el in dict.items().iter() {
641-
let tuple = el.cast_as::<PyTuple>().unwrap();
642-
key_sum += tuple.get_item(0).extract::<i32>().unwrap();
643-
value_sum += tuple.get_item(1).extract::<i32>().unwrap();
644-
}
645-
assert_eq!(7 + 8 + 9, key_sum);
646-
assert_eq!(32 + 42 + 123, value_sum);
647-
});
631+
let mut v = HashMap::new();
632+
v.insert(7, 32);
633+
v.insert(8, 42);
634+
v.insert(9, 123);
635+
let ob = v.to_object(py);
636+
let dict = <PyDict as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
637+
// Can't just compare against a vector of tuples since we don't have a guaranteed ordering.
638+
let mut key_sum = 0;
639+
let mut value_sum = 0;
640+
for el in dict.items().iter() {
641+
let tuple = el.cast_as::<PyTuple>().unwrap();
642+
key_sum += tuple.get_item(0).unwrap().extract::<i32>().unwrap();
643+
value_sum += tuple.get_item(1).unwrap().extract::<i32>().unwrap();
644+
}
645+
assert_eq!(7 + 8 + 9, key_sum);
646+
assert_eq!(32 + 42 + 123, value_sum);
647+
});
648648
}
649649

650650
#[test]

0 commit comments

Comments
 (0)