Skip to content

Commit 9f18610

Browse files
kngwyukngwyu
authored
Merge pull request #901 from davidhewitt/simplify-callbacks
Simplify callback code using callback_body! macro
2 parents a19ed50 + 9a2d908 commit 9f18610

File tree

9 files changed

+142
-200
lines changed

9 files changed

+142
-200
lines changed

‎pyo3-derive-backend/src/module.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,16 +219,11 @@ fn function_c_wrapper(name: &Ident, spec: &method::FnSpec<'_>) -> TokenStream {
219219
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
220220
{
221221
const _LOCATION: &'static str = concat!(stringify!(#name), "()");
222-
223-
let _pool = pyo3::GILPool::new();
224-
let _py = _pool.python();
225-
pyo3::run_callback(_py, || {
222+
pyo3::callback_body!(_py, {
226223
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
227224
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
228225

229226
#body
230-
231-
pyo3::callback::convert(_py, _result)
232227
})
233228
}
234229
}

‎pyo3-derive-backend/src/pymethod.rs

Lines changed: 24 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,7 @@ fn impl_wrap_common(
105105
{
106106
const _LOCATION: &'static str = concat!(
107107
stringify!(#cls), ".", stringify!(#python_name), "()");
108-
let _pool = pyo3::GILPool::new();
109-
let _py = _pool.python();
110-
pyo3::run_callback(_py, || {
108+
pyo3::callback_body_without_convert!(_py, {
111109
#slf
112110
pyo3::callback::convert(_py, #body)
113111
})
@@ -124,16 +122,12 @@ fn impl_wrap_common(
124122
{
125123
const _LOCATION: &'static str = concat!(
126124
stringify!(#cls), ".", stringify!(#python_name), "()");
127-
let _pool = pyo3::GILPool::new();
128-
let _py = _pool.python();
129-
pyo3::run_callback(_py, || {
125+
pyo3::callback_body_without_convert!(_py, {
130126
#slf
131127
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
132128
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
133129

134-
#body
135-
136-
pyo3::callback::convert(_py, _result)
130+
pyo3::callback::convert(_py, #body)
137131
})
138132
}
139133
}
@@ -155,17 +149,13 @@ pub fn impl_proto_wrap(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
155149
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
156150
{
157151
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
158-
let _pool = pyo3::GILPool::new();
159-
let _py = _pool.python();
160-
pyo3::run_callback(_py, || {
152+
pyo3::callback_body_without_convert!(_py, {
161153
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
162154
#borrow_self
163155
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
164156
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
165157

166-
#body
167-
168-
pyo3::callback::convert(_py, _result)
158+
pyo3::callback::convert(_py, #body)
169159
})
170160
}
171161
}
@@ -177,11 +167,7 @@ pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
177167
let python_name = &spec.python_name;
178168
let names: Vec<syn::Ident> = get_arg_names(&spec);
179169
let cb = quote! { #cls::#name(#(#names),*) };
180-
let body = impl_arg_params_(
181-
spec,
182-
cb,
183-
quote! { pyo3::derive_utils::IntoPyNewResult::into_pynew_result },
184-
);
170+
let body = impl_arg_params(spec, cb);
185171

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

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

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

235-
#body
236-
237-
pyo3::callback::convert(_py, _result)
216+
pyo3::callback::convert(_py, #body)
238217
})
239218
}
240219
}
@@ -257,15 +236,11 @@ pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
257236
_kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject
258237
{
259238
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
260-
let _pool = pyo3::GILPool::new();
261-
let _py = _pool.python();
262-
pyo3::run_callback(_py, || {
239+
pyo3::callback_body_without_convert!(_py, {
263240
let _args = _py.from_borrowed_ptr::<pyo3::types::PyTuple>(_args);
264241
let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
265242

266-
#body
267-
268-
pyo3::callback::convert(_py, _result)
243+
pyo3::callback::convert(_py, #body)
269244
})
270245
}
271246
}
@@ -314,10 +289,7 @@ pub(crate) fn impl_wrap_getter(
314289
_slf: *mut pyo3::ffi::PyObject, _: *mut ::std::os::raw::c_void) -> *mut pyo3::ffi::PyObject
315290
{
316291
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
317-
318-
let _pool = pyo3::GILPool::new();
319-
let _py = _pool.python();
320-
pyo3::run_callback(_py, || {
292+
pyo3::callback_body_without_convert!(_py, {
321293
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
322294
#borrow_self
323295
pyo3::callback::convert(_py, #getter_impl)
@@ -343,9 +315,9 @@ fn impl_call_setter(spec: &FnSpec) -> syn::Result<TokenStream> {
343315

344316
let name = &spec.name;
345317
let fncall = if py_arg.is_some() {
346-
quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_py, _val))?;)
318+
quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_py, _val))?)
347319
} else {
348-
quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_val))?;)
320+
quote!(pyo3::derive_utils::IntoPyResult::into_py_result(_slf.#name(_val))?)
349321
};
350322

351323
Ok(fncall)
@@ -359,7 +331,7 @@ pub(crate) fn impl_wrap_setter(
359331
let (python_name, setter_impl) = match property_type {
360332
PropertyType::Descriptor(field) => {
361333
let name = field.ident.as_ref().unwrap();
362-
(name.unraw(), quote!(_slf.#name = _val;))
334+
(name.unraw(), quote!({ _slf.#name = _val; }))
363335
}
364336
PropertyType::Function(spec) => (spec.python_name.clone(), impl_call_setter(&spec)?),
365337
};
@@ -372,14 +344,13 @@ pub(crate) fn impl_wrap_setter(
372344
_value: *mut pyo3::ffi::PyObject, _: *mut ::std::os::raw::c_void) -> pyo3::libc::c_int
373345
{
374346
const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()");
375-
let _pool = pyo3::GILPool::new();
376-
let _py = _pool.python();
377-
pyo3::run_callback(_py, || {
347+
pyo3::callback_body_without_convert!(_py, {
378348
let _slf = _py.from_borrowed_ptr::<pyo3::PyCell<#cls>>(_slf);
379349
#borrow_self
380350
let _value = _py.from_borrowed_ptr::<pyo3::types::PyAny>(_value);
381351
let _val = pyo3::FromPyObject::extract(_value)?;
382-
pyo3::callback::convert(_py, {#setter_impl})
352+
353+
pyo3::callback::convert(_py, #setter_impl)
383354
})
384355
}
385356
})
@@ -398,12 +369,10 @@ fn impl_call(_cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
398369
quote! { _slf.#fname(#(#names),*) }
399370
}
400371

401-
fn impl_arg_params_(spec: &FnSpec<'_>, body: TokenStream, into_result: TokenStream) -> TokenStream {
372+
pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream {
402373
if spec.args.is_empty() {
403374
return quote! {
404-
let _result = {
405-
#into_result (#body)
406-
};
375+
#body
407376
};
408377
}
409378

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

467436
#(#param_conversion)*
468437

469-
let _result = #into_result(#body);
470-
}
471-
}
472-
473-
pub fn impl_arg_params(spec: &FnSpec<'_>, body: TokenStream) -> TokenStream {
474-
impl_arg_params_(
475-
spec,
476-
body,
477-
quote! { pyo3::derive_utils::IntoPyResult::into_py_result },
478-
)
438+
#body
439+
}}
479440
}
480441

481442
/// Re option_pos: The option slice doesn't contain the py: Python argument, so the argument

‎src/callback.rs

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,14 +152,65 @@ where
152152
T::ERR_VALUE
153153
}
154154

155+
/// Use this macro for all internal callback functions which Python will call.
156+
///
157+
/// It sets up the GILPool and converts the output into a Python object. It also restores
158+
/// any python error returned as an Err variant from the body.
159+
///
160+
/// # Safety
161+
/// This macro assumes the GIL is held. (It makes use of unsafe code, so usage of it is only
162+
/// possible inside unsafe blocks.)
155163
#[doc(hidden)]
156-
pub fn run_callback<T, F>(py: Python, callback: F) -> T
157-
where
158-
F: FnOnce() -> PyResult<T>,
159-
T: PyCallbackOutput,
160-
{
161-
callback().unwrap_or_else(|e| {
162-
e.restore(py);
163-
T::ERR_VALUE
164-
})
164+
#[macro_export]
165+
macro_rules! callback_body {
166+
($py:ident, $body:expr) => {{
167+
$crate::callback_body_without_convert!($py, $crate::callback::convert($py, $body))
168+
}};
169+
}
170+
171+
/// Variant of the above which does not perform the callback conversion. This allows the callback
172+
/// conversion to be done manually in the case where lifetimes might otherwise cause issue.
173+
///
174+
/// For example this pyfunction:
175+
///
176+
/// ```ignore
177+
/// fn foo(&self) -> &Bar {
178+
/// &self.bar
179+
/// }
180+
/// ```
181+
///
182+
/// It is wrapped in proc macros with callback_body_without_convert like so:
183+
///
184+
/// ```ignore
185+
/// pyo3::callback_body_without_convert!(py, {
186+
/// let _slf = #slf;
187+
/// pyo3::callback::convert(py, #foo)
188+
/// })
189+
/// ```
190+
///
191+
/// If callback_body was used instead:
192+
///
193+
/// ```ignore
194+
/// pyo3::callback_body!(py, {
195+
/// let _slf = #slf;
196+
/// #foo
197+
/// })
198+
/// ```
199+
///
200+
/// Then this will fail to compile, because the result of #foo borrows _slf, but _slf drops when
201+
/// the block passed to the macro ends.
202+
#[doc(hidden)]
203+
#[macro_export]
204+
macro_rules! callback_body_without_convert {
205+
($py:ident, $body:expr) => {{
206+
let pool = $crate::GILPool::new();
207+
208+
let $py = pool.python();
209+
let callback = move || -> $crate::PyResult<_> { $body };
210+
211+
callback().unwrap_or_else(|e| {
212+
e.restore(pool.python());
213+
$crate::callback::callback_error()
214+
})
215+
}};
165216
}

‎src/class/basic.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@
1111
use crate::callback::HashCallbackOutput;
1212
use crate::class::methods::PyMethodDef;
1313
use crate::{
14-
callback, exceptions, ffi, run_callback, FromPyObject, GILPool, IntoPy, ObjectProtocol, PyAny,
15-
PyCell, PyClass, PyErr, PyObject, PyResult,
14+
exceptions, ffi, FromPyObject, IntoPy, ObjectProtocol, PyAny, PyCell, PyClass, PyErr, PyObject,
15+
PyResult,
1616
};
1717
use std::os::raw::c_int;
1818

@@ -218,9 +218,7 @@ where
218218
where
219219
T: for<'p> PyObjectGetAttrProtocol<'p>,
220220
{
221-
let pool = GILPool::new();
222-
let py = pool.python();
223-
run_callback(py, || {
221+
crate::callback_body!(py, {
224222
// Behave like python's __getattr__ (as opposed to __getattribute__) and check
225223
// for existing fields and methods first
226224
let existing = ffi::PyObject_GenericGetAttr(slf, arg);
@@ -233,7 +231,7 @@ where
233231

234232
let slf = py.from_borrowed_ptr::<PyCell<T>>(slf);
235233
let arg = py.from_borrowed_ptr::<PyAny>(arg);
236-
callback::convert(py, call_ref!(slf, __getattr__, arg))
234+
call_ref!(slf, __getattr__, arg)
237235
})
238236
}
239237
Some(wrap::<T>)
@@ -484,17 +482,14 @@ where
484482
where
485483
T: for<'p> PyObjectRichcmpProtocol<'p>,
486484
{
487-
let pool = GILPool::new();
488-
let py = pool.python();
489-
run_callback(py, || {
485+
crate::callback_body!(py, {
490486
let slf = py.from_borrowed_ptr::<crate::PyCell<T>>(slf);
491487
let arg = py.from_borrowed_ptr::<PyAny>(arg);
492488

493-
let borrowed_slf = slf.try_borrow()?;
494489
let op = extract_op(op)?;
495490
let arg = arg.extract()?;
496-
let result = borrowed_slf.__richcmp__(arg, op).into();
497-
callback::convert(py, result)
491+
492+
slf.try_borrow()?.__richcmp__(arg, op).into()
498493
})
499494
}
500495
Some(wrap::<T>)

‎src/class/buffer.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,7 @@
55
//! For more information check [buffer protocol](https://docs.python.org/3/c-api/buffer.html)
66
//! c-api
77
use crate::err::PyResult;
8-
use crate::gil::GILPool;
9-
use crate::{callback, ffi, run_callback, PyCell, PyClass, PyRefMut};
8+
use crate::{ffi, PyCell, PyClass, PyRefMut};
109
use std::os::raw::c_int;
1110

1211
/// Buffer protocol interface
@@ -91,12 +90,9 @@ where
9190
where
9291
T: for<'p> PyBufferGetBufferProtocol<'p>,
9392
{
94-
let pool = GILPool::new();
95-
let py = pool.python();
96-
run_callback(py, || {
93+
crate::callback_body!(py, {
9794
let slf = py.from_borrowed_ptr::<PyCell<T>>(slf);
98-
let result = T::bf_getbuffer(slf.try_borrow_mut()?, arg1, arg2).into();
99-
callback::convert(py, result)
95+
T::bf_getbuffer(slf.try_borrow_mut()?, arg1, arg2).into()
10096
})
10197
}
10298
Some(wrap::<T>)
@@ -126,12 +122,9 @@ where
126122
where
127123
T: for<'p> PyBufferReleaseBufferProtocol<'p>,
128124
{
129-
let pool = GILPool::new();
130-
let py = pool.python();
131-
run_callback(py, || {
125+
crate::callback_body!(py, {
132126
let slf = py.from_borrowed_ptr::<crate::PyCell<T>>(slf);
133-
let result = T::bf_releasebuffer(slf.try_borrow_mut()?, arg1).into();
134-
crate::callback::convert(py, result)
127+
T::bf_releasebuffer(slf.try_borrow_mut()?, arg1).into()
135128
})
136129
}
137130
Some(wrap::<T>)

0 commit comments

Comments
 (0)