Skip to content

Simplify callback code using callback_body! macro #901

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 5, 2020
Merged

Simplify callback code using callback_body! macro #901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions pyo3-derive-backend/src/module.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,16 +219,11 @@ fn function_c_wrapper(name: &Ident, spec: &method::FnSpec<'_>) -> TokenStream {
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
{
const _LOCATION: &'static str = concat!(stringify!(#name), "()");

let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body!(_py, {
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);

#body

pyo3::callback::convert(_py, _result)
})
}
}
Expand Down
87 changes: 24 additions & 63 deletions pyo3-derive-backend/src/pymethod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ fn impl_wrap_common(
{
const _LOCATION: &'static str = concat!(
stringify!(#cls), ".", stringify!(#python_name), "()");
let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body_without_convert!(_py, {
#slf
pyo3::callback::convert(_py, #body)
})
Expand All @@ -124,16 +122,12 @@ fn impl_wrap_common(
{
const _LOCATION: &'static str = concat!(
stringify!(#cls), ".", stringify!(#python_name), "()");
let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body_without_convert!(_py, {
#slf
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);

#body

pyo3::callback::convert(_py, _result)
pyo3::callback::convert(_py, #body)
})
}
}
Expand All @@ -155,17 +149,13 @@ pub fn impl_proto_wrap(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body_without_convert!(_py, {
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
#borrow_self
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);

#body

pyo3::callback::convert(_py, _result)
pyo3::callback::convert(_py, #body)
})
}
}
Expand All @@ -177,11 +167,7 @@ pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
let python_name = &spec.python_name;
let names: Vec<syn::Ident> = get_arg_names(&spec);
let cb = quote! { #cls::#name(#(#names),*) };
let body = impl_arg_params_(
spec,
cb,
quote! { pyo3::derive_utils::IntoPyNewResult::into_pynew_result },
);
let body = impl_arg_params(spec, cb);

quote! {
#[allow(unused_mut)]
Expand All @@ -193,14 +179,11 @@ pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
use pyo3::type_object::PyTypeInfo;

const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body_without_convert!(_py, {
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);

#body

let _result = pyo3::derive_utils::IntoPyNewResult::into_pynew_result(#body);
let cell = pyo3::PyClassInitializer::from(_result?).create_cell(_py)?;
Ok(cell as *mut pyo3::ffi::PyObject)
})
Expand All @@ -225,16 +208,12 @@ pub fn impl_wrap_class(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body_without_convert!(_py, {
let _cls = pyo3::types::PyType::from_type_ptr(_py, _cls as *mut pyo3::ffi::PyTypeObject);
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);

#body

pyo3::callback::convert(_py, _result)
pyo3::callback::convert(_py, #body)
})
}
}
Expand All @@ -257,15 +236,11 @@ pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body_without_convert!(_py, {
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);

#body

pyo3::callback::convert(_py, _result)
pyo3::callback::convert(_py, #body)
})
}
}
Expand Down Expand Up @@ -314,10 +289,7 @@ pub(crate) fn impl_wrap_getter(
_slf: *mut pyo3::ffi::PyObject, _: *mut ::std::os::raw::c_void) -> *mut pyo3::ffi::PyObject
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");

let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body_without_convert!(_py, {
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
#borrow_self
pyo3::callback::convert(_py, #getter_impl)
Expand All @@ -343,9 +315,9 @@ fn impl_call_setter(spec: &FnSpec) -> syn::Result<TokenStream> {

let name = &spec.name;
let fncall = if py_arg.is_some() {
quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_py, _val))?;)
quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_py, _val))?)
} else {
quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_val))?;)
quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_val))?)
};

