implement PySetMethods

src/prelude.rs

@@ -34,4 +34,5 @@ pub use crate::types::float::PyFloatMethods;
 pub use crate::types::list::PyListMethods;
 pub use crate::types::mapping::PyMappingMethods;
 pub use crate::types::sequence::PySequenceMethods;
+pub use crate::types::set::PySetMethods;
 pub use crate::types::string::PyStringMethods;

src/types/mod.rs

@@ -79,7 +79,7 @@ pub mod iter {
     pub use super::dict::{BoundDictIterator, PyDictIterator};
     pub use super::frozenset::PyFrozenSetIterator;
     pub use super::list::{BoundListIterator, PyListIterator};
-    pub use super::set::PySetIterator;
+    pub use super::set::{BoundSetIterator, PySetIterator};
     pub use super::tuple::PyTupleIterator;
 }
 

src/types/set.rs

@@ -2,7 +2,9 @@
 use crate::types::PyIterator;
 use crate::{
     err::{self, PyErr, PyResult},
-    Py,
+    ffi_ptr_ext::FfiPtrExt,
+    instance::Bound,
+    Py, PyNativeType,
 };
 use crate::{ffi, PyAny, PyObject, Python, ToPyObject};
 use std::ptr;
@@ -46,9 +48,7 @@ impl PySet {
     /// Removes all elements from the set.
     #[inline]
     pub fn clear(&self) {
-        unsafe {
+        self.as_borrowed().clear()
-            ffi::PySet_Clear(self.as_ptr());
-        }
     }
 
     /// Returns the number of items in the set.
@@ -56,12 +56,12 @@ impl PySet {
     /// This is equivalent to the Python expression `len(self)`.
     #[inline]
     pub fn len(&self) -> usize {
-        unsafe { ffi::PySet_Size(self.as_ptr()) as usize }
+        self.as_borrowed().len()
     }
 
     /// Checks if set is empty.
     pub fn is_empty(&self) -> bool {
-        self.len() == 0
+        self.as_borrowed().is_empty()
     }
 
     /// Determines if the set contains the specified key.
@@ -71,15 +71,7 @@ impl PySet {
     where
         K: ToPyObject,
     {
-        fn inner(set: &PySet, key: PyObject) -> PyResult<bool> {
+        self.as_borrowed().contains(key)
-            match unsafe { ffi::PySet_Contains(set.as_ptr(), key.as_ptr()) } {
-                1 => Ok(true),
-                0 => Ok(false),
-                _ => Err(PyErr::fetch(set.py())),
-            }
-        }
-
-        inner(self, key.to_object(self.py()))
     }
 
     /// Removes the element from the set if it is present.
@@ -89,15 +81,7 @@ impl PySet {
     where
         K: ToPyObject,
     {
-        fn inner(set: &PySet, key: PyObject) -> PyResult<bool> {
+        self.as_borrowed().discard(key)
-            match unsafe { ffi::PySet_Discard(set.as_ptr(), key.as_ptr()) } {
-                1 => Ok(true),
-                0 => Ok(false),
-                _ => Err(PyErr::fetch(set.py())),
-            }
-        }
-
-        inner(self, key.to_object(self.py()))
     }
 
     /// Adds an element to the set.
@@ -105,23 +89,12 @@ impl PySet {
     where
         K: ToPyObject,
     {
-        fn inner(set: &PySet, key: PyObject) -> PyResult<()> {
+        self.as_borrowed().add(key)
-            err::error_on_minusone(set.py(), unsafe {
-                ffi::PySet_Add(set.as_ptr(), key.as_ptr())
-            })
-        }
-
-        inner(self, key.to_object(self.py()))
     }
 
     /// Removes and returns an arbitrary element from the set.
     pub fn pop(&self) -> Option<PyObject> {
-        let element =
+        self.as_borrowed().pop().map(Py::from)
-            unsafe { PyObject::from_owned_ptr_or_err(self.py(), ffi::PySet_Pop(self.as_ptr())) };
-        match element {
-            Ok(e) => Some(e),
-            Err(_) => None,
-        }
     }
 
     /// Returns an iterator of values in this set.
@@ -130,7 +103,191 @@ impl PySet {
     ///
     /// If PyO3 detects that the set is mutated during iteration, it will panic.
     pub fn iter(&self) -> PySetIterator<'_> {
-        IntoIterator::into_iter(self)
+        PySetIterator(BoundSetIterator::new(self.as_borrowed().to_owned()))
+    }
+}
+
+/// Implementation of functionality for [`PySet`].
+///
+/// These methods are defined for the `Bound<'py, PySet>` smart pointer, so to use method call
+/// syntax these methods are separated into a trait, because stable Rust does not yet support
+/// `arbitrary_self_types`.
+#[doc(alias = "PySet")]
+pub trait PySetMethods<'py> {
+    /// Removes all elements from the set.
+    fn clear(&self);
+
+    /// Returns the number of items in the set.
+    ///
+    /// This is equivalent to the Python expression `len(self)`.
+    fn len(&self) -> usize;
+
+    /// Checks if set is empty.
+    fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
+    /// Determines if the set contains the specified key.
+    ///
+    /// This is equivalent to the Python expression `key in self`.
+    fn contains<K>(&self, key: K) -> PyResult<bool>
+    where
+        K: ToPyObject;
+
+    /// Removes the element from the set if it is present.
+    ///
+    /// Returns `true` if the element was present in the set.
+    fn discard<K>(&self, key: K) -> PyResult<bool>
+    where
+        K: ToPyObject;
+
+    /// Adds an element to the set.
+    fn add<K>(&self, key: K) -> PyResult<()>
+    where
+        K: ToPyObject;
+
+    /// Removes and returns an arbitrary element from the set.
+    fn pop(&self) -> Option<Bound<'py, PyAny>>;
+
+    /// Returns an iterator of values in this set.
+    ///
+    /// # Panics
+    ///
+    /// If PyO3 detects that the set is mutated during iteration, it will panic.
+    fn iter(&self) -> BoundSetIterator<'py>;
+}
+
+impl<'py> PySetMethods<'py> for Bound<'py, PySet> {
+    #[inline]
+    fn clear(&self) {
+        unsafe {
+            ffi::PySet_Clear(self.as_ptr());
+        }
+    }
+
+    #[inline]
+    fn len(&self) -> usize {
+        unsafe { ffi::PySet_Size(self.as_ptr()) as usize }
+    }
+
+    fn contains<K>(&self, key: K) -> PyResult<bool>
+    where
+        K: ToPyObject,
+    {
+        fn inner(set: &Bound<'_, PySet>, key: Bound<'_, PyAny>) -> PyResult<bool> {
+            match unsafe { ffi::PySet_Contains(set.as_ptr(), key.as_ptr()) } {
+                1 => Ok(true),
+                0 => Ok(false),
+                _ => Err(PyErr::fetch(set.py())),
+            }
+        }
+
+        let py = self.py();
+        inner(self, key.to_object(py).into_bound(py))
+    }
+
+    fn discard<K>(&self, key: K) -> PyResult<bool>
+    where
+        K: ToPyObject,
+    {
+        fn inner(set: &Bound<'_, PySet>, key: Bound<'_, PyAny>) -> PyResult<bool> {
+            match unsafe { ffi::PySet_Discard(set.as_ptr(), key.as_ptr()) } {
+                1 => Ok(true),
+                0 => Ok(false),
+                _ => Err(PyErr::fetch(set.py())),
+            }
+        }
+
+        let py = self.py();
+        inner(self, key.to_object(py).into_bound(py))
+    }
+
+    fn add<K>(&self, key: K) -> PyResult<()>
+    where
+        K: ToPyObject,
+    {
+        fn inner(set: &Bound<'_, PySet>, key: Bound<'_, PyAny>) -> PyResult<()> {
+            err::error_on_minusone(set.py(), unsafe {
+                ffi::PySet_Add(set.as_ptr(), key.as_ptr())
+            })
+        }
+
+        let py = self.py();
+        inner(self, key.to_object(py).into_bound(py))
+    }
+
+    fn pop(&self) -> Option<Bound<'py, PyAny>> {
+        let element = unsafe { ffi::PySet_Pop(self.as_ptr()).assume_owned_or_err(self.py()) };
+        match element {
+            Ok(e) => Some(e),
+            Err(_) => None,
+        }
+    }
+
+    fn iter(&self) -> BoundSetIterator<'py> {
+        BoundSetIterator::new(self.clone())
+    }
+}
+
+/// PyO3 implementation of an iterator for a Python `set` object.
+pub struct PySetIterator<'py>(BoundSetIterator<'py>);
+
+impl<'py> Iterator for PySetIterator<'py> {
+    type Item = &'py super::PyAny;
+
+    /// Advances the iterator and returns the next value.
+    ///
+    /// # Panics
+    ///
+    /// If PyO3 detects that the set is mutated during iteration, it will panic.
+    #[inline]
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next().map(Bound::into_gil_ref)
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        self.0.size_hint()
+    }
+}
+
+impl<'py> IntoIterator for &'py PySet {
+    type Item = &'py PyAny;
+    type IntoIter = PySetIterator<'py>;
+    /// Returns an iterator of values in this set.
+    ///
+    /// # Panics
+    ///
+    /// If PyO3 detects that the set is mutated during iteration, it will panic.
+    fn into_iter(self) -> Self::IntoIter {
+        PySetIterator(BoundSetIterator::new(self.as_borrowed().to_owned()))
+    }
+}
+
+impl<'py> IntoIterator for Bound<'py, PySet> {
+    type Item = Bound<'py, PyAny>;
+    type IntoIter = BoundSetIterator<'py>;
+
+    /// Returns an iterator of values in this set.
+    ///
+    /// # Panics
+    ///
+    /// If PyO3 detects that the set is mutated during iteration, it will panic.
+    fn into_iter(self) -> Self::IntoIter {
+        BoundSetIterator::new(self)
+    }
+}
+
+impl<'py> IntoIterator for &'_ Bound<'py, PySet> {
+    type Item = Bound<'py, PyAny>;
+    type IntoIter = BoundSetIterator<'py>;
+
+    /// Returns an iterator of values in this set.
+    ///
+    /// # Panics
+    ///
+    /// If PyO3 detects that the set is mutated during iteration, it will panic.
+    fn into_iter(self) -> Self::IntoIter {
+        BoundSetIterator::new(self.clone())
     }
 }
 
