Skip to content

Commit

Permalink
Merge pull request #858 from davidhewitt/refactor-callbacks
Browse files Browse the repository at this point in the history 
Refactor CallbackConverter code
  • Loading branch information
@kngwyu
kngwyu authored Apr 9, 2020
2 parents a9357ef + d8effb2 commit 724a3fc
Show file tree
Hide file tree
Showing 17 changed files with 528 additions and 834 deletions.
2 changes: 1 addition & 1 deletion pyo3-derive-backend/src/method.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl<'a> FnSpec<'a> {
let is_mut = self
.self_
.expect("impl_borrow_self is called for non-self fn");
crate::utils::borrow_self(is_mut, true)
crate::utils::borrow_self(is_mut)
}

/// Parser function signature and function attributes
Expand Down
12 changes: 7 additions & 5 deletions pyo3-derive-backend/src/module.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,13 +221,15 @@ fn function_c_wrapper(name: &Ident, spec: &method::FnSpec<'_>) -> TokenStream {
const _LOCATION: &'static str = concat!(stringify!(#name), "()");

let _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_py);
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_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
#body

pyo3::callback::cb_obj_convert(_py, _result)
pyo3::callback::convert(_py, _result)
})
}
}
}
167 changes: 78 additions & 89 deletions pyo3-derive-backend/src/pymethod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,7 @@ pub fn impl_wrap_pyslf(
};
let slf = quote! {
let _cell = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
let _slf: #self_ty = match std::convert::TryFrom::try_from(_cell) {
Ok(_slf) => _slf,
Err(e) => return pyo3::PyErr::from(e).restore_and_null(_py),
};
let _slf: #self_ty = std::convert::TryFrom::try_from(_cell)?;
};
impl_wrap_common(cls, spec, noargs, slf, body)
}
Expand All @@ -109,13 +106,11 @@ fn impl_wrap_common(
const _LOCATION: &'static str = concat!(
stringify!(#cls), ".", stringify!(#python_name), "()");
let _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_py);
#slf
let _result = {
pyo3::derive_utils::IntoPyResult::into_py_result(#body)
};

pyo3::callback::cb_obj_convert(_py, _result)
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_py);
#slf
pyo3::callback::convert(_py, #body)
})
}
}
} else {
Expand All @@ -130,14 +125,16 @@ fn impl_wrap_common(
const _LOCATION: &'static str = concat!(
stringify!(#cls), ".", stringify!(#python_name), "()");
let _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_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);
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_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
#body

pyo3::callback::cb_obj_convert(_py, _result)
pyo3::callback::convert(_py, _result)
})
}
}
}
Expand All @@ -159,15 +156,17 @@ pub fn impl_proto_wrap(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_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);
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_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
#body

pyo3::callback::cb_obj_convert(_py, _result)
pyo3::callback::convert(_py, _result)
})
}
}
}
Expand Down Expand Up @@ -195,16 +194,16 @@ pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {

const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_py);
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_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
#body

match _result.and_then(|init| pyo3::PyClassInitializer::from(init).create_cell(_py)) {
Ok(slf) => slf as _,
Err(e) => e.restore_and_null(_py),
}
let cell = pyo3::PyClassInitializer::from(_result?).create_cell(_py)?;
Ok(cell as *mut pyo3::ffi::PyObject)
})
}
}
}
Expand All @@ -227,14 +226,16 @@ pub fn impl_wrap_class(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_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);
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_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
#body

pyo3::callback::cb_obj_convert(_py, _result)
pyo3::callback::convert(_py, _result)
})
}
}
}
Expand All @@ -257,13 +258,15 @@ pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_py);
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_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
#body

pyo3::callback::cb_obj_convert(_py, _result)
pyo3::callback::convert(_py, _result)
})
}
}
}
Expand Down Expand Up @@ -305,24 +308,20 @@ pub(crate) fn impl_wrap_getter(
PropertyType::Function(spec) => (spec.python_name.clone(), impl_call_getter(&spec)?),
};

let borrow_self = crate::utils::borrow_self(false, true);
let borrow_self = crate::utils::borrow_self(false);
Ok(quote! {
unsafe extern "C" fn __wrap(
_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 _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_py);
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
#borrow_self

let result = pyo3::derive_utils::IntoPyResult::into_py_result(#getter_impl);

match result {
Ok(val) => pyo3::IntoPyPointer::into_ptr(pyo3::IntoPy::<PyObject>::into_py(val, _py)),
Err(e) => e.restore_and_null(_py),
}
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_py);
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
#borrow_self
pyo3::callback::convert(_py, #getter_impl)
})
}
})
}
Expand All @@ -344,9 +343,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 @@ -360,12 +359,12 @@ 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; Ok(()) }))
(name.unraw(), quote!(_slf.#name = _val;))
}
PropertyType::Function(spec) => (spec.python_name.clone(), impl_call_setter(&spec)?),
};

let borrow_self = crate::utils::borrow_self(true, false);
let borrow_self = crate::utils::borrow_self(true);
Ok(quote! {
#[allow(unused_mut)]
unsafe extern "C" fn __wrap(
Expand All @@ -374,21 +373,14 @@ pub(crate) fn impl_wrap_setter(
{
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
let _py = pyo3::Python::assume_gil_acquired();
let _pool = pyo3::GILPool::new(_py);
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
#borrow_self
let _value = _py.from_borrowed_ptr(_value);

let _result = match pyo3::FromPyObject::extract(_value) {
Ok(_val) => {
#setter_impl
}
Err(e) => Err(e)
};
match _result {
Ok(_) => 0,
Err(e) => e.restore_and_minus1(_py),
}
pyo3::run_callback(_py, || {
let _pool = pyo3::GILPool::new(_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})
})
}
})
}
Expand Down Expand Up @@ -462,22 +454,19 @@ fn impl_arg_params_(spec: &FnSpec<'_>, body: TokenStream, into_result: TokenStre
let mut _args = _args;
let mut _kwargs = _kwargs;

// Workaround to use the question mark operator without rewriting everything
let _result = (|| {
let (_args, _kwargs) = pyo3::derive_utils::parse_fn_args(
Some(_LOCATION),
PARAMS,
_args,
_kwargs,
#accept_args,
#accept_kwargs,
&mut output
)?;

#(#param_conversion)*

#into_result(#body)
})();
let (_args, _kwargs) = pyo3::derive_utils::parse_fn_args(
Some(_LOCATION),
PARAMS,
_args,
_kwargs,
#accept_args,
#accept_kwargs,
&mut output
)?;

#(#param_conversion)*

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

Expand Down
17 changes: 3 additions & 14 deletions pyo3-derive-backend/src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,14 @@ use proc_macro2::TokenStream;
use quote::quote;
use std::fmt::Display;

pub(crate) fn borrow_self(is_mut: bool, return_null: bool) -> TokenStream {
let ret = if return_null {
quote! { restore_and_null }
} else {
quote! { restore_and_minus1 }
};
pub(crate) fn borrow_self(is_mut: bool) -> TokenStream {
if is_mut {
quote! {
let mut _slf = match _slf.try_borrow_mut() {
Ok(ref_) => ref_,
Err(e) => return pyo3::PyErr::from(e).#ret(_py),
};
let mut _slf = _slf.try_borrow_mut()?;
}
} else {
quote! {
let _slf = match _slf.try_borrow() {
Ok(ref_) => ref_,
Err(e) => return pyo3::PyErr::from(e).#ret(_py),
};
let _slf = _slf.try_borrow()?;
}
}
}
Expand Down
Loading

0 comments on commit 724a3fc

Please sign in to comment.