diff --git a/newsfragments/3270.added.md b/newsfragments/3270.added.md new file mode 100644 index 00000000000..66b2db9d83d --- /dev/null +++ b/newsfragments/3270.added.md @@ -0,0 +1 @@ +Add `PyDict::get_item_with_error` on PyPy. diff --git a/pyo3-ffi/src/dictobject.rs b/pyo3-ffi/src/dictobject.rs index 3215d882c5c..8d522df97e2 100644 --- a/pyo3-ffi/src/dictobject.rs +++ b/pyo3-ffi/src/dictobject.rs @@ -24,6 +24,7 @@ extern "C" { pub fn PyDict_New() -> *mut PyObject; #[cfg_attr(PyPy, link_name = "PyPyDict_GetItem")] pub fn PyDict_GetItem(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject; + #[cfg_attr(PyPy, link_name = "PyPyDict_GetItemWithError")] pub fn PyDict_GetItemWithError(mp: *mut PyObject, key: *mut PyObject) -> *mut PyObject; #[cfg_attr(PyPy, link_name = "PyPyDict_SetItem")] pub fn PyDict_SetItem(mp: *mut PyObject, key: *mut PyObject, item: *mut PyObject) -> c_int; diff --git a/src/types/dict.rs b/src/types/dict.rs index cb1036cf946..27b1a6e2003 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -5,7 +5,6 @@ use crate::types::{PyAny, PyList}; use crate::{ffi, AsPyPointer, Python, ToPyObject}; #[cfg(not(PyPy))] use crate::{IntoPyPointer, PyObject}; -use std::ptr::NonNull; /// Represents a Python `dict`. #[repr(transparent)] @@ -143,12 +142,17 @@ impl PyDict { where K: ToPyObject, { + let key = key.to_object(self.py()); + self.get_item_impl(key) + } + + fn get_item_impl(&self, key: PyObject) -> Option<&PyAny> { + let py = self.py(); unsafe { - let ptr = ffi::PyDict_GetItem(self.as_ptr(), key.to_object(self.py()).as_ptr()); - NonNull::new(ptr).map(|p| { - // PyDict_GetItem return s borrowed ptr, must make it owned for safety (see #890). - self.py().from_owned_ptr(ffi::_Py_NewRef(p.as_ptr())) - }) + let ptr = ffi::PyDict_GetItem(self.as_ptr(), key.as_ptr()); + // PyDict_GetItem returns a borrowed ptr, must make it owned for safety (see #890). + // PyObject::from_borrowed_ptr_or_opt will take ownership in this way. + PyObject::from_borrowed_ptr_or_opt(py, ptr).map(|pyobject| pyobject.into_ref(py)) } } @@ -157,19 +161,24 @@ impl PyDict { /// returns `Ok(None)` if item is not present, or `Err(PyErr)` if an error occurs. /// /// To get a `KeyError` for non-existing keys, use `PyAny::get_item_with_error`. - #[cfg(not(PyPy))] pub fn get_item_with_error(&self, key: K) -> PyResult> where K: ToPyObject, { + let key = key.to_object(self.py()); + self.get_item_with_error_impl(key) + } + + fn get_item_with_error_impl(&self, key: PyObject) -> PyResult> { + let py = self.py(); unsafe { - let ptr = - ffi::PyDict_GetItemWithError(self.as_ptr(), key.to_object(self.py()).as_ptr()); - if !ffi::PyErr_Occurred().is_null() { - return Err(PyErr::fetch(self.py())); + let ptr = ffi::PyDict_GetItemWithError(self.as_ptr(), key.as_ptr()); + // PyDict_GetItemWithError returns a borrowed ptr, must make it owned for safety (see #890). + // PyObject::from_borrowed_ptr_or_opt will take ownership in this way. + match PyObject::from_borrowed_ptr_or_opt(py, ptr) { + Some(pyobject) => Ok(Some(pyobject.into_ref(py))), + None => PyErr::take(py).map(Err).transpose(), } - - Ok(NonNull::new(ptr).map(|p| self.py().from_owned_ptr(ffi::_Py_NewRef(p.as_ptr())))) } }