@@ -138,29 +295,21 @@ impl PySet {
 mod impl_ {
     use super::*;
 
-    impl<'a> std::iter::IntoIterator for &'a PySet {
+    /// PyO3 implementation of an iterator for a Python `set` object.
-        type Item = &'a PyAny;
+    pub struct BoundSetIterator<'p> {
-        type IntoIter = PySetIterator<'a>;
+        it: Bound<'p, PyIterator>,
+    }
 
-        /// Returns an iterator of values in this set.
+    impl<'py> BoundSetIterator<'py> {
-        ///
+        pub(super) fn new(set: Bound<'py, PySet>) -> Self {
-        /// # Panics
+            Self {
-        ///
+                it: PyIterator::from_object2(&set).unwrap(),
-        /// If PyO3 detects that the set is mutated during iteration, it will panic.
-        fn into_iter(self) -> Self::IntoIter {
-            PySetIterator {
-                it: PyIterator::from_object(self).unwrap(),
             }
         }
     }
 
-    /// PyO3 implementation of an iterator for a Python `set` object.
+    impl<'py> Iterator for BoundSetIterator<'py> {
-    pub struct PySetIterator<'p> {
+        type Item = Bound<'py, super::PyAny>;
-        it: &'p PyIterator,
-    }
-
-    impl<'py> Iterator for PySetIterator<'py> {
-        type Item = &'py super::PyAny;
 
         /// Advances the iterator and returns the next value.
         ///
@@ -179,31 +328,21 @@ mod impl_ {
     use super::*;
 
     /// PyO3 implementation of an iterator for a Python `set` object.
-    pub struct PySetIterator<'py> {
+    pub struct BoundSetIterator<'py> {
-        set: &'py super::PySet,
+        set: Bound<'py, super::PySet>,
         pos: ffi::Py_ssize_t,
         used: ffi::Py_ssize_t,
     }
 
-    impl<'a> std::iter::IntoIterator for &'a PySet {
+    impl<'py> BoundSetIterator<'py> {
-        type Item = &'a PyAny;
+        pub(super) fn new(set: Bound<'py, PySet>) -> Self {
-        type IntoIter = PySetIterator<'a>;
+            let used = unsafe { ffi::PySet_Size(set.as_ptr()) };
-        /// Returns an iterator of values in this set.
+            BoundSetIterator { set, pos: 0, used }
-        ///
-        /// # Panics
-        ///
-        /// If PyO3 detects that the set is mutated during iteration, it will panic.
-        fn into_iter(self) -> Self::IntoIter {
-            PySetIterator {
-                set: self,
-                pos: 0,
-                used: unsafe { ffi::PySet_Size(self.as_ptr()) },
-            }
         }
     }
 
-    impl<'py> Iterator for PySetIterator<'py> {
+    impl<'py> Iterator for BoundSetIterator<'py> {
-        type Item = &'py super::PyAny;
+        type Item = Bound<'py, super::PyAny>;
 
         /// Advances the iterator and returns the next value.
         ///
@@ -221,7 +360,7 @@ mod impl_ {
                 if ffi::_PySet_NextEntry(self.set.as_ptr(), &mut self.pos, &mut key, &mut hash) != 0
                 {
                     // _PySet_NextEntry returns borrowed object; for safety must make owned (see #890)
-                    Some(self.set.py().from_owned_ptr(ffi::_Py_NewRef(key)))
+                    Some(key.assume_borrowed(self.set.py()).to_owned())
                 } else {
                     None
                 }
@@ -235,11 +374,17 @@ mod impl_ {
         }
     }
 
-    impl<'py> ExactSizeIterator for PySetIterator<'py> {
+    impl<'py> ExactSizeIterator for BoundSetIterator<'py> {
         fn len(&self) -> usize {
             self.set.len().saturating_sub(self.pos as usize)
         }
     }
+
+    impl<'py> ExactSizeIterator for PySetIterator<'py> {
+        fn len(&self) -> usize {
+            self.0.len()
+        }
+    }
 }
 
 pub use impl_::*;
@@ -289,6 +434,7 @@ mod tests {
         Python::with_gil(|py| {
             let set = PySet::empty(py).unwrap();
             assert_eq!(0, set.len());
+            assert!(set.is_empty());
         });
     }
 
@@ -406,19 +552,30 @@ mod tests {
     }
 
     #[test]
+    #[cfg(not(Py_LIMITED_API))]
     fn test_set_iter_size_hint() {
         Python::with_gil(|py| {
             let set = PySet::new(py, &[1]).unwrap();
-
             let mut iter = set.iter();
 
-            if cfg!(Py_LIMITED_API) {
+            // Exact size
-                assert_eq!(iter.size_hint(), (0, None));
+            assert_eq!(iter.len(), 1);
-            } else {
+            assert_eq!(iter.size_hint(), (1, Some(1)));
-                assert_eq!(iter.size_hint(), (1, Some(1)));
+            iter.next();
-                iter.next();
+            assert_eq!(iter.len(), 0);
-                assert_eq!(iter.size_hint(), (0, Some(0)));
+            assert_eq!(iter.size_hint(), (0, Some(0)));
-            }
+        });
+    }
+
+    #[test]
+    #[cfg(Py_LIMITED_API)]
+    fn test_set_iter_size_hint() {
+        Python::with_gil(|py| {
+            let set = PySet::new(py, &[1]).unwrap();
+            let iter = set.iter();
+
+            // No known bounds
+            assert_eq!(iter.size_hint(), (0, None));
         });
     }
 }