diff --git a/newsfragments/3281.changed.md b/newsfragments/3281.changed.md new file mode 100644 index 00000000..777ff815 --- /dev/null +++ b/newsfragments/3281.changed.md @@ -0,0 +1 @@ +Change `PySet::discard` to return `PyResult` (previously returned nothing). diff --git a/newsfragments/3281.fixed.md b/newsfragments/3281.fixed.md new file mode 100644 index 00000000..ce90a50a --- /dev/null +++ b/newsfragments/3281.fixed.md @@ -0,0 +1 @@ +Handle exceptions properly in `PySet::discard`. diff --git a/src/types/set.rs b/src/types/set.rs index 76771862..493d7411 100644 --- a/src/types/set.rs +++ b/src/types/set.rs @@ -81,13 +81,23 @@ impl PySet { } /// Removes the element from the set if it is present. - pub fn discard(&self, key: K) + /// + /// Returns `true` if the element was present in the set. + pub fn discard(&self, key: K) -> PyResult where K: ToPyObject, { - unsafe { - ffi::PySet_Discard(self.as_ptr(), key.to_object(self.py()).as_ptr()); + fn inner(set: &PySet, key: PyObject) -> PyResult { + unsafe { + match 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. @@ -322,10 +332,14 @@ mod tests { fn test_set_discard() { Python::with_gil(|py| { let set = PySet::new(py, &[1]).unwrap(); - set.discard(2); + assert!(!set.discard(2).unwrap()); assert_eq!(1, set.len()); - set.discard(1); + + assert!(set.discard(1).unwrap()); assert_eq!(0, set.len()); + assert!(!set.discard(1).unwrap()); + + assert!(set.discard(vec![1, 2]).is_err()); }); }