Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add PyDict::get_item_with_error for PyPy #3270

Merged
merged 2 commits into from Jun 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/3270.added.md
@@ -0,0 +1 @@
Add `PyDict::get_item_with_error` on PyPy.
1 change: 1 addition & 0 deletions pyo3-ffi/src/dictobject.rs
Expand Up @@ -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;
Expand Down
39 changes: 23 additions & 16 deletions src/types/dict.rs
Expand Up @@ -2,10 +2,9 @@ use super::PyMapping;
use crate::err::{self, PyErr, PyResult};
use crate::ffi::Py_ssize_t;
use crate::types::{PyAny, PyList};
use crate::{ffi, AsPyPointer, Python, ToPyObject};
#[cfg(not(PyPy))]
use crate::{IntoPyPointer, PyObject};
use std::ptr::NonNull;
use crate::IntoPyPointer;
use crate::{ffi, AsPyPointer, PyObject, Python, ToPyObject};

/// Represents a Python `dict`.
#[repr(transparent)]
Expand Down Expand Up @@ -143,12 +142,16 @@ impl PyDict {
where
K: ToPyObject,
{
self.get_item_impl(key.to_object(self.py()))
}

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

Expand All @@ -157,19 +160,23 @@ 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,
{
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()));
}
self.get_item_with_error_impl(key.to_object(self.py()))
}

Ok(NonNull::new(ptr).map(|p| self.py().from_owned_ptr(ffi::_Py_NewRef(p.as_ptr()))))
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.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.
PyObject::from_borrowed_ptr_or_opt(py, ptr)
.map(|pyobject| Ok(pyobject.into_ref(py)))
.or_else(|| PyErr::take(py).map(Err))
.transpose()
}
}

Expand Down