From eed196329d8ae8129aa399f40d529dd1c08ca270 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 23 Jan 2024 08:39:19 +0000 Subject: [PATCH] add list bound constructors --- pyo3-benches/benches/bench_frompyobject.rs | 20 +++------ pyo3-benches/benches/bench_list.rs | 15 +++---- src/conversions/smallvec.rs | 8 ++-- src/marker.rs | 10 ++--- src/tests/hygiene/pymethods.rs | 8 ++-- src/types/dict.rs | 1 + src/types/list.rs | 47 ++++++++++++++++++++-- src/types/module.rs | 2 +- src/types/sequence.rs | 1 + tests/test_frompyobject.rs | 3 +- tests/test_getter_setter.rs | 4 +- tests/test_methods.rs | 4 +- tests/test_proto_methods.rs | 4 +- 13 files changed, 79 insertions(+), 48 deletions(-) diff --git a/pyo3-benches/benches/bench_frompyobject.rs b/pyo3-benches/benches/bench_frompyobject.rs index 1ad53c71..c94cb114 100644 --- a/pyo3-benches/benches/bench_frompyobject.rs +++ b/pyo3-benches/benches/bench_frompyobject.rs @@ -23,21 +23,17 @@ fn enum_from_pyobject(b: &mut Bencher<'_>) { fn list_via_downcast(b: &mut Bencher<'_>) { Python::with_gil(|py| { - let any: &PyAny = PyList::empty(py).into(); + let any: &Bound<'_, PyAny> = &PyList::empty_bound(py); - b.iter(|| { - let _list: &PyList = black_box(any).downcast().unwrap(); - }); + b.iter(|| black_box(any).downcast::().unwrap()); }) } fn list_via_extract(b: &mut Bencher<'_>) { Python::with_gil(|py| { - let any: &PyAny = PyList::empty(py).into(); + let any: &Bound<'_, PyAny> = &PyList::empty_bound(py); - b.iter(|| { - let _list: &PyList = black_box(any).extract().unwrap(); - }); + b.iter(|| black_box(any).extract::>().unwrap()); }) } @@ -45,9 +41,7 @@ fn not_a_list_via_downcast(b: &mut Bencher<'_>) { Python::with_gil(|py| { let any: &PyAny = PyString::new(py, "foobar").into(); - b.iter(|| { - black_box(any).downcast::().unwrap_err(); - }); + b.iter(|| black_box(any).downcast::().unwrap_err()); }) } @@ -55,9 +49,7 @@ fn not_a_list_via_extract(b: &mut Bencher<'_>) { Python::with_gil(|py| { let any: &PyAny = PyString::new(py, "foobar").into(); - b.iter(|| { - black_box(any).extract::<&PyList>().unwrap_err(); - }); + b.iter(|| black_box(any).extract::<&PyList>().unwrap_err()); }) } diff --git a/pyo3-benches/benches/bench_list.rs b/pyo3-benches/benches/bench_list.rs index 34bf9d85..6f8a678b 100644 --- a/pyo3-benches/benches/bench_list.rs +++ b/pyo3-benches/benches/bench_list.rs @@ -6,10 +6,10 @@ use pyo3::types::{PyList, PySequence}; fn iter_list(b: &mut Bencher<'_>) { Python::with_gil(|py| { const LEN: usize = 100_000; - let list = PyList::new(py, 0..LEN); + let list = PyList::new_bound(py, 0..LEN); let mut sum = 0; b.iter(|| { - for x in list { + for x in list.iter() { let i: u64 = x.extract().unwrap(); sum += i; } @@ -20,14 +20,14 @@ fn iter_list(b: &mut Bencher<'_>) { fn list_new(b: &mut Bencher<'_>) { Python::with_gil(|py| { const LEN: usize = 50_000; - b.iter(|| PyList::new(py, 0..LEN)); + b.iter(|| PyList::new_bound(py, 0..LEN)); }); } fn list_get_item(b: &mut Bencher<'_>) { Python::with_gil(|py| { const LEN: usize = 50_000; - let list = PyList::new(py, 0..LEN); + let list = PyList::new_bound(py, 0..LEN); let mut sum = 0; b.iter(|| { for i in 0..LEN { @@ -41,7 +41,7 @@ fn list_get_item(b: &mut Bencher<'_>) { fn list_get_item_unchecked(b: &mut Bencher<'_>) { Python::with_gil(|py| { const LEN: usize = 50_000; - let list = PyList::new(py, 0..LEN); + let list = PyList::new_bound(py, 0..LEN); let mut sum = 0; b.iter(|| { for i in 0..LEN { @@ -56,9 +56,10 @@ fn list_get_item_unchecked(b: &mut Bencher<'_>) { fn sequence_from_list(b: &mut Bencher<'_>) { Python::with_gil(|py| { const LEN: usize = 50_000; - let list = PyList::new(py, 0..LEN).to_object(py); + let list = PyList::new_bound(py, 0..LEN).to_object(py); b.iter(|| { - let _: &PySequence = list.extract(py).unwrap(); + let seq: &PySequence = list.extract(py).unwrap(); + seq }); }); } diff --git a/src/conversions/smallvec.rs b/src/conversions/smallvec.rs index d2e84421..37ae0922 100644 --- a/src/conversions/smallvec.rs +++ b/src/conversions/smallvec.rs @@ -95,14 +95,14 @@ where #[cfg(test)] mod tests { use super::*; - use crate::types::{PyDict, PyList}; + use crate::types::{any::PyAnyMethods, PyDict, PyList}; #[test] fn test_smallvec_into_py() { Python::with_gil(|py| { let sv: SmallVec<[u64; 8]> = [1, 2, 3, 4, 5].iter().cloned().collect(); let hso: PyObject = sv.clone().into_py(py); - let l = PyList::new(py, [1, 2, 3, 4, 5]); + let l = PyList::new_bound(py, [1, 2, 3, 4, 5]); assert!(l.eq(hso).unwrap()); }); } @@ -110,7 +110,7 @@ mod tests { #[test] fn test_smallvec_from_py_object() { Python::with_gil(|py| { - let l = PyList::new(py, [1, 2, 3, 4, 5]); + let l = PyList::new_bound(py, [1, 2, 3, 4, 5]); let sv: SmallVec<[u64; 8]> = l.extract().unwrap(); assert_eq!(sv.as_slice(), [1, 2, 3, 4, 5]); }); @@ -133,7 +133,7 @@ mod tests { Python::with_gil(|py| { let sv: SmallVec<[u64; 8]> = [1, 2, 3, 4, 5].iter().cloned().collect(); let hso: PyObject = sv.to_object(py); - let l = PyList::new(py, [1, 2, 3, 4, 5]); + let l = PyList::new_bound(py, [1, 2, 3, 4, 5]); assert!(l.eq(hso).unwrap()); }); } diff --git a/src/marker.rs b/src/marker.rs index 97f826e5..74c7095f 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -1067,8 +1067,7 @@ impl<'unbound> Python<'unbound> { #[cfg(test)] mod tests { use super::*; - use crate::types::{IntoPyDict, PyDict, PyList}; - use crate::Py; + use crate::types::{any::PyAnyMethods, IntoPyDict, PyDict, PyList}; use std::sync::Arc; #[test] @@ -1150,17 +1149,14 @@ mod tests { // If allow_threads is implemented correctly, this thread still owns the GIL here // so the following Python calls should not cause crashes. - let list = PyList::new(py, [1, 2, 3, 4]); + let list = PyList::new_bound(py, [1, 2, 3, 4]); assert_eq!(list.extract::>().unwrap(), vec![1, 2, 3, 4]); }); } #[test] fn test_allow_threads_pass_stuff_in() { - let list: Py = Python::with_gil(|py| { - let list = PyList::new(py, vec!["foo", "bar"]); - list.into() - }); + let list = Python::with_gil(|py| PyList::new_bound(py, vec!["foo", "bar"]).unbind()); let mut v = vec![1, 2, 3]; let a = Arc::new(String::from("foo")); diff --git a/src/tests/hygiene/pymethods.rs b/src/tests/hygiene/pymethods.rs index 8e5bce8e..51657185 100644 --- a/src/tests/hygiene/pymethods.rs +++ b/src/tests/hygiene/pymethods.rs @@ -76,8 +76,8 @@ impl Dummy { fn __delattr__(&mut self, name: ::std::string::String) {} - fn __dir__<'py>(&self, py: crate::Python<'py>) -> &'py crate::types::PyList { - crate::types::PyList::new(py, ::std::vec![0_u8]) + fn __dir__<'py>(&self, py: crate::Python<'py>) -> crate::Bound<'py, crate::types::PyList> { + crate::types::PyList::new_bound(py, ::std::vec![0_u8]) } ////////////////////// @@ -469,8 +469,8 @@ impl Dummy { fn __delattr__(&mut self, name: ::std::string::String) {} - fn __dir__<'py>(&self, py: crate::Python<'py>) -> &'py crate::types::PyList { - crate::types::PyList::new(py, ::std::vec![0_u8]) + fn __dir__<'py>(&self, py: crate::Python<'py>) -> crate::Bound<'py, crate::types::PyList> { + crate::types::PyList::new_bound(py, ::std::vec![0_u8]) } ////////////////////// diff --git a/src/types/dict.rs b/src/types/dict.rs index 0fe48662..898c980f 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -697,6 +697,7 @@ where } #[cfg(test)] +#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] mod tests { use super::*; #[cfg(not(PyPy))] diff --git a/src/types/list.rs b/src/types/list.rs index f8db0e62..140d1e20 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -56,6 +56,24 @@ pub(crate) fn new_from_iter<'py>( } impl PyList { + /// Deprecated form of [`PyList::new_bound`]. + #[inline] + #[track_caller] + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`PyList::new` will be replaced by `PyList::new_bound` in a future PyO3 version" + ) + )] + pub fn new(py: Python<'_>, elements: impl IntoIterator) -> &PyList + where + T: ToPyObject, + U: ExactSizeIterator, + { + Self::new_bound(py, elements).into_gil_ref() + } + /// Constructs a new list with the given elements. /// /// If you want to create a [`PyList`] with elements of different or unknown types, or from an @@ -82,18 +100,38 @@ impl PyList { /// All standard library structures implement this trait correctly, if they do, so calling this /// function with (for example) [`Vec`]`` or `&[T]` will always succeed. #[track_caller] - pub fn new(py: Python<'_>, elements: impl IntoIterator) -> &PyList + pub fn new_bound( + py: Python<'_>, + elements: impl IntoIterator, + ) -> Bound<'_, PyList> where T: ToPyObject, U: ExactSizeIterator, { let mut iter = elements.into_iter().map(|e| e.to_object(py)); - new_from_iter(py, &mut iter).into_gil_ref() + new_from_iter(py, &mut iter) + } + + /// Deprecated form of [`PyList::empty_bound`]. + #[inline] + #[cfg_attr( + not(feature = "gil-refs"), + deprecated( + since = "0.21.0", + note = "`PyList::empty` will be replaced by `PyList::empty_bound` in a future PyO3 version" + ) + )] + pub fn empty(py: Python<'_>) -> &PyList { + Self::empty_bound(py).into_gil_ref() } /// Constructs a new empty list. - pub fn empty(py: Python<'_>) -> &PyList { - unsafe { py.from_owned_ptr(ffi::PyList_New(0)) } + pub fn empty_bound(py: Python<'_>) -> Bound<'_, PyList> { + unsafe { + ffi::PyList_New(0) + .assume_owned(py) + .downcast_into_unchecked() + } } /// Returns the length of the list. @@ -667,6 +705,7 @@ impl<'py> IntoIterator for Bound<'py, PyList> { } #[cfg(test)] +#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] mod tests { use crate::types::{PyList, PyTuple}; use crate::Python; diff --git a/src/types/module.rs b/src/types/module.rs index aa6649a9..416090a1 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -560,7 +560,7 @@ impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> { Ok(idx) => idx.downcast_into().map_err(PyErr::from), Err(err) => { if err.is_instance_of::(self.py()) { - let l = PyList::empty(self.py()).as_borrowed().to_owned(); + let l = PyList::empty_bound(self.py()); self.setattr(__all__, &l).map_err(PyErr::from)?; Ok(l) } else { diff --git a/src/types/sequence.rs b/src/types/sequence.rs index fa15fe06..64e8f316 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -566,6 +566,7 @@ impl<'v> crate::PyTryFrom<'v> for PySequence { } #[cfg(test)] +#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))] mod tests { use crate::types::{PyList, PySequence, PyTuple}; use crate::{PyObject, Python, ToPyObject}; diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index aa0c1cef..8eea9b89 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -564,7 +564,8 @@ pub struct TransparentFromPyWith { #[test] fn test_transparent_from_py_with() { Python::with_gil(|py| { - let result = TransparentFromPyWith::extract(PyList::new(py, [1, 2, 3])).unwrap(); + let result = + TransparentFromPyWith::extract(PyList::new_bound(py, [1, 2, 3]).as_gil_ref()).unwrap(); let expected = TransparentFromPyWith { len: 3 }; assert_eq!(result, expected); diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index f16ce611..148380ec 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -42,8 +42,8 @@ impl ClassWithProperties { } #[getter] - fn get_data_list<'py>(&self, py: Python<'py>) -> &'py PyList { - PyList::new(py, [self.num]) + fn get_data_list<'py>(&self, py: Python<'py>) -> Bound<'py, PyList> { + PyList::new_bound(py, [self.num]) } } diff --git a/tests/test_methods.rs b/tests/test_methods.rs index 6e42ad66..50b179fd 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -689,12 +689,12 @@ struct MethodWithLifeTime {} #[pymethods] impl MethodWithLifeTime { - fn set_to_list<'py>(&self, py: Python<'py>, set: &'py PySet) -> PyResult<&'py PyList> { + fn set_to_list<'py>(&self, py: Python<'py>, set: &'py PySet) -> PyResult> { let mut items = vec![]; for _ in 0..set.len() { items.push(set.pop().unwrap()); } - let list = PyList::new(py, items); + let list = PyList::new_bound(py, items); list.sort()?; Ok(list) } diff --git a/tests/test_proto_methods.rs b/tests/test_proto_methods.rs index caaae751..a328ab9e 100644 --- a/tests/test_proto_methods.rs +++ b/tests/test_proto_methods.rs @@ -850,7 +850,7 @@ struct DefaultedContains; #[pymethods] impl DefaultedContains { fn __iter__(&self, py: Python<'_>) -> PyObject { - PyList::new(py, ["a", "b", "c"]) + PyList::new_bound(py, ["a", "b", "c"]) .as_ref() .iter() .unwrap() @@ -864,7 +864,7 @@ struct NoContains; #[pymethods] impl NoContains { fn __iter__(&self, py: Python<'_>) -> PyObject { - PyList::new(py, ["a", "b", "c"]) + PyList::new_bound(py, ["a", "b", "c"]) .as_ref() .iter() .unwrap()