From 3396bfee3ba792f4d1e6d6c8a970b7aec251bdab Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 8 Sep 2024 17:42:48 +0200 Subject: [PATCH 1/3] migrate `PyList::new` --- guide/src/conversions/traits.md | 2 +- guide/src/exception.md | 7 +- guide/src/trait-bounds.md | 10 +-- guide/src/types.md | 2 +- pyo3-benches/benches/bench_list.rs | 8 +- src/conversion.rs | 4 +- src/conversions/smallvec.rs | 6 +- src/macros.rs | 7 +- src/marker.rs | 4 +- src/tests/hygiene/pymethods.rs | 5 +- src/types/any.rs | 2 +- src/types/dict.rs | 4 +- src/types/list.rs | 132 ++++++++++++++--------------- src/types/sequence.rs | 8 +- src/types/tuple.rs | 2 +- tests/test_frompyobject.rs | 1 + tests/test_getter_setter.rs | 2 +- tests/test_methods.rs | 2 +- tests/test_proto_methods.rs | 2 + 19 files changed, 113 insertions(+), 97 deletions(-) diff --git a/guide/src/conversions/traits.md b/guide/src/conversions/traits.md index 7f61616eefe..631acdb2bf3 100644 --- a/guide/src/conversions/traits.md +++ b/guide/src/conversions/traits.md @@ -13,7 +13,7 @@ fails, so usually you will use something like # use pyo3::types::PyList; # fn main() -> PyResult<()> { # Python::with_gil(|py| { -# let list = PyList::new(py, b"foo"); +# let list = PyList::new(py, b"foo")?; let v: Vec = list.extract()?; # assert_eq!(&v, &[102, 111, 111]); # Ok(()) diff --git a/guide/src/exception.md b/guide/src/exception.md index 32a56078af5..4a1f1425d72 100644 --- a/guide/src/exception.md +++ b/guide/src/exception.md @@ -78,12 +78,15 @@ In PyO3 every object has the [`PyAny::is_instance`] and [`PyAny::is_instance_of` use pyo3::prelude::*; use pyo3::types::{PyBool, PyList}; +# fn main() -> PyResult<()> { Python::with_gil(|py| { assert!(PyBool::new(py, true).is_instance_of::()); - let list = PyList::new(py, &[1, 2, 3, 4]); + let list = PyList::new(py, &[1, 2, 3, 4])?; assert!(!list.is_instance_of::()); assert!(list.is_instance_of::()); -}); +# Ok(()) +}) +# } ``` To check the type of an exception, you can similarly do: diff --git a/guide/src/trait-bounds.md b/guide/src/trait-bounds.md index 434ac7dd968..e1b8e82f1db 100644 --- a/guide/src/trait-bounds.md +++ b/guide/src/trait-bounds.md @@ -84,7 +84,7 @@ impl Model for UserModel { Python::with_gil(|py| { self.model .bind(py) - .call_method("set_variables", (PyList::new(py, var),), None) + .call_method("set_variables", (PyList::new(py, var).unwrap(),), None) .unwrap(); }) } @@ -182,7 +182,7 @@ This wrapper will also perform the type conversions between Python and Rust. # println!("Rust calling Python to set the variables"); # Python::with_gil(|py| { # self.model.bind(py) -# .call_method("set_variables", (PyList::new(py, var),), None) +# .call_method("set_variables", (PyList::new(py, var).unwrap(),), None) # .unwrap(); # }) # } @@ -360,7 +360,7 @@ impl Model for UserModel { # println!("Rust calling Python to set the variables"); # Python::with_gil(|py| { # self.model.bind(py) -# .call_method("set_variables", (PyList::new(py, var),), None) +# .call_method("set_variables", (PyList::new(py, var).unwrap(),), None) # .unwrap(); # }) # } @@ -419,7 +419,7 @@ impl Model for UserModel { # println!("Rust calling Python to set the variables"); # Python::with_gil(|py| { # let py_model = self.model.bind(py) -# .call_method("set_variables", (PyList::new(py, var),), None) +# .call_method("set_variables", (PyList::new(py, var).unwrap(),), None) # .unwrap(); # }) # } @@ -517,7 +517,7 @@ impl Model for UserModel { Python::with_gil(|py| { self.model .bind(py) - .call_method("set_variables", (PyList::new(py, var),), None) + .call_method("set_variables", (PyList::new(py, var).unwrap(),), None) .unwrap(); }) } diff --git a/guide/src/types.md b/guide/src/types.md index bd47c127b60..df4d6b4812d 100644 --- a/guide/src/types.md +++ b/guide/src/types.md @@ -232,7 +232,7 @@ fn get_first_item<'py>(list: &Bound<'py, PyList>) -> PyResult> list.get_item(0) } # Python::with_gil(|py| { -# let l = PyList::new(py, ["hello world"]); +# let l = PyList::new(py, ["hello world"]).unwrap(); # assert!(get_first_item(&l).unwrap().eq("hello world").unwrap()); # }) ``` diff --git a/pyo3-benches/benches/bench_list.rs b/pyo3-benches/benches/bench_list.rs index 4f6374d75eb..39829f52ce8 100644 --- a/pyo3-benches/benches/bench_list.rs +++ b/pyo3-benches/benches/bench_list.rs @@ -8,7 +8,7 @@ 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(py, 0..LEN).unwrap(); let mut sum = 0; b.iter(|| { for x in &list { @@ -29,7 +29,7 @@ fn list_new(b: &mut Bencher<'_>) { 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(py, 0..LEN).unwrap(); let mut sum = 0; b.iter(|| { for i in 0..LEN { @@ -43,7 +43,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(py, 0..LEN).unwrap(); let mut sum = 0; b.iter(|| { for i in 0..LEN { @@ -58,7 +58,7 @@ 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); + let list = &PyList::new(py, 0..LEN).unwrap(); b.iter(|| black_box(list).downcast::().unwrap()); }); } diff --git a/src/conversion.rs b/src/conversion.rs index c74c529bbb8..3cc73072ed9 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -304,7 +304,7 @@ pub trait IntoPyObject<'py>: Sized { let mut iter = iter.into_iter().map(|e| { e.into_pyobject(py) .map(BoundObject::into_any) - .map(BoundObject::unbind) + .map(BoundObject::into_bound) .map_err(Into::into) }); let list = crate::types::list::try_new_from_iter(py, &mut iter); @@ -327,7 +327,7 @@ pub trait IntoPyObject<'py>: Sized { let mut iter = iter.into_iter().map(|e| { e.into_pyobject(py) .map(BoundObject::into_any) - .map(BoundObject::unbind) + .map(BoundObject::into_bound) .map_err(Into::into) }); let list = crate::types::list::try_new_from_iter(py, &mut iter); diff --git a/src/conversions/smallvec.rs b/src/conversions/smallvec.rs index 8c13c7e8299..be90113344e 100644 --- a/src/conversions/smallvec.rs +++ b/src/conversions/smallvec.rs @@ -140,7 +140,7 @@ mod tests { 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(py, [1, 2, 3, 4, 5]).unwrap(); assert!(l.eq(hso).unwrap()); }); } @@ -148,7 +148,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(py, [1, 2, 3, 4, 5]).unwrap(); let sv: SmallVec<[u64; 8]> = l.extract().unwrap(); assert_eq!(sv.as_slice(), [1, 2, 3, 4, 5]); }); @@ -171,7 +171,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(py, [1, 2, 3, 4, 5]).unwrap(); assert!(l.eq(hso).unwrap()); }); } diff --git a/src/macros.rs b/src/macros.rs index e4ae4c9dc16..6148d9662c5 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -11,10 +11,13 @@ /// ``` /// use pyo3::{prelude::*, py_run, types::PyList}; /// +/// # fn main() -> PyResult<()> { /// Python::with_gil(|py| { -/// let list = PyList::new(py, &[1, 2, 3]); +/// let list = PyList::new(py, &[1, 2, 3])?; /// py_run!(py, list, "assert list == [1, 2, 3]"); -/// }); +/// # Ok(()) +/// }) +/// # } /// ``` /// /// You can use this macro to test pyfunctions or pyclasses quickly. diff --git a/src/marker.rs b/src/marker.rs index 8af307621a8..92a154b3694 100644 --- a/src/marker.rs +++ b/src/marker.rs @@ -939,7 +939,7 @@ 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(py, [1, 2, 3, 4]).unwrap(); assert_eq!(list.extract::>().unwrap(), vec![1, 2, 3, 4]); }); } @@ -947,7 +947,7 @@ mod tests { #[cfg(not(pyo3_disable_reference_pool))] #[test] fn test_allow_threads_pass_stuff_in() { - let list = Python::with_gil(|py| PyList::new(py, vec!["foo", "bar"]).unbind()); + let list = Python::with_gil(|py| PyList::new(py, vec!["foo", "bar"]).unwrap().unbind()); let mut v = vec![1, 2, 3]; let a = std::sync::Arc::new(String::from("foo")); diff --git a/src/tests/hygiene/pymethods.rs b/src/tests/hygiene/pymethods.rs index 6a1a2a50d13..5c027a5c3ae 100644 --- a/src/tests/hygiene/pymethods.rs +++ b/src/tests/hygiene/pymethods.rs @@ -72,7 +72,10 @@ impl Dummy { fn __delattr__(&mut self, name: ::std::string::String) {} - fn __dir__<'py>(&self, py: crate::Python<'py>) -> crate::Bound<'py, crate::types::PyList> { + fn __dir__<'py>( + &self, + py: crate::Python<'py>, + ) -> crate::PyResult> { crate::types::PyList::new(py, ::std::vec![0_u8]) } diff --git a/src/types/any.rs b/src/types/any.rs index 5e40a0fdc4b..37b79720730 100644 --- a/src/types/any.rs +++ b/src/types/any.rs @@ -2021,7 +2021,7 @@ class SimpleClass: let empty_list = PyList::empty(py).into_any(); assert!(empty_list.is_empty().unwrap()); - let list = PyList::new(py, vec![1, 2, 3]).into_any(); + let list = PyList::new(py, vec![1, 2, 3]).unwrap().into_any(); assert!(!list.is_empty().unwrap()); let not_container = 5.to_object(py).into_bound(py); diff --git a/src/types/dict.rs b/src/types/dict.rs index 1be0ed22362..2136158e89e 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -629,7 +629,7 @@ mod tests { #[cfg(not(any(PyPy, GraalPy)))] fn test_from_sequence() { Python::with_gil(|py| { - let items = PyList::new(py, vec![("a", 1), ("b", 2)]); + let items = PyList::new(py, vec![("a", 1), ("b", 2)]).unwrap(); let dict = PyDict::from_sequence(&items).unwrap(); assert_eq!( 1, @@ -660,7 +660,7 @@ mod tests { #[cfg(not(any(PyPy, GraalPy)))] fn test_from_sequence_err() { Python::with_gil(|py| { - let items = PyList::new(py, vec!["a", "b"]); + let items = PyList::new(py, vec!["a", "b"]).unwrap(); assert!(PyDict::from_sequence(&items).is_err()); }); } diff --git a/src/types/list.rs b/src/types/list.rs index 9c6f8ff21c3..a50a78bd2e5 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -25,18 +25,18 @@ pyobject_native_type_core!(PyList, pyobject_native_static_type_object!(ffi::PyLi #[inline] #[track_caller] -pub(crate) fn new_from_iter<'py>( - py: Python<'py>, - elements: &mut dyn ExactSizeIterator, -) -> Bound<'py, PyList> { - try_new_from_iter(py, &mut elements.map(Ok)).unwrap() +pub(crate) fn new_from_iter( + py: Python<'_>, + elements: impl ExactSizeIterator, +) -> Bound<'_, PyList> { + try_new_from_iter(py, elements.map(|e| e.into_bound(py)).map(Ok)).unwrap() } #[inline] #[track_caller] pub(crate) fn try_new_from_iter<'py>( py: Python<'py>, - elements: &mut dyn ExactSizeIterator>, + mut elements: impl ExactSizeIterator>>, ) -> PyResult> { unsafe { // PyList_New checks for overflow but has a bad error message, so we check ourselves @@ -54,7 +54,7 @@ pub(crate) fn try_new_from_iter<'py>( let mut counter: Py_ssize_t = 0; - for obj in elements.take(len as usize) { + for obj in (&mut elements).take(len as usize) { #[cfg(not(Py_LIMITED_API))] ffi::PyList_SET_ITEM(ptr, counter, obj?.into_ptr()); #[cfg(Py_LIMITED_API)] @@ -81,12 +81,13 @@ impl PyList { /// use pyo3::prelude::*; /// use pyo3::types::PyList; /// - /// # fn main() { + /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| { /// let elements: Vec = vec![0, 1, 2, 3, 4, 5]; - /// let list = PyList::new(py, elements); + /// let list = PyList::new(py, elements)?; /// assert_eq!(format!("{:?}", list), "[0, 1, 2, 3, 4, 5]"); - /// }); + /// # Ok(()) + /// }) /// # } /// ``` /// @@ -96,16 +97,21 @@ 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<'_>, + pub fn new<'py, T, U>( + py: Python<'py>, elements: impl IntoIterator, - ) -> Bound<'_, PyList> + ) -> PyResult> where - T: ToPyObject, + T: IntoPyObject<'py>, U: ExactSizeIterator, { - let mut iter = elements.into_iter().map(|e| e.to_object(py)); - new_from_iter(py, &mut iter) + let iter = elements.into_iter().map(|e| { + e.into_pyobject(py) + .map(BoundObject::into_any) + .map(BoundObject::into_bound) + .map_err(Into::into) + }); + try_new_from_iter(py, iter) } /// Deprecated name for [`PyList::new`]. @@ -120,7 +126,7 @@ impl PyList { T: ToPyObject, U: ExactSizeIterator, { - Self::new(py, elements) + Self::new(py, elements.into_iter().map(|e| e.to_object(py))).unwrap() } /// Constructs a new empty list. @@ -164,7 +170,7 @@ pub trait PyListMethods<'py>: crate::sealed::Sealed { /// ``` /// use pyo3::{prelude::*, types::PyList}; /// Python::with_gil(|py| { - /// let list = PyList::new(py, [2, 3, 5, 7]); + /// let list = PyList::new(py, [2, 3, 5, 7]).unwrap(); /// let obj = list.get_item(0); /// assert_eq!(obj.unwrap().extract::().unwrap(), 2); /// }); @@ -282,7 +288,7 @@ impl<'py> PyListMethods<'py> for Bound<'py, PyList> { /// ``` /// use pyo3::{prelude::*, types::PyList}; /// Python::with_gil(|py| { - /// let list = PyList::new(py, [2, 3, 5, 7]); + /// let list = PyList::new(py, [2, 3, 5, 7]).unwrap(); /// let obj = list.get_item(0); /// assert_eq!(obj.unwrap().extract::().unwrap(), 2); /// }); @@ -573,7 +579,7 @@ mod tests { #[test] fn test_new() { Python::with_gil(|py| { - let list = PyList::new(py, [2, 3, 5, 7]); + let list = PyList::new(py, [2, 3, 5, 7]).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()); @@ -584,7 +590,7 @@ mod tests { #[test] fn test_len() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 2, 3, 4]); + let list = PyList::new(py, [1, 2, 3, 4]).unwrap(); assert_eq!(4, list.len()); }); } @@ -592,7 +598,7 @@ mod tests { #[test] fn test_get_item() { Python::with_gil(|py| { - let list = PyList::new(py, [2, 3, 5, 7]); + let list = PyList::new(py, [2, 3, 5, 7]).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()); @@ -603,7 +609,7 @@ mod tests { #[test] fn test_get_slice() { Python::with_gil(|py| { - let list = PyList::new(py, [2, 3, 5, 7]); + let list = PyList::new(py, [2, 3, 5, 7]).unwrap(); let slice = list.get_slice(1, 3); assert_eq!(2, slice.len()); let slice = list.get_slice(1, 7); @@ -614,7 +620,7 @@ mod tests { #[test] fn test_set_item() { Python::with_gil(|py| { - let list = PyList::new(py, [2, 3, 5, 7]); + let list = PyList::new(py, [2, 3, 5, 7]).unwrap(); let val = 42i32.into_pyobject(py).unwrap(); let val2 = 42i32.into_pyobject(py).unwrap(); assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); @@ -644,7 +650,7 @@ mod tests { #[test] fn test_insert() { Python::with_gil(|py| { - let list = PyList::new(py, [2, 3, 5, 7]); + let list = PyList::new(py, [2, 3, 5, 7]).unwrap(); let val = 42i32.into_pyobject(py).unwrap(); let val2 = 43i32.into_pyobject(py).unwrap(); assert_eq!(4, list.len()); @@ -676,7 +682,7 @@ mod tests { #[test] fn test_append() { Python::with_gil(|py| { - let list = PyList::new(py, [2]); + let list = PyList::new(py, [2]).unwrap(); list.append(3).unwrap(); assert_eq!(2, list.get_item(0).unwrap().extract::().unwrap()); assert_eq!(3, list.get_item(1).unwrap().extract::().unwrap()); @@ -701,7 +707,7 @@ mod tests { fn test_iter() { Python::with_gil(|py| { let v = vec![2, 3, 5, 7]; - let list = PyList::new(py, &v); + let list = PyList::new(py, &v).unwrap(); let mut idx = 0; for el in list { assert_eq!(v[idx], el.extract::().unwrap()); @@ -761,7 +767,7 @@ mod tests { #[test] fn test_into_iter() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 2, 3, 4]); + let list = PyList::new(py, [1, 2, 3, 4]).unwrap(); for (i, item) in list.iter().enumerate() { assert_eq!((i + 1) as i32, item.extract::().unwrap()); } @@ -773,7 +779,7 @@ mod tests { use crate::types::any::PyAnyMethods; Python::with_gil(|py| { - let list = PyList::new(py, [1, 2, 3, 4]); + let list = PyList::new(py, [1, 2, 3, 4]).unwrap(); let mut items = vec![]; for item in &list { items.push(item.extract::().unwrap()); @@ -785,7 +791,7 @@ mod tests { #[test] fn test_as_sequence() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 2, 3, 4]); + let list = PyList::new(py, [1, 2, 3, 4]).unwrap(); assert_eq!(list.as_sequence().len().unwrap(), 4); assert_eq!( @@ -802,7 +808,7 @@ mod tests { #[test] fn test_into_sequence() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 2, 3, 4]); + let list = PyList::new(py, [1, 2, 3, 4]).unwrap(); let sequence = list.into_sequence(); @@ -815,7 +821,7 @@ mod tests { fn test_extract() { Python::with_gil(|py| { let v = vec![2, 3, 5, 7]; - let list = PyList::new(py, &v); + let list = PyList::new(py, &v).unwrap(); let v2 = list.as_ref().extract::>().unwrap(); assert_eq!(v, v2); }); @@ -825,7 +831,7 @@ mod tests { fn test_sort() { Python::with_gil(|py| { let v = vec![7, 3, 2, 5]; - let list = PyList::new(py, &v); + let list = PyList::new(py, &v).unwrap(); 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()); @@ -842,7 +848,7 @@ mod tests { fn test_reverse() { Python::with_gil(|py| { let v = vec![2, 3, 5, 7]; - let list = PyList::new(py, &v); + let list = PyList::new(py, &v).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()); @@ -868,7 +874,7 @@ mod tests { #[test] fn test_list_get_item_invalid_index() { Python::with_gil(|py| { - let list = PyList::new(py, [2, 3, 5, 7]); + let list = PyList::new(py, [2, 3, 5, 7]).unwrap(); let obj = list.get_item(5); assert!(obj.is_err()); assert_eq!( @@ -881,7 +887,7 @@ mod tests { #[test] fn test_list_get_item_sanity() { Python::with_gil(|py| { - let list = PyList::new(py, [2, 3, 5, 7]); + let list = PyList::new(py, [2, 3, 5, 7]).unwrap(); let obj = list.get_item(0); assert_eq!(obj.unwrap().extract::().unwrap(), 2); }); @@ -891,7 +897,7 @@ mod tests { #[test] fn test_list_get_item_unchecked_sanity() { Python::with_gil(|py| { - let list = PyList::new(py, [2, 3, 5, 7]); + let list = PyList::new(py, [2, 3, 5, 7]).unwrap(); let obj = unsafe { list.get_item_unchecked(0) }; assert_eq!(obj.extract::().unwrap(), 2); }); @@ -900,7 +906,7 @@ mod tests { #[test] fn test_list_del_item() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 1, 2, 3, 5, 8]); + let list = PyList::new(py, [1, 1, 2, 3, 5, 8]).unwrap(); assert!(list.del_item(10).is_err()); assert_eq!(1, list.get_item(0).unwrap().extract::().unwrap()); assert!(list.del_item(0).is_ok()); @@ -922,8 +928,8 @@ mod tests { #[test] fn test_list_set_slice() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 1, 2, 3, 5, 8]); - let ins = PyList::new(py, [7, 4]); + let list = PyList::new(py, [1, 1, 2, 3, 5, 8]).unwrap(); + let ins = PyList::new(py, [7, 4]).unwrap(); list.set_slice(1, 4, &ins).unwrap(); assert_eq!([1, 7, 4, 5, 8], list.extract::<[i32; 5]>().unwrap()); list.set_slice(3, 100, &PyList::empty(py)).unwrap(); @@ -934,7 +940,7 @@ mod tests { #[test] fn test_list_del_slice() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 1, 2, 3, 5, 8]); + let list = PyList::new(py, [1, 1, 2, 3, 5, 8]).unwrap(); list.del_slice(1, 4).unwrap(); assert_eq!([1, 5, 8], list.extract::<[i32; 3]>().unwrap()); list.del_slice(1, 100).unwrap(); @@ -945,7 +951,7 @@ mod tests { #[test] fn test_list_contains() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 1, 2, 3, 5, 8]); + let list = PyList::new(py, [1, 1, 2, 3, 5, 8]).unwrap(); assert_eq!(6, list.len()); let bad_needle = 7i32.into_pyobject(py).unwrap(); @@ -962,7 +968,7 @@ mod tests { #[test] fn test_list_index() { Python::with_gil(|py| { - let list = PyList::new(py, [1, 1, 2, 3, 5, 8]); + let list = PyList::new(py, [1, 1, 2, 3, 5, 8]).unwrap(); assert_eq!(0, list.index(1i32).unwrap()); assert_eq!(2, list.index(2i32).unwrap()); assert_eq!(3, list.index(3i32).unwrap()); @@ -1000,7 +1006,7 @@ mod tests { fn too_long_iterator() { Python::with_gil(|py| { let iter = FaultyIter(0..usize::MAX, 73); - let _list = PyList::new(py, iter); + let _list = PyList::new(py, iter).unwrap(); }) } @@ -1011,7 +1017,7 @@ mod tests { fn too_short_iterator() { Python::with_gil(|py| { let iter = FaultyIter(0..35, 73); - let _list = PyList::new(py, iter); + let _list = PyList::new(py, iter).unwrap(); }) } @@ -1023,40 +1029,34 @@ mod tests { Python::with_gil(|py| { let iter = FaultyIter(0..0, usize::MAX); - let _list = PyList::new(py, iter); + let _list = PyList::new(py, iter).unwrap(); }) } - #[cfg(feature = "macros")] #[test] - fn bad_clone_mem_leaks() { - use crate::{IntoPy, Py, PyAny, ToPyObject}; + fn bad_intopyobject_doesnt_cause_leaks() { + use crate::types::PyInt; + use std::convert::Infallible; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; static NEEDS_DESTRUCTING_COUNT: AtomicUsize = AtomicUsize::new(0); - #[crate::pyclass] - #[pyo3(crate = "crate")] struct Bad(usize); - impl Clone for Bad { - fn clone(&self) -> Self { - // This panic should not lead to a memory leak - assert_ne!(self.0, 42); - NEEDS_DESTRUCTING_COUNT.fetch_add(1, SeqCst); - - Bad(self.0) - } - } - impl Drop for Bad { fn drop(&mut self) { NEEDS_DESTRUCTING_COUNT.fetch_sub(1, SeqCst); } } - impl ToPyObject for Bad { - fn to_object(&self, py: Python<'_>) -> Py { - self.to_owned().into_py(py) + impl<'py> IntoPyObject<'py> for Bad { + type Target = PyInt; + type Output = crate::Bound<'py, Self::Target>; + type Error = Infallible; + + fn into_pyobject(self, py: Python<'py>) -> Result { + // This panic should not lead to a memory leak + assert_ne!(self.0, 42); + self.0.into_pyobject(py) } } @@ -1082,7 +1082,7 @@ mod tests { Python::with_gil(|py| { std::panic::catch_unwind(|| { let iter = FaultyIter(0..50, 50); - let _list = PyList::new(py, iter); + let _list = PyList::new(py, iter).unwrap(); }) .unwrap_err(); }); @@ -1097,7 +1097,7 @@ mod tests { #[test] fn test_list_to_tuple() { Python::with_gil(|py| { - let list = PyList::new(py, vec![1, 2, 3]); + let list = PyList::new(py, vec![1, 2, 3]).unwrap(); let tuple = list.to_tuple(); let tuple_expected = PyTuple::new(py, vec![1, 2, 3]); assert!(tuple.eq(tuple_expected).unwrap()); diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 2fca2b18a51..ae337ae7583 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -746,7 +746,11 @@ mod tests { let v = vec!["foo", "bar"]; let ob = (&v).into_pyobject(py).unwrap(); let seq = ob.downcast::().unwrap(); - assert!(seq.to_list().unwrap().eq(PyList::new(py, &v)).unwrap()); + assert!(seq + .to_list() + .unwrap() + .eq(PyList::new(py, &v).unwrap()) + .unwrap()); }); } @@ -759,7 +763,7 @@ mod tests { assert!(seq .to_list() .unwrap() - .eq(PyList::new(py, ["f", "o", "o"])) + .eq(PyList::new(py, ["f", "o", "o"]).unwrap()) .unwrap()); }); } diff --git a/src/types/tuple.rs b/src/types/tuple.rs index 642a5c2d055..265ce27708a 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -1299,7 +1299,7 @@ mod tests { Python::with_gil(|py| { let tuple = PyTuple::new(py, vec![1, 2, 3]); let list = tuple.to_list(); - let list_expected = PyList::new(py, vec![1, 2, 3]); + let list_expected = PyList::new(py, vec![1, 2, 3]).unwrap(); assert!(list.eq(list_expected).unwrap()); }) } diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index 96f34ffd5b6..f7699d305b2 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -599,6 +599,7 @@ pub struct TransparentFromPyWith { fn test_transparent_from_py_with() { Python::with_gil(|py| { let result = PyList::new(py, [1, 2, 3]) + .unwrap() .extract::() .unwrap(); let expected = TransparentFromPyWith { len: 3 }; diff --git a/tests/test_getter_setter.rs b/tests/test_getter_setter.rs index dc67c040fc5..8966471abe2 100644 --- a/tests/test_getter_setter.rs +++ b/tests/test_getter_setter.rs @@ -53,7 +53,7 @@ impl ClassWithProperties { } #[getter] - fn get_data_list<'py>(&self, py: Python<'py>) -> Bound<'py, PyList> { + fn get_data_list<'py>(&self, py: Python<'py>) -> PyResult> { PyList::new(py, [self.num]) } } diff --git a/tests/test_methods.rs b/tests/test_methods.rs index ecf7f64e94c..2a9ffbec788 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -703,7 +703,7 @@ impl MethodWithLifeTime { for _ in 0..set.len() { items.push(set.pop().unwrap()); } - let list = PyList::new(py, items); + let list = PyList::new(py, items)?; list.sort()?; Ok(list) } diff --git a/tests/test_proto_methods.rs b/tests/test_proto_methods.rs index 3cca16151aa..9fa6a8b888e 100644 --- a/tests/test_proto_methods.rs +++ b/tests/test_proto_methods.rs @@ -851,6 +851,7 @@ struct DefaultedContains; impl DefaultedContains { fn __iter__(&self, py: Python<'_>) -> PyObject { PyList::new(py, ["a", "b", "c"]) + .unwrap() .as_ref() .iter() .unwrap() @@ -865,6 +866,7 @@ struct NoContains; impl NoContains { fn __iter__(&self, py: Python<'_>) -> PyObject { PyList::new(py, ["a", "b", "c"]) + .unwrap() .as_ref() .iter() .unwrap() From 9206dc5c4c8719dec333d10e63f26208aaab8e13 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Sun, 8 Sep 2024 18:19:19 +0200 Subject: [PATCH 2/3] migrate `PyTuple::new` --- guide/src/conversions/traits.md | 4 +- guide/src/python-from-rust/function-calls.md | 2 +- guide/src/types.md | 4 +- pytests/src/datetime.rs | 12 +- src/impl_/extract_argument.rs | 2 +- src/impl_/pymodule.rs | 2 +- src/instance.rs | 2 +- src/types/datetime.rs | 2 +- src/types/list.rs | 2 +- src/types/sequence.rs | 8 +- src/types/tuple.rs | 133 ++++++++++--------- src/types/typeobject.rs | 5 +- tests/test_frompyobject.rs | 20 ++- tests/test_various.rs | 5 +- tests/ui/invalid_property_args.stderr | 14 +- tests/ui/missing_intopy.stderr | 14 +- 16 files changed, 129 insertions(+), 102 deletions(-) diff --git a/guide/src/conversions/traits.md b/guide/src/conversions/traits.md index 631acdb2bf3..fd9e90097f2 100644 --- a/guide/src/conversions/traits.md +++ b/guide/src/conversions/traits.md @@ -183,7 +183,7 @@ struct RustyTuple(String, String); # use pyo3::types::PyTuple; # fn main() -> PyResult<()> { # Python::with_gil(|py| -> PyResult<()> { -# let tuple = PyTuple::new(py, vec!["test", "test2"]); +# let tuple = PyTuple::new(py, vec!["test", "test2"])?; # # let rustytuple: RustyTuple = tuple.extract()?; # assert_eq!(rustytuple.0, "test"); @@ -206,7 +206,7 @@ struct RustyTuple((String,)); # use pyo3::types::PyTuple; # fn main() -> PyResult<()> { # Python::with_gil(|py| -> PyResult<()> { -# let tuple = PyTuple::new(py, vec!["test"]); +# let tuple = PyTuple::new(py, vec!["test"])?; # # let rustytuple: RustyTuple = tuple.extract()?; # assert_eq!((rustytuple.0).0, "test"); diff --git a/guide/src/python-from-rust/function-calls.md b/guide/src/python-from-rust/function-calls.md index 12544dc02bd..f1eb8025431 100644 --- a/guide/src/python-from-rust/function-calls.md +++ b/guide/src/python-from-rust/function-calls.md @@ -50,7 +50,7 @@ fn main() -> PyResult<()> { fun.call1(py, args)?; // call object with Python tuple of positional arguments - let args = PyTuple::new(py, &[arg1, arg2, arg3]); + let args = PyTuple::new(py, &[arg1, arg2, arg3])?; fun.call1(py, args)?; Ok(()) }) diff --git a/guide/src/types.md b/guide/src/types.md index df4d6b4812d..06559d49d13 100644 --- a/guide/src/types.md +++ b/guide/src/types.md @@ -145,7 +145,7 @@ use pyo3::types::PyTuple; # fn example<'py>(py: Python<'py>) -> PyResult<()> { // Create a new tuple with the elements (0, 1, 2) -let t = PyTuple::new(py, [0, 1, 2]); +let t = PyTuple::new(py, [0, 1, 2])?; for i in 0..=2 { let entry: Borrowed<'_, 'py, PyAny> = t.get_borrowed_item(i)?; // `PyAnyMethods::extract` is available on `Borrowed` @@ -295,7 +295,7 @@ For example, the following snippet extracts a Rust tuple of integers from a Pyth # use pyo3::types::PyTuple; # fn example<'py>(py: Python<'py>) -> PyResult<()> { // create a new Python `tuple`, and use `.into_any()` to erase the type -let obj: Bound<'py, PyAny> = PyTuple::new(py, [1, 2, 3]).into_any(); +let obj: Bound<'py, PyAny> = PyTuple::new(py, [1, 2, 3])?.into_any(); // extracting the Python `tuple` to a rust `(i32, i32, i32)` tuple let (x, y, z) = obj.extract::<(i32, i32, i32)>()?; diff --git a/pytests/src/datetime.rs b/pytests/src/datetime.rs index 744be279431..3bdf103b62e 100644 --- a/pytests/src/datetime.rs +++ b/pytests/src/datetime.rs @@ -12,7 +12,7 @@ fn make_date(py: Python<'_>, year: i32, month: u8, day: u8) -> PyResult(d: &Bound<'py, PyDate>) -> Bound<'py, PyTuple> { +fn get_date_tuple<'py>(d: &Bound<'py, PyDate>) -> PyResult> { PyTuple::new( d.py(), [d.get_year(), d.get_month() as i32, d.get_day() as i32], @@ -52,7 +52,7 @@ fn time_with_fold<'py>( } #[pyfunction] -fn get_time_tuple<'py>(dt: &Bound<'py, PyTime>) -> Bound<'py, PyTuple> { +fn get_time_tuple<'py>(dt: &Bound<'py, PyTime>) -> PyResult> { PyTuple::new( dt.py(), [ @@ -65,7 +65,7 @@ fn get_time_tuple<'py>(dt: &Bound<'py, PyTime>) -> Bound<'py, PyTuple> { } #[pyfunction] -fn get_time_tuple_fold<'py>(dt: &Bound<'py, PyTime>) -> Bound<'py, PyTuple> { +fn get_time_tuple_fold<'py>(dt: &Bound<'py, PyTime>) -> PyResult> { PyTuple::new( dt.py(), [ @@ -89,7 +89,7 @@ fn make_delta( } #[pyfunction] -fn get_delta_tuple<'py>(delta: &Bound<'py, PyDelta>) -> Bound<'py, PyTuple> { +fn get_delta_tuple<'py>(delta: &Bound<'py, PyDelta>) -> PyResult> { PyTuple::new( delta.py(), [ @@ -128,7 +128,7 @@ fn make_datetime<'py>( } #[pyfunction] -fn get_datetime_tuple<'py>(dt: &Bound<'py, PyDateTime>) -> Bound<'py, PyTuple> { +fn get_datetime_tuple<'py>(dt: &Bound<'py, PyDateTime>) -> PyResult> { PyTuple::new( dt.py(), [ @@ -144,7 +144,7 @@ fn get_datetime_tuple<'py>(dt: &Bound<'py, PyDateTime>) -> Bound<'py, PyTuple> { } #[pyfunction] -fn get_datetime_tuple_fold<'py>(dt: &Bound<'py, PyDateTime>) -> Bound<'py, PyTuple> { +fn get_datetime_tuple_fold<'py>(dt: &Bound<'py, PyDateTime>) -> PyResult> { PyTuple::new( dt.py(), [ diff --git a/src/impl_/extract_argument.rs b/src/impl_/extract_argument.rs index bb31f96d8b7..098722060c6 100644 --- a/src/impl_/extract_argument.rs +++ b/src/impl_/extract_argument.rs @@ -703,7 +703,7 @@ impl<'py> VarargsHandler<'py> for TupleVarargs { varargs: &[Option>], _function_description: &FunctionDescription, ) -> PyResult { - Ok(PyTuple::new(py, varargs)) + PyTuple::new(py, varargs) } #[inline] diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index 270d32f15a9..ab38bc49e8e 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -97,7 +97,7 @@ impl ModuleDef { .import("sys")? .getattr("implementation")? .getattr("version")?; - if version.lt(crate::types::PyTuple::new(py, PYPY_GOOD_VERSION))? { + if version.lt(crate::types::PyTuple::new(py, PYPY_GOOD_VERSION)?)? { let warn = py.import("warnings")?.getattr("warn")?; warn.call1(( "PyPy 3.7 versions older than 7.3.8 are known to have binary \ diff --git a/src/instance.rs b/src/instance.rs index 26c6b14ffa0..1ff18f82466 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -640,7 +640,7 @@ impl<'a, 'py, T> Borrowed<'a, 'py, T> { /// /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| -> PyResult<()> { - /// let tuple = PyTuple::new(py, [1, 2, 3]); + /// let tuple = PyTuple::new(py, [1, 2, 3])?; /// /// // borrows from `tuple`, so can only be /// // used while `tuple` stays alive diff --git a/src/types/datetime.rs b/src/types/datetime.rs index 2f6906064f3..ac956c250d3 100644 --- a/src/types/datetime.rs +++ b/src/types/datetime.rs @@ -223,7 +223,7 @@ impl PyDate { /// /// This is equivalent to `datetime.date.fromtimestamp` pub fn from_timestamp(py: Python<'_>, timestamp: i64) -> PyResult> { - let time_tuple = PyTuple::new(py, [timestamp]); + let time_tuple = PyTuple::new(py, [timestamp])?; // safety ensure that the API is loaded let _api = ensure_datetime_api(py)?; diff --git a/src/types/list.rs b/src/types/list.rs index a50a78bd2e5..4db14849d46 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -1099,7 +1099,7 @@ mod tests { Python::with_gil(|py| { let list = PyList::new(py, vec![1, 2, 3]).unwrap(); let tuple = list.to_tuple(); - let tuple_expected = PyTuple::new(py, vec![1, 2, 3]); + let tuple_expected = PyTuple::new(py, vec![1, 2, 3]).unwrap(); assert!(tuple.eq(tuple_expected).unwrap()); }) } diff --git a/src/types/sequence.rs b/src/types/sequence.rs index ae337ae7583..03e20644ee5 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -777,7 +777,7 @@ mod tests { assert!(seq .to_tuple() .unwrap() - .eq(PyTuple::new(py, ["foo", "bar"])) + .eq(PyTuple::new(py, ["foo", "bar"]).unwrap()) .unwrap()); }); } @@ -788,7 +788,11 @@ mod tests { let v = vec!["foo", "bar"]; let ob = (&v).into_pyobject(py).unwrap(); let seq = ob.downcast::().unwrap(); - assert!(seq.to_tuple().unwrap().eq(PyTuple::new(py, &v)).unwrap()); + assert!(seq + .to_tuple() + .unwrap() + .eq(PyTuple::new(py, &v).unwrap()) + .unwrap()); }); } diff --git a/src/types/tuple.rs b/src/types/tuple.rs index 265ce27708a..8f26e4d89e6 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -17,10 +17,10 @@ use crate::{ #[inline] #[track_caller] -fn new_from_iter<'py>( +fn try_new_from_iter<'py>( py: Python<'py>, - elements: &mut dyn ExactSizeIterator, -) -> Bound<'py, PyTuple> { + mut elements: impl ExactSizeIterator>>, +) -> PyResult> { unsafe { // PyTuple_New checks for overflow but has a bad error message, so we check ourselves let len: Py_ssize_t = elements @@ -36,18 +36,18 @@ fn new_from_iter<'py>( let mut counter: Py_ssize_t = 0; - for obj in elements.take(len as usize) { + for obj in (&mut elements).take(len as usize) { #[cfg(not(any(Py_LIMITED_API, PyPy, GraalPy)))] - ffi::PyTuple_SET_ITEM(ptr, counter, obj.into_ptr()); + ffi::PyTuple_SET_ITEM(ptr, counter, obj?.into_ptr()); #[cfg(any(Py_LIMITED_API, PyPy, GraalPy))] - ffi::PyTuple_SetItem(ptr, counter, obj.into_ptr()); + ffi::PyTuple_SetItem(ptr, counter, obj?.into_ptr()); counter += 1; } assert!(elements.next().is_none(), "Attempted to create PyTuple but `elements` was larger than reported by its `ExactSizeIterator` implementation."); assert_eq!(len, counter, "Attempted to create PyTuple but `elements` was smaller than reported by its `ExactSizeIterator` implementation."); - tup + Ok(tup) } } @@ -76,12 +76,13 @@ impl PyTuple { /// use pyo3::prelude::*; /// use pyo3::types::PyTuple; /// - /// # fn main() { + /// # fn main() -> PyResult<()> { /// Python::with_gil(|py| { /// let elements: Vec = vec![0, 1, 2, 3, 4, 5]; - /// let tuple = PyTuple::new(py, elements); + /// let tuple = PyTuple::new(py, elements)?; /// assert_eq!(format!("{:?}", tuple), "(0, 1, 2, 3, 4, 5)"); - /// }); + /// # Ok(()) + /// }) /// # } /// ``` /// @@ -91,16 +92,21 @@ impl PyTuple { /// All standard library structures implement this trait correctly, if they do, so calling this /// function using [`Vec`]`` or `&[T]` will always succeed. #[track_caller] - pub fn new( - py: Python<'_>, + pub fn new<'py, T, U>( + py: Python<'py>, elements: impl IntoIterator, - ) -> Bound<'_, PyTuple> + ) -> PyResult> where - T: ToPyObject, + T: IntoPyObject<'py>, U: ExactSizeIterator, { - let mut elements = elements.into_iter().map(|e| e.to_object(py)); - new_from_iter(py, &mut elements) + let elements = elements.into_iter().map(|e| { + e.into_pyobject(py) + .map(BoundObject::into_any) + .map(BoundObject::into_bound) + .map_err(Into::into) + }); + try_new_from_iter(py, elements) } /// Deprecated name for [`PyTuple::new`]. @@ -115,7 +121,7 @@ impl PyTuple { T: ToPyObject, U: ExactSizeIterator, { - PyTuple::new(py, elements) + PyTuple::new(py, elements.into_iter().map(|e| e.to_object(py))).unwrap() } /// Constructs an empty tuple (on the Python side, a singleton object). @@ -542,6 +548,19 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+ } } + impl <'a, 'py, $($T),+> IntoPyObject<'py> for &'a ($($T,)+) + where + $(&'a $T: IntoPyObject<'py>,)+ + { + type Target = PyTuple; + type Output = Bound<'py, Self::Target>; + type Error = PyErr; + + fn into_pyobject(self, py: Python<'py>) -> Result { + Ok(array_into_tuple(py, [$(self.$n.into_pyobject(py).map_err(Into::into)?.into_any().unbind()),+]).into_bound(py)) + } + } + impl <$($T: IntoPy),+> IntoPy> for ($($T,)+) { fn into_py(self, py: Python<'_>) -> Py { array_into_tuple(py, [$(self.$n.into_py(py)),+]) @@ -805,7 +824,7 @@ mod tests { #[test] fn test_new() { Python::with_gil(|py| { - let ob = PyTuple::new(py, [1, 2, 3]); + let ob = PyTuple::new(py, [1, 2, 3]).unwrap(); assert_eq!(3, ob.len()); let ob = ob.as_any(); assert_eq!((1, 2, 3), ob.extract().unwrap()); @@ -813,7 +832,7 @@ mod tests { let mut map = HashSet::new(); map.insert(1); map.insert(2); - PyTuple::new(py, map); + PyTuple::new(py, map).unwrap(); }); } @@ -841,7 +860,7 @@ mod tests { #[test] fn test_slice() { Python::with_gil(|py| { - let tup = PyTuple::new(py, [2, 3, 5, 7]); + let tup = PyTuple::new(py, [2, 3, 5, 7]).unwrap(); let slice = tup.get_slice(1, 3); assert_eq!(2, slice.len()); let slice = tup.get_slice(1, 7); @@ -900,7 +919,7 @@ mod tests { #[test] fn test_bound_iter() { Python::with_gil(|py| { - let tuple = PyTuple::new(py, [1, 2, 3]); + let tuple = PyTuple::new(py, [1, 2, 3]).unwrap(); assert_eq!(3, tuple.len()); let mut iter = tuple.iter(); @@ -923,7 +942,7 @@ mod tests { #[test] fn test_bound_iter_rev() { Python::with_gil(|py| { - let tuple = PyTuple::new(py, [1, 2, 3]); + let tuple = PyTuple::new(py, [1, 2, 3]).unwrap(); assert_eq!(3, tuple.len()); let mut iter = tuple.iter().rev(); @@ -1175,37 +1194,31 @@ mod tests { }) } - #[cfg(feature = "macros")] #[test] - fn bad_clone_mem_leaks() { - use crate::{IntoPy, Py, PyAny, ToPyObject}; + fn bad_intopyobject_doesnt_cause_leaks() { + use crate::types::PyInt; + use std::convert::Infallible; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; static NEEDS_DESTRUCTING_COUNT: AtomicUsize = AtomicUsize::new(0); - #[crate::pyclass] - #[pyo3(crate = "crate")] struct Bad(usize); - impl Clone for Bad { - fn clone(&self) -> Self { - // This panic should not lead to a memory leak - assert_ne!(self.0, 42); - NEEDS_DESTRUCTING_COUNT.fetch_add(1, SeqCst); - - Bad(self.0) - } - } - impl Drop for Bad { fn drop(&mut self) { NEEDS_DESTRUCTING_COUNT.fetch_sub(1, SeqCst); } } - impl ToPyObject for Bad { - fn to_object(&self, py: Python<'_>) -> Py { - self.to_owned().into_py(py) + impl<'py> IntoPyObject<'py> for Bad { + type Target = PyInt; + type Output = crate::Bound<'py, Self::Target>; + type Error = Infallible; + + fn into_pyobject(self, py: Python<'py>) -> Result { + // This panic should not lead to a memory leak + assert_ne!(self.0, 42); + self.0.into_pyobject(py) } } @@ -1243,37 +1256,31 @@ mod tests { ); } - #[cfg(feature = "macros")] #[test] - fn bad_clone_mem_leaks_2() { - use crate::{IntoPy, Py, PyAny, ToPyObject}; + fn bad_intopyobject_doesnt_cause_leaks_2() { + use crate::types::PyInt; + use std::convert::Infallible; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; static NEEDS_DESTRUCTING_COUNT: AtomicUsize = AtomicUsize::new(0); - #[crate::pyclass] - #[pyo3(crate = "crate")] struct Bad(usize); - impl Clone for Bad { - fn clone(&self) -> Self { - // This panic should not lead to a memory leak - assert_ne!(self.0, 3); - NEEDS_DESTRUCTING_COUNT.fetch_add(1, SeqCst); - - Bad(self.0) - } - } - impl Drop for Bad { fn drop(&mut self) { NEEDS_DESTRUCTING_COUNT.fetch_sub(1, SeqCst); } } - impl ToPyObject for Bad { - fn to_object(&self, py: Python<'_>) -> Py { - self.to_owned().into_py(py) + impl<'py> IntoPyObject<'py> for &Bad { + type Target = PyInt; + type Output = crate::Bound<'py, Self::Target>; + type Error = Infallible; + + fn into_pyobject(self, py: Python<'py>) -> Result { + // This panic should not lead to a memory leak + assert_ne!(self.0, 3); + self.0.into_pyobject(py) } } @@ -1281,7 +1288,7 @@ mod tests { NEEDS_DESTRUCTING_COUNT.store(4, SeqCst); Python::with_gil(|py| { std::panic::catch_unwind(|| { - let _tuple: Py = s.to_object(py); + let _tuple = (&s).into_pyobject(py).unwrap(); }) .unwrap_err(); }); @@ -1297,7 +1304,7 @@ mod tests { #[test] fn test_tuple_to_list() { Python::with_gil(|py| { - let tuple = PyTuple::new(py, vec![1, 2, 3]); + let tuple = PyTuple::new(py, vec![1, 2, 3]).unwrap(); let list = tuple.to_list(); let list_expected = PyList::new(py, vec![1, 2, 3]).unwrap(); assert!(list.eq(list_expected).unwrap()); @@ -1307,7 +1314,7 @@ mod tests { #[test] fn test_tuple_as_sequence() { Python::with_gil(|py| { - let tuple = PyTuple::new(py, vec![1, 2, 3]); + let tuple = PyTuple::new(py, vec![1, 2, 3]).unwrap(); let sequence = tuple.as_sequence(); assert!(tuple.get_item(0).unwrap().eq(1).unwrap()); assert!(sequence.get_item(0).unwrap().eq(1).unwrap()); @@ -1320,7 +1327,7 @@ mod tests { #[test] fn test_tuple_into_sequence() { Python::with_gil(|py| { - let tuple = PyTuple::new(py, vec![1, 2, 3]); + let tuple = PyTuple::new(py, vec![1, 2, 3]).unwrap(); let sequence = tuple.into_sequence(); assert!(sequence.get_item(0).unwrap().eq(1).unwrap()); assert_eq!(sequence.len().unwrap(), 3); @@ -1330,7 +1337,7 @@ mod tests { #[test] fn test_bound_tuple_get_item() { Python::with_gil(|py| { - let tuple = PyTuple::new(py, vec![1, 2, 3, 4]); + let tuple = PyTuple::new(py, vec![1, 2, 3, 4]).unwrap(); assert_eq!(tuple.len(), 4); assert_eq!(tuple.get_item(0).unwrap().extract::().unwrap(), 1); diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 8d4e759580c..7a66b7ad0df 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -285,7 +285,8 @@ mod tests { py.get_type::(), py.get_type::() ] - )) + ) + .unwrap()) .unwrap()); }); } @@ -296,7 +297,7 @@ mod tests { assert!(py .get_type::() .bases() - .eq(PyTuple::new(py, [py.get_type::()])) + .eq(PyTuple::new(py, [py.get_type::()]).unwrap()) .unwrap()); }); } diff --git a/tests/test_frompyobject.rs b/tests/test_frompyobject.rs index f7699d305b2..a1b91c25128 100644 --- a/tests/test_frompyobject.rs +++ b/tests/test_frompyobject.rs @@ -168,10 +168,24 @@ pub struct Tuple(String, usize); #[test] fn test_tuple_struct() { Python::with_gil(|py| { - let tup = PyTuple::new(py, &[1.into_py(py), "test".into_py(py)]); + let tup = PyTuple::new( + py, + &[ + 1i32.into_pyobject(py).unwrap().into_any(), + "test".into_pyobject(py).unwrap().into_any(), + ], + ) + .unwrap(); let tup = tup.extract::(); assert!(tup.is_err()); - let tup = PyTuple::new(py, &["test".into_py(py), 1.into_py(py)]); + let tup = PyTuple::new( + py, + &[ + "test".into_pyobject(py).unwrap().into_any(), + 1i32.into_pyobject(py).unwrap().into_any(), + ], + ) + .unwrap(); let tup = tup .extract::() .expect("Failed to extract Tuple from PyTuple"); @@ -333,7 +347,7 @@ pub struct PyBool { #[test] fn test_enum() { Python::with_gil(|py| { - let tup = PyTuple::new(py, &[1.into_py(py), "test".into_py(py)]); + let tup = PyTuple::new(py, &[1i32.into_py(py), "test".into_py(py)]).unwrap(); let f = tup .extract::>() .expect("Failed to extract Foo from tuple"); diff --git a/tests/test_various.rs b/tests/test_various.rs index dc6bbc76dba..27192aba3bb 100644 --- a/tests/test_various.rs +++ b/tests/test_various.rs @@ -91,7 +91,7 @@ fn intopytuple_pyclass() { #[test] fn pytuple_primitive_iter() { Python::with_gil(|py| { - let tup = PyTuple::new(py, [1u32, 2, 3].iter()); + let tup = PyTuple::new(py, [1u32, 2, 3].iter()).unwrap(); py_assert!(py, tup, "tup == (1, 2, 3)"); }); } @@ -106,7 +106,8 @@ fn pytuple_pyclass_iter() { Py::new(py, SimplePyClass {}).unwrap(), ] .iter(), - ); + ) + .unwrap(); py_assert!(py, tup, "type(tup[0]).__name__ == 'SimplePyClass'"); py_assert!(py, tup, "type(tup[0]).__name__ == type(tup[0]).__name__"); py_assert!(py, tup, "tup[0] != tup[1]"); diff --git a/tests/ui/invalid_property_args.stderr b/tests/ui/invalid_property_args.stderr index 70f3dd4e8f8..786533efd53 100644 --- a/tests/ui/invalid_property_args.stderr +++ b/tests/ui/invalid_property_args.stderr @@ -56,13 +56,13 @@ error[E0277]: `PhantomData` cannot be converted to a Python object = note: implement `IntoPyObject` for `&PhantomData` or `IntoPyObject + Clone` for `PhantomData` to define the conversion = help: the following other types implement trait `IntoPyObject<'py>`: &&str - &'a BTreeMap - &'a BTreeSet - &'a Cell - &'a HashMap - &'a HashSet - &'a Option - &'a Py + &'a (T0, T1) + &'a (T0, T1, T2) + &'a (T0, T1, T2, T3) + &'a (T0, T1, T2, T3, T4) + &'a (T0, T1, T2, T3, T4, T5) + &'a (T0, T1, T2, T3, T4, T5, T6) + &'a (T0, T1, T2, T3, T4, T5, T6, T7) and $N others = note: required for `PhantomData` to implement `for<'py> PyO3GetField<'py>` note: required by a bound in `PyClassGetterGenerator::::generate` diff --git a/tests/ui/missing_intopy.stderr b/tests/ui/missing_intopy.stderr index f7dcca419bf..653fb785dfd 100644 --- a/tests/ui/missing_intopy.stderr +++ b/tests/ui/missing_intopy.stderr @@ -9,13 +9,13 @@ error[E0277]: `Blah` cannot be converted to a Python object = note: if you do not own `Blah` you can perform a manual conversion to one of the types in `pyo3::types::*` = help: the following other types implement trait `IntoPyObject<'py>`: &&str - &'a BTreeMap - &'a BTreeSet - &'a Cell - &'a HashMap - &'a HashSet - &'a Option - &'a Py + &'a (T0, T1) + &'a (T0, T1, T2) + &'a (T0, T1, T2, T3) + &'a (T0, T1, T2, T3, T4) + &'a (T0, T1, T2, T3, T4, T5) + &'a (T0, T1, T2, T3, T4, T5, T6) + &'a (T0, T1, T2, T3, T4, T5, T6, T7) and $N others note: required by a bound in `UnknownReturnType::::wrap` --> src/impl_/wrap.rs From 42b6c12f7b51a514ad65f8e07beb5609fc1b6b18 Mon Sep 17 00:00:00 2001 From: Icxolu <10486322+Icxolu@users.noreply.github.com> Date: Fri, 27 Sep 2024 20:05:02 +0200 Subject: [PATCH 3/3] add newsfragment --- newsfragments/4580.changed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/4580.changed.md diff --git a/newsfragments/4580.changed.md b/newsfragments/4580.changed.md new file mode 100644 index 00000000000..72b838d4055 --- /dev/null +++ b/newsfragments/4580.changed.md @@ -0,0 +1 @@ +`PyList::new` and `PyTuple::new` are now fallible due to `IntoPyObject` migration. \ No newline at end of file