Skip to content

Commit

Permalink
fixup borrowed object
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Dec 14, 2023
1 parent 1a1c8c8 commit 9ed701d
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 26 deletions.
2 changes: 1 addition & 1 deletion pyo3-macros-backend/src/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub fn impl_arg_params(
.collect::<Result<_>>()?;
return Ok((
quote! {
let _args = _pyo3::impl_::extract_argument::PyArg::from_ptr_unchecked(py, _args);
let _args = _pyo3::impl_::extract_argument::PyArg::from_ptr(py, _args);
let _kwargs = _pyo3::impl_::extract_argument::PyArg::from_ptr_or_opt(py, _kwargs);
},
arg_convert,
Expand Down
75 changes: 59 additions & 16 deletions src/impl_/extract_argument.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,52 @@ use crate::{
},
PyAny, PyClass, PyErr, PyRef, PyRefMut, PyResult, PyTypeCheck, Python,
};
use crate::{PyObject, ToPyObject};

/// Function argument extraction borrows input arguments.
pub type PyArg<'py> = Py2Borrowed<'py, 'py, PyAny>;
///
/// TODO maybe change this to be pub type instead of a wrapper when Py2Borrowed is public
#[derive(Copy, Clone)]
#[repr(transparent)]
pub struct PyArg<'py>(Py2Borrowed<'py, 'py, PyAny>);

impl<'py> PyArg<'py> {
pub unsafe fn from_ptr_or_opt(py: Python<'py>, ptr: *mut ffi::PyObject) -> Option<Self> {
Py2Borrowed::from_ptr_or_opt(py, ptr).map(Self)
}

pub unsafe fn from_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self {
Self(Py2Borrowed::from_ptr(py, ptr))
}

pub fn from_gil_ref(instance: &'py PyAny) -> Self {
Self(Py2Borrowed::from_gil_ref(instance))
}

pub fn as_gil_ref(self) -> &'py PyAny {
self.0.as_gil_ref()
}
}

// These two `From` implementations help make various macro code pieces compile
impl<'py, T> From<&'py Py2<'py, T>> for PyArg<'py> {
fn from(other: &'py Py2<'py, T>) -> Self {
Self(Py2Borrowed::from(other))
}
}

impl<'py> From<&'_ Self> for PyArg<'py> {
fn from(other: &'_ Self) -> Self {
Self(other.0)
}
}

impl ToPyObject for PyArg<'_> {
#[inline]
fn to_object(&self, py: Python<'_>) -> PyObject {
self.0.to_object(py)
}
}

/// A trait which is used to help PyO3 macros extract function arguments.
///
Expand All @@ -35,7 +78,7 @@ where

#[inline]
fn extract(obj: PyArg<'py>, _: &'a mut ()) -> PyResult<Self> {
obj.as_gil_ref().extract()
obj.0.as_gil_ref().extract()
}
}

Expand All @@ -47,7 +90,7 @@ where

#[inline]
fn extract(obj: PyArg<'py>, holder: &'a mut Option<Py2<'py, T>>) -> PyResult<Self> {
Ok(&*holder.insert(obj.extract()?))
Ok(&*holder.insert(obj.0.extract()?))
}
}

Expand All @@ -70,15 +113,15 @@ pub fn extract_pyclass_ref<'a, 'py: 'a, T: PyClass>(
obj: PyArg<'py>,
holder: &'a mut Option<PyRef<'py, T>>,
) -> PyResult<&'a T> {
Ok(&*holder.insert(obj.extract()?))
Ok(&*holder.insert(obj.0.extract()?))
}

#[inline]
pub fn extract_pyclass_ref_mut<'a, 'py: 'a, T: PyClass<Frozen = False>>(
obj: PyArg<'py>,
holder: &'a mut Option<PyRefMut<'py, T>>,
) -> PyResult<&'a mut T> {
Ok(&mut *holder.insert(obj.extract()?))
Ok(&mut *holder.insert(obj.0.extract()?))
}

/// The standard implementation of how PyO3 extracts a `#[pyfunction]` or `#[pymethod]` function argument.
Expand All @@ -93,7 +136,7 @@ where
{
match PyFunctionArgument::extract(obj, holder) {
Ok(value) => Ok(value),
Err(e) => Err(argument_extraction_error(obj.py(), arg_name, e)),
Err(e) => Err(argument_extraction_error(obj.0.py(), arg_name, e)),
}
}

Expand All @@ -111,7 +154,7 @@ where
{
match obj {
Some(obj) => {
if obj.is_none() {
if obj.0.is_none() {
// Explicit `None` will result in None being used as the function argument
Ok(None)
} else {
Expand Down Expand Up @@ -146,9 +189,9 @@ pub fn from_py_with<'py, T>(
arg_name: &str,
extractor: fn(&'py PyAny) -> PyResult<T>,
) -> PyResult<T> {
match extractor(obj.as_gil_ref()) {
match extractor(obj.0.as_gil_ref()) {
Ok(value) => Ok(value),
Err(e) => Err(argument_extraction_error(obj.py(), arg_name, e)),
Err(e) => Err(argument_extraction_error(obj.0.py(), arg_name, e)),
}
}

Expand Down Expand Up @@ -286,7 +329,7 @@ impl FunctionDescription {
);

self.handle_kwargs::<K, _>(
kwnames.iter().zip(kwargs.iter().copied()),
kwnames.iter().map(PyArg).zip(kwargs.iter().copied()),
&mut varkeywords,
num_positional_parameters,
output,
Expand Down Expand Up @@ -341,7 +384,7 @@ impl FunctionDescription {

// Copy positional arguments into output
for (i, arg) in args.iter().take(num_positional_parameters).enumerate() {
output[i] = Some(arg);
output[i] = Some(PyArg(arg));
}

// If any arguments remain, push them to varargs (if possible) or error
Expand All @@ -351,7 +394,7 @@ impl FunctionDescription {
let mut varkeywords = K::Varkeywords::default();
if let Some(kwargs) = kwargs {
self.handle_kwargs::<K, _>(
kwargs.iter_borrowed(),
kwargs.iter_borrowed().map(|(k, v)| (PyArg(k), PyArg(v))),
&mut varkeywords,
num_positional_parameters,
output,
Expand Down Expand Up @@ -392,7 +435,7 @@ impl FunctionDescription {
// If it isn't, then it will be handled below as a varkeyword (which may raise an
// error if this function doesn't accept **kwargs). Rust source is always UTF-8
// and so all argument names in `#[pyfunction]` signature must be UTF-8.
if let Ok(kwarg_name) = kwarg_name_py.downcast::<PyString>()?.to_str() {
if let Ok(kwarg_name) = kwarg_name_py.0.downcast::<PyString>()?.to_str() {
// Try to place parameter in keyword only parameters
if let Some(i) = self.find_keyword_parameter_in_keyword_only(kwarg_name) {
if output[i + num_positional_parameters]
Expand Down Expand Up @@ -515,7 +558,7 @@ impl FunctionDescription {
PyTypeError::new_err(format!(
"{} got an unexpected keyword argument '{}'",
self.full_name(),
argument.as_any()
argument.0.as_any()
))
}

Expand All @@ -529,7 +572,7 @@ impl FunctionDescription {
&mut msg,
parameter_names
.iter()
.map(|a| a.downcast::<PyString>().unwrap().to_str().unwrap()),
.map(|a| a.0.downcast::<PyString>().unwrap().to_str().unwrap()),
);
PyTypeError::new_err(msg)
}
Expand Down Expand Up @@ -706,7 +749,7 @@ impl<'py> VarkeywordsHandler<'py> for DictVarkeywords {
_function_description: &FunctionDescription,
) -> PyResult<()> {
varkeywords
.get_or_insert_with(|| PyDict::new2(name.py()))
.get_or_insert_with(|| PyDict::new2(name.0.py()))
.set_item(name, value)
}
}
Expand Down
11 changes: 2 additions & 9 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,13 +477,6 @@ impl<'a, 'py, T> From<&'a Py2<'py, T>> for Py2Borrowed<'a, 'py, PyAny> {
}
}

impl<'a, 'py> From<&'_ Self> for Py2Borrowed<'a, 'py, PyAny> {
/// Copy the pointer (useful in macro code)
fn from(other: &'_ Self) -> Self {
*other
}
}

impl<'py, T> Py2Borrowed<'_, 'py, T> {
pub(crate) fn clone_ref(self) -> Py2<'py, T> {
unsafe {
Expand All @@ -494,15 +487,15 @@ impl<'py, T> Py2Borrowed<'_, 'py, T> {

impl<'py> Py2Borrowed<'py, 'py, PyAny> {
#[doc(hidden)] // Used in macro code.
pub fn from_gil_ref(instance: &'py PyAny) -> Self {
pub(crate) fn from_gil_ref(instance: &'py PyAny) -> Self {
Self(
unsafe { NonNull::new_unchecked(instance.as_ptr()) },
PhantomData,
)
}

#[doc(hidden)] // Used in macro code.
pub fn as_gil_ref(self) -> &'py PyAny {
pub(crate) fn as_gil_ref(self) -> &'py PyAny {
// safety: &'py PyAny has the same layout as NonNull<ffi::PyObject>
unsafe { &*(self.0.as_ptr().cast()) }
}
Expand Down

0 comments on commit 9ed701d

Please sign in to comment.