Ok(fncall)
Expand All @@ -359,7 +331,7 @@ pub(crate) fn impl_wrap_setter(
let (python_name, setter_impl) = match property_type {
PropertyType::Descriptor(field) => {
let name = field.ident.as_ref().unwrap();
(name.unraw(), quote!(_slf.#name = _val;))
(name.unraw(), quote!({ _slf.#name = _val; }))
}
PropertyType::Function(spec) => (spec.python_name.clone(), impl_call_setter(&spec)?),
};
Expand All @@ -372,14 +344,13 @@ pub(crate) fn impl_wrap_setter(
_value: *mut pyo3::ffi::PyObject, _: *mut ::std::os::raw::c_void) -> pyo3::libc::c_int
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _pool = pyo3::GILPool::new();
let _py = _pool.python();
pyo3::run_callback(_py, || {
pyo3::callback_body_without_convert!(_py, {
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
#borrow_self
let _value = _py.from_borrowed_ptr::<pyo3::types::PyAny>(_value);
let _val = pyo3::FromPyObject::extract(_value)?;
pyo3::callback::convert(_py, {#setter_impl})

pyo3::callback::convert(_py, #setter_impl)
})
}
})
Expand All @@ -398,12 +369,10 @@ fn impl_call(_cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
quote! { _slf.#fname(#(#names),*) }
}

fn impl_arg_params_(spec: &FnSpec<'_>, body: TokenStream, into_result: TokenStream) -> TokenStream {
pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream {
if spec.args.is_empty() {
return quote! {
let _result = {
#into_result (#body)
};
#body
};
}

Expand Down Expand Up @@ -444,7 +413,7 @@ fn impl_arg_params_(spec: &FnSpec<'_>, body: TokenStream, into_result: TokenStre
}
let num_normal_params = params.len();
// create array of arguments, and then parse
quote! {
quote! {{
use pyo3::ObjectProtocol;
const PARAMS: &'static [pyo3::derive_utils::ParamDescription] = &[
#(#params),*
Expand All @@ -466,16 +435,8 @@ fn impl_arg_params_(spec: &FnSpec<'_>, body: TokenStream, into_result: TokenStre

#(#param_conversion)*

let _result = #into_result(#body);
}
}

pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream {
impl_arg_params_(
spec,
body,
quote! { pyo3::derive_utils::IntoPyResult::into_py_result },
)
#body
}}
}

/// Re option_pos: The option slice doesn't contain the py: Python argument, so the argument
Expand Down
69 changes: 60 additions & 9 deletions src/callback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,65 @@ where
T::ERR_VALUE
}

/// Use this macro for all internal callback functions which Python will call.
///
/// It sets up the GILPool and converts the output into a Python object. It also restores
/// any python error returned as an Err variant from the body.
///
/// # Safety
/// This macro assumes the GIL is held. (It makes use of unsafe code, so usage of it is only
/// possible inside unsafe blocks.)
#[doc(hidden)]
pub fn run_callback<T, F>(py: Python, callback: F) -> T
where
F: FnOnce() -> PyResult<T>,
T: PyCallbackOutput,
{
callback().unwrap_or_else(|e| {
e.restore(py);
T::ERR_VALUE
})
#[macro_export]
macro_rules! callback_body {
($py:ident, $body:expr) => {{
$crate::callback_body_without_convert!($py, $crate::callback::convert($py, $body))
}};
}

/// Variant of the above which does not perform the callback conversion. This allows the callback
/// conversion to be done manually in the case where lifetimes might otherwise cause issue.
///
/// For example this pyfunction:
///
/// ```ignore
/// fn foo(&self) -> &Bar {
/// &self.bar
/// }
/// ```
///
/// It is wrapped in proc macros with callback_body_without_convert like so:
///
/// ```ignore
/// pyo3::callback_body_without_convert!(py, {
/// let _slf = #slf;
/// pyo3::callback::convert(py, #foo)
/// })
/// ```
///
/// If callback_body was used instead:
///
/// ```ignore
/// pyo3::callback_body!(py, {
/// let _slf = #slf;
/// #foo
/// })
/// ```
///
/// Then this will fail to compile, because the result of #foo borrows _slf, but _slf drops when
/// the block passed to the macro ends.
#[doc(hidden)]
#[macro_export]
macro_rules! callback_body_without_convert {
($py:ident, $body:expr) => {{
let pool = $crate::GILPool::new();

let $py = pool.python();
let callback = move || -> $crate::PyResult<_> { $body };

callback().unwrap_or_else(|e| {
e.restore(pool.python());
$crate::callback::callback_error()
})
}};
}
19 changes: 7 additions & 12 deletions src/class/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
use crate::callback::HashCallbackOutput;
use crate::class::methods::PyMethodDef;
use crate::{
callback, exceptions, ffi, run_callback, FromPyObject, GILPool, IntoPy, ObjectProtocol, PyAny,
PyCell, PyClass, PyErr, PyObject, PyResult,
exceptions, ffi, FromPyObject, IntoPy, ObjectProtocol, PyAny, PyCell, PyClass, PyErr, PyObject,
PyResult,
};
use std::os::raw::c_int;

Expand Down Expand Up @@ -218,9 +218,7 @@ where
where
T: for<'p> PyObjectGetAttrProtocol<'p>,
{
let pool = GILPool::new();
let py = pool.python();
run_callback(py, || {
crate::callback_body!(py, {
// Behave like python's __getattr__ (as opposed to __getattribute__) and check
// for existing fields and methods first
let existing = ffi::PyObject_GenericGetAttr(slf, arg);
Expand All @@ -233,7 +231,7 @@ where

let slf = py.from_borrowed_ptr::<PyCell<T>>(slf);
let arg = py.from_borrowed_ptr::<PyAny>(arg);
callback::convert(py, call_ref!(slf, __getattr__, arg))
call_ref!(slf, __getattr__, arg)
})
}
Some(wrap::<T>)
Expand Down Expand Up @@ -484,17 +482,14 @@ where
where
T: for<'p> PyObjectRichcmpProtocol<'p>,
{
let pool = GILPool::new();
let py = pool.python();
run_callback(py, || {
crate::callback_body!(py, {
let slf = py.from_borrowed_ptr::<crate::PyCell<T>>(slf);
let arg = py.from_borrowed_ptr::<PyAny>(arg);

let borrowed_slf = slf.try_borrow()?;
let op = extract_op(op)?;
let arg = arg.extract()?;
let result = borrowed_slf.__richcmp__(arg, op).into();
callback::convert(py, result)

slf.try_borrow()?.__richcmp__(arg, op).into()
})
}
Some(wrap::<T>)
Expand Down
17 changes: 5 additions & 12 deletions src/class/buffer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
//! For more information check [buffer protocol](https://docs.python.org/3/c-api/buffer.html)
//! c-api
use crate::err::PyResult;
use crate::gil::GILPool;
use crate::{callback, ffi, run_callback, PyCell, PyClass, PyRefMut};
use crate::{ffi, PyCell, PyClass, PyRefMut};
use std::os::raw::c_int;

/// Buffer protocol interface
Expand Down Expand Up @@ -91,12 +90,9 @@ where
where
T: for<'p> PyBufferGetBufferProtocol<'p>,
{
let pool = GILPool::new();
let py = pool.python();
run_callback(py, || {
crate::callback_body!(py, {
let slf = py.from_borrowed_ptr::<PyCell<T>>(slf);
let result = T::bf_getbuffer(slf.try_borrow_mut()?, arg1, arg2).into();
callback::convert(py, result)
T::bf_getbuffer(slf.try_borrow_mut()?, arg1, arg2).into()
})
}
Some(wrap::<T>)
Expand Down Expand Up @@ -126,12 +122,9 @@ where
where
T: for<'p> PyBufferReleaseBufferProtocol<'p>,
{
let pool = GILPool::new();
let py = pool.python();
run_callback(py, || {
crate::callback_body!(py, {
let slf = py.from_borrowed_ptr::<crate::PyCell<T>>(slf);
let result = T::bf_releasebuffer(slf.try_borrow_mut()?, arg1).into();
crate::callback::convert(py, result)
T::bf_releasebuffer(slf.try_borrow_mut()?, arg1).into()
})
}
Some(wrap::<T>)
Expand Down
Loading