add PyDict::get_item_with_error for PyPy

newsfragments/3270.added.md

@@ -0,0 +1 @@
+Add `PyDict::get_item_with_error` on PyPy.

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;

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<K>(&self, key: K) -> PyResult<Option<&PyAny>>
     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<Option<&PyAny>> {
+        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()))))
         }
     }