Merge pull request #3743 from davidhewitt/set-bound-constructors

add bound constructors for `PySet` and `PyFrozenSet`

pyo3-benches/benches/bench_set.rs

@@ -10,21 +10,17 @@ fn set_new(b: &mut Bencher<'_>) {
         // Create Python objects up-front, so that the benchmark doesn't need to include
         // the cost of allocating LEN Python integers
         let elements: Vec<PyObject> = (0..LEN).map(|i| i.into_py(py)).collect();
-        b.iter(|| {
+        b.iter_with_large_drop(|| PySet::new_bound(py, &elements).unwrap());
-            let pool = unsafe { py.new_pool() };
-            PySet::new(py, &elements).unwrap();
-            drop(pool);
-        });
     });
 }
 
 fn iter_set(b: &mut Bencher<'_>) {
     Python::with_gil(|py| {
         const LEN: usize = 100_000;
-        let set = PySet::new(py, &(0..LEN).collect::<Vec<_>>()).unwrap();
+        let set = PySet::new_bound(py, &(0..LEN).collect::<Vec<_>>()).unwrap();
         let mut sum = 0;
         b.iter(|| {
-            for x in set {
+            for x in set.iter() {
                 let i: u64 = x.extract().unwrap();
                 sum += i;
             }
@@ -35,16 +31,16 @@ fn iter_set(b: &mut Bencher<'_>) {
 fn extract_hashset(b: &mut Bencher<'_>) {
     Python::with_gil(|py| {
         const LEN: usize = 100_000;
-        let set = PySet::new(py, &(0..LEN).collect::<Vec<_>>()).unwrap();
+        let set = PySet::new_bound(py, &(0..LEN).collect::<Vec<_>>()).unwrap();
-        b.iter(|| HashSet::<u64>::extract(set));
+        b.iter_with_large_drop(|| HashSet::<u64>::extract(set.as_gil_ref()));
     });
 }
 
 fn extract_btreeset(b: &mut Bencher<'_>) {
     Python::with_gil(|py| {
         const LEN: usize = 100_000;
-        let set = PySet::new(py, &(0..LEN).collect::<Vec<_>>()).unwrap();
+        let set = PySet::new_bound(py, &(0..LEN).collect::<Vec<_>>()).unwrap();
-        b.iter(|| BTreeSet::<u64>::extract(set));
+        b.iter_with_large_drop(|| BTreeSet::<u64>::extract(set.as_gil_ref()));
     });
 }
 
@@ -52,8 +48,8 @@ fn extract_btreeset(b: &mut Bencher<'_>) {
 fn extract_hashbrown_set(b: &mut Bencher<'_>) {
     Python::with_gil(|py| {
         const LEN: usize = 100_000;
-        let set = PySet::new(py, &(0..LEN).collect::<Vec<_>>()).unwrap();
+        let set = PySet::new_bound(py, &(0..LEN).collect::<Vec<_>>()).unwrap();
-        b.iter(|| hashbrown::HashSet::<u64>::extract(set));
+        b.iter_with_large_drop(|| hashbrown::HashSet::<u64>::extract(set.as_gil_ref()));
     });
 }
 

src/conversions/hashbrown.rs

@@ -109,6 +109,7 @@ where
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::types::any::PyAnyMethods;
 
     #[test]
     fn test_hashbrown_hashmap_to_python() {
@@ -178,11 +179,11 @@ mod tests {
     #[test]
     fn test_extract_hashbrown_hashset() {
         Python::with_gil(|py| {
-            let set = PySet::new(py, &[1, 2, 3, 4, 5]).unwrap();
+            let set = PySet::new_bound(py, &[1, 2, 3, 4, 5]).unwrap();
             let hash_set: hashbrown::HashSet<usize> = set.extract().unwrap();
             assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
 
-            let set = PyFrozenSet::new(py, &[1, 2, 3, 4, 5]).unwrap();
+            let set = PyFrozenSet::new_bound(py, &[1, 2, 3, 4, 5]).unwrap();
             let hash_set: hashbrown::HashSet<usize> = set.extract().unwrap();
             assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
         });

src/conversions/std/set.rs

@@ -113,18 +113,18 @@ where
 
 #[cfg(test)]
 mod tests {
-    use super::{PyFrozenSet, PySet};
+    use crate::types::{any::PyAnyMethods, PyFrozenSet, PySet};
     use crate::{IntoPy, PyObject, Python, ToPyObject};
     use std::collections::{BTreeSet, HashSet};
 
     #[test]
     fn test_extract_hashset() {
         Python::with_gil(|py| {
-            let set = PySet::new(py, &[1, 2, 3, 4, 5]).unwrap();
+            let set = PySet::new_bound(py, &[1, 2, 3, 4, 5]).unwrap();
             let hash_set: HashSet<usize> = set.extract().unwrap();
             assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
 
-            let set = PyFrozenSet::new(py, &[1, 2, 3, 4, 5]).unwrap();
+            let set = PyFrozenSet::new_bound(py, &[1, 2, 3, 4, 5]).unwrap();
             let hash_set: HashSet<usize> = set.extract().unwrap();
             assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
         });
@@ -133,11 +133,11 @@ mod tests {
     #[test]
     fn test_extract_btreeset() {
         Python::with_gil(|py| {
-            let set = PySet::new(py, &[1, 2, 3, 4, 5]).unwrap();
+            let set = PySet::new_bound(py, &[1, 2, 3, 4, 5]).unwrap();
             let hash_set: BTreeSet<usize> = set.extract().unwrap();
             assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
 
-            let set = PyFrozenSet::new(py, &[1, 2, 3, 4, 5]).unwrap();
+            let set = PyFrozenSet::new_bound(py, &[1, 2, 3, 4, 5]).unwrap();
             let hash_set: BTreeSet<usize> = set.extract().unwrap();
             assert_eq!(hash_set, [1, 2, 3, 4, 5].iter().copied().collect());
         });

src/types/frozenset.rs

@@ -1,10 +1,12 @@
-#[cfg(not(Py_LIMITED_API))]
-use crate::ffi_ptr_ext::FfiPtrExt;
 #[cfg(Py_LIMITED_API)]
 use crate::types::PyIterator;
 use crate::{
     err::{self, PyErr, PyResult},
-    ffi, Bound, Py, PyAny, PyNativeType, PyObject, Python, ToPyObject,
+    ffi,
+    ffi_ptr_ext::FfiPtrExt,
+    py_result_ext::PyResultExt,
+    types::any::PyAnyMethods,
+    Bound, PyAny, PyNativeType, PyObject, Python, ToPyObject,
 };
 use std::ptr;
 
@@ -63,20 +65,45 @@ pyobject_native_type_core!(
 );
 
 impl PyFrozenSet {
-    /// Creates a new frozenset.
+    /// Deprecated form of [`PyFrozenSet::new_bound`].
-    ///
-    /// May panic when running out of memory.
     #[inline]
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "`PyFrozenSet::new` will be replaced by `PyFrozenSet::new_bound` in a future PyO3 version"
+        )
+    )]
     pub fn new<'a, 'p, T: ToPyObject + 'a>(
         py: Python<'p>,
         elements: impl IntoIterator<Item = &'a T>,
     ) -> PyResult<&'p PyFrozenSet> {
-        new_from_iter(py, elements).map(|set| set.into_ref(py))
+        Self::new_bound(py, elements).map(Bound::into_gil_ref)
+    }
+
+    /// Creates a new frozenset.
+    ///
+    /// May panic when running out of memory.
+    #[inline]
+    pub fn new_bound<'a, 'p, T: ToPyObject + 'a>(
+        py: Python<'p>,
+        elements: impl IntoIterator<Item = &'a T>,
+    ) -> PyResult<Bound<'p, PyFrozenSet>> {
+        new_from_iter(py, elements)
+    }
+
+    /// Deprecated form of [`PyFrozenSet::empty_bound`].
+    pub fn empty(py: Python<'_>) -> PyResult<&'_ PyFrozenSet> {
+        Self::empty_bound(py).map(Bound::into_gil_ref)
     }
 
     /// Creates a new empty frozen set
-    pub fn empty(py: Python<'_>) -> PyResult<&PyFrozenSet> {
+    pub fn empty_bound(py: Python<'_>) -> PyResult<Bound<'_, PyFrozenSet>> {
-        unsafe { py.from_owned_ptr_or_err(ffi::PyFrozenSet_New(ptr::null_mut())) }
+        unsafe {
+            ffi::PyFrozenSet_New(ptr::null_mut())
+                .assume_owned_or_err(py)
+                .downcast_into_unchecked()
+        }
     }
 
     /// Return the number of items in the set.
@@ -285,14 +312,16 @@ pub use impl_::*;
 pub(crate) fn new_from_iter<T: ToPyObject>(
     py: Python<'_>,
     elements: impl IntoIterator<Item = T>,
-) -> PyResult<Py<PyFrozenSet>> {
+) -> PyResult<Bound<'_, PyFrozenSet>> {
-    fn inner(
+    fn inner<'py>(
-        py: Python<'_>,
+        py: Python<'py>,
         elements: &mut dyn Iterator<Item = PyObject>,
-    ) -> PyResult<Py<PyFrozenSet>> {
+    ) -> PyResult<Bound<'py, PyFrozenSet>> {
-        let set: Py<PyFrozenSet> = unsafe {
+        let set = unsafe {
             // We create the  `Py` pointer because its Drop cleans up the set if user code panics.
-            Py::from_owned_ptr_or_err(py, ffi::PyFrozenSet_New(std::ptr::null_mut()))?
+            ffi::PyFrozenSet_New(std::ptr::null_mut())
+                .assume_owned_or_err(py)?
+                .downcast_into_unchecked()
         };
         let ptr = set.as_ptr();
 
@@ -308,6 +337,7 @@ pub(crate) fn new_from_iter<T: ToPyObject>(
 }
 
 #[cfg(test)]
+#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))]
 mod tests {
     use super::*;
 

src/types/set.rs

@@ -4,7 +4,9 @@ use crate::{
     err::{self, PyErr, PyResult},
     ffi_ptr_ext::FfiPtrExt,
     instance::Bound,
-    Py, PyNativeType,
+    py_result_ext::PyResultExt,
+    types::any::PyAnyMethods,
+    PyNativeType,
 };
 use crate::{ffi, PyAny, PyObject, Python, ToPyObject};
 use std::ptr;
@@ -29,20 +31,45 @@ pyobject_native_type_core!(
 );
 
 impl PySet {
-    /// Creates a new set with elements from the given slice.
+    /// Deprecated form of [`PySet::new_bound`].
-    ///
+    #[cfg_attr(
-    /// Returns an error if some element is not hashable.
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "`PySet::new` will be replaced by `PySet::new_bound` in a future PyO3 version"
+        )
+    )]
     #[inline]
     pub fn new<'a, 'p, T: ToPyObject + 'a>(
         py: Python<'p>,
         elements: impl IntoIterator<Item = &'a T>,
     ) -> PyResult<&'p PySet> {
-        new_from_iter(py, elements).map(|set| set.into_ref(py))
+        Self::new_bound(py, elements).map(Bound::into_gil_ref)
+    }
+
+    /// Creates a new set with elements from the given slice.
+    ///
+    /// Returns an error if some element is not hashable.
+    #[inline]
+    pub fn new_bound<'a, 'p, T: ToPyObject + 'a>(
+        py: Python<'p>,
+        elements: impl IntoIterator<Item = &'a T>,
+    ) -> PyResult<Bound<'p, PySet>> {
+        new_from_iter(py, elements)
+    }
+
+    /// Deprecated form of [`PySet::empty_bound`].
+    pub fn empty(py: Python<'_>) -> PyResult<&'_ PySet> {
+        Self::empty_bound(py).map(Bound::into_gil_ref)
     }
 
     /// Creates a new empty set.
-    pub fn empty(py: Python<'_>) -> PyResult<&PySet> {
+    pub fn empty_bound(py: Python<'_>) -> PyResult<Bound<'_, PySet>> {
-        unsafe { py.from_owned_ptr_or_err(ffi::PySet_New(ptr::null_mut())) }
+        unsafe {
+            ffi::PySet_New(ptr::null_mut())
+                .assume_owned_or_err(py)
+                .downcast_into_unchecked()
+        }
     }
 
     /// Removes all elements from the set.
@@ -379,11 +406,16 @@ pub use impl_::*;
 pub(crate) fn new_from_iter<T: ToPyObject>(
     py: Python<'_>,
     elements: impl IntoIterator<Item = T>,
-) -> PyResult<Py<PySet>> {
+) -> PyResult<Bound<'_, PySet>> {
-    fn inner(py: Python<'_>, elements: &mut dyn Iterator<Item = PyObject>) -> PyResult<Py<PySet>> {
+    fn inner<'py>(
-        let set: Py<PySet> = unsafe {
+        py: Python<'py>,
+        elements: &mut dyn Iterator<Item = PyObject>,
+    ) -> PyResult<Bound<'py, PySet>> {
+        let set = unsafe {
             // We create the  `Py` pointer because its Drop cleans up the set if user code panics.
-            Py::from_owned_ptr_or_err(py, ffi::PySet_New(std::ptr::null_mut()))?
+            ffi::PySet_New(std::ptr::null_mut())
+                .assume_owned_or_err(py)?
+                .downcast_into_unchecked()
         };
         let ptr = set.as_ptr();
 
@@ -399,6 +431,7 @@ pub(crate) fn new_from_iter<T: ToPyObject>(
 }
 
 #[cfg(test)]
+#[cfg_attr(not(feature = "gil-refs"), allow(deprecated))]
 mod tests {
     use super::PySet;
     use crate::{Python, ToPyObject};