From 6d1f025c61d5466684e987565cb8f70ef9cc4e53 Mon Sep 17 00:00:00 2001
From: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
Date: Sun, 25 Jun 2023 17:01:48 +0100
Subject: [PATCH] add `PyDict::get_item_with_error` for PyPy

---
 newsfragments/3270.added.md |  1 +
 pyo3-ffi/src/dictobject.rs  |  1 +
 src/types/dict.rs           | 35 ++++++++++++++++++++++-------------
 3 files changed, 24 insertions(+), 13 deletions(-)
 create mode 100644 newsfragments/3270.added.md

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<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()))))
         }
     }