From f97c3a947975b37848214d9918caf1f9c91ac8b8 Mon Sep 17 00:00:00 2001 From: konstin Date: Mon, 1 Mar 2021 12:42:19 +0100 Subject: [PATCH] Convert callback_body_without_convert function to handle_panic function --- pyo3-macros-backend/src/module.rs | 8 ++--- pyo3-macros-backend/src/pymethod.rs | 32 ++++++++++---------- src/callback.rs | 45 +++++++++++++++-------------- tests/ui/static_ref.stderr | 26 +++++++++++++---- 4 files changed, 63 insertions(+), 48 deletions(-) diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index d0fac0f1..64bfe42c 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -25,7 +25,7 @@ pub fn py_init(fnname: &Ident, name: &Ident, doc: syn::LitStr) -> TokenStream { const NAME: &'static str = concat!(stringify!(#name), "\0"); static MODULE_DEF: ModuleDef = unsafe { ModuleDef::new(NAME) }; - pyo3::callback_body!(_py, { MODULE_DEF.make_module(#doc, #fnname) }) + pyo3::callback::handle_panic(|_py| { MODULE_DEF.make_module(#doc, #fnname) }) } } } @@ -229,14 +229,14 @@ fn function_c_wrapper( let slf_module; if pass_module { cb = quote! { - #name(_slf, #(#names),*) + pyo3::callback::convert(_py, #name(_slf, #(#names),*)) }; slf_module = Some(quote! { let _slf = _py.from_borrowed_ptr::(_slf); }); } else { cb = quote! { - #name(#(#names),*) + pyo3::callback::convert(_py, #name(#(#names),*)) }; slf_module = None; }; @@ -248,7 +248,7 @@ fn function_c_wrapper( _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject { const _LOCATION: &'static str = concat!(stringify!(#name), "()"); - pyo3::callback_body!(_py, { + pyo3::callback::handle_panic(|_py| { #slf_module let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index c5fee659..238c677d 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -121,9 +121,9 @@ fn impl_wrap_common( { const _LOCATION: &'static str = concat!( stringify!(#cls), ".", stringify!(#python_name), "()"); - pyo3::callback_body_without_convert!(_py, { + pyo3::callback::handle_panic(|_py| { #slf - pyo3::callback::convert(_py, #body) + #body }) } } @@ -138,12 +138,12 @@ fn impl_wrap_common( { const _LOCATION: &'static str = concat!( stringify!(#cls), ".", stringify!(#python_name), "()"); - pyo3::callback_body_without_convert!(_py, { + pyo3::callback::handle_panic(|_py| { #slf let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); - pyo3::callback::convert(_py, #body) + #body }) } } @@ -165,12 +165,12 @@ pub fn impl_proto_wrap(cls: &syn::Type, spec: &FnSpec<'_>, self_ty: &SelfType) - _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject { const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()"); - pyo3::callback_body_without_convert!(_py, { + pyo3::callback::handle_panic(|_py| { #slf let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); - pyo3::callback::convert(_py, #body) + #body }) } } @@ -196,7 +196,7 @@ pub fn impl_wrap_new(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { use std::convert::TryFrom; const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()"); - pyo3::callback_body_without_convert!(_py, { + pyo3::callback::handle_panic(|_py| { let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); @@ -213,7 +213,7 @@ pub fn impl_wrap_class(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { let name = &spec.name; let python_name = &spec.python_name; let names: Vec = get_arg_names(&spec); - let cb = quote! { #cls::#name(&_cls, #(#names),*) }; + let cb = quote! { pyo3::callback::convert(_py, #cls::#name(&_cls, #(#names),*)) }; let body = impl_arg_params(spec, Some(cls), cb); @@ -225,12 +225,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),"()"); - pyo3::callback_body_without_convert!(_py, { + pyo3::callback::handle_panic(|_py| { let _cls = pyo3::types::PyType::from_type_ptr(_py, _cls as *mut pyo3::ffi::PyTypeObject); let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); - pyo3::callback::convert(_py, #body) + #body }) } } @@ -241,7 +241,7 @@ pub fn impl_wrap_static(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { let name = &spec.name; let python_name = &spec.python_name; let names: Vec = get_arg_names(&spec); - let cb = quote! { #cls::#name(#(#names),*) }; + let cb = quote! { pyo3::callback::convert(_py, #cls::#name(#(#names),*)) }; let body = impl_arg_params(spec, Some(cls), cb); @@ -253,11 +253,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),"()"); - pyo3::callback_body_without_convert!(_py, { + pyo3::callback::handle_panic(|_py| { let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); - pyo3::callback::convert(_py, #body) + #body }) } } @@ -319,7 +319,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),"()"); - pyo3::callback_body_without_convert!(_py, { + pyo3::callback::handle_panic(|_py| { #slf pyo3::callback::convert(_py, #getter_impl) }) @@ -371,7 +371,7 @@ pub(crate) fn impl_wrap_setter( _value: *mut pyo3::ffi::PyObject, _: *mut std::os::raw::c_void) -> std::os::raw::c_int { const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#python_name),"()"); - pyo3::callback_body_without_convert!(_py, { + pyo3::callback::handle_panic(|_py| { #slf let _value = _py.from_borrowed_ptr::(_value); let _val = pyo3::FromPyObject::extract(_value)?; @@ -392,7 +392,7 @@ pub fn get_arg_names(spec: &FnSpec) -> Vec { fn impl_call(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream { let fname = &spec.name; let names = get_arg_names(spec); - quote! { #cls::#fname(_slf, #(#names),*) } + quote! { pyo3::callback::convert(_py, #cls::#fname(_slf, #(#names),*)) } } pub fn impl_arg_params( diff --git a/src/callback.rs b/src/callback.rs index 59da2662..309e449d 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -9,6 +9,7 @@ use crate::IntoPyPointer; use crate::{IntoPy, PyObject, Python}; use std::isize; use std::os::raw::c_int; +use std::panic::UnwindSafe; /// A type which can be the return type of a python C-API callback pub trait PyCallbackOutput: Copy { @@ -194,9 +195,9 @@ where #[doc(hidden)] #[macro_export] macro_rules! callback_body { - ($py:ident, $body:expr) => {{ - $crate::callback_body_without_convert!($py, $crate::callback::convert($py, $body)) - }}; + ($py:ident, $body:expr) => { + $crate::callback::handle_panic(|$py| $crate::callback::convert($py, $body)) + }; } /// Variant of the above which does not perform the callback conversion. This allows the callback @@ -210,10 +211,10 @@ macro_rules! callback_body { /// } /// ``` /// -/// It is wrapped in proc macros with callback_body_without_convert like so: +/// It is wrapped in proc macros with handle_panic like so: /// /// ```ignore -/// pyo3::callback_body_without_convert!(py, { +/// pyo3::callback::handle_panic(|_py| { /// let _slf = #slf; /// pyo3::callback::convert(py, #foo) /// }) @@ -231,33 +232,33 @@ macro_rules! callback_body { /// 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 unwind_safe_py = std::panic::AssertUnwindSafe(pool.python()); - let result = match std::panic::catch_unwind(move || -> $crate::PyResult<_> { - let $py = *unwind_safe_py; - $body - }) { +pub unsafe fn handle_panic< + R: PyCallbackOutput, + F: FnOnce(Python) -> crate::PyResult + UnwindSafe, +>( + body: F, +) -> R { + let pool = crate::GILPool::new(); + let unwind_safe_py = std::panic::AssertUnwindSafe(pool.python()); + let result = + match std::panic::catch_unwind(move || -> crate::PyResult<_> { body(*unwind_safe_py) }) { Ok(result) => result, Err(e) => { // Try to format the error in the same way panic does if let Some(string) = e.downcast_ref::() { - Err($crate::panic::PanicException::new_err((string.clone(),))) + Err(crate::panic::PanicException::new_err((string.clone(),))) } else if let Some(s) = e.downcast_ref::<&str>() { - Err($crate::panic::PanicException::new_err((s.to_string(),))) + Err(crate::panic::PanicException::new_err((s.to_string(),))) } else { - Err($crate::panic::PanicException::new_err(( + Err(crate::panic::PanicException::new_err(( "panic from Rust code", ))) } } }; - result.unwrap_or_else(|e| { - e.restore(pool.python()); - $crate::callback::callback_error() - }) - }}; + result.unwrap_or_else(|e| { + e.restore(pool.python()); + crate::callback::callback_error() + }) } diff --git a/tests/ui/static_ref.stderr b/tests/ui/static_ref.stderr index 1e093a35..b4f452e2 100644 --- a/tests/ui/static_ref.stderr +++ b/tests/ui/static_ref.stderr @@ -1,11 +1,25 @@ -error[E0597]: `pool` does not live long enough +error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'p` due to conflicting requirements --> $DIR/static_ref.rs:4:1 | 4 | #[pyfunction] | ^^^^^^^^^^^^^ - | | - | borrowed value does not live long enough - | `pool` dropped here while still borrowed - | cast requires that `pool` is borrowed for `'static` | - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) +note: first, the lifetime cannot outlive the anonymous lifetime #1 defined on the body at 4:1... + --> $DIR/static_ref.rs:4:1 + | +4 | #[pyfunction] + | ^^^^^^^^^^^^^ +note: ...so that the types are compatible + --> $DIR/static_ref.rs:4:1 + | +4 | #[pyfunction] + | ^^^^^^^^^^^^^ + = note: expected `pyo3::Python<'_>` + found `pyo3::Python<'_>` + = note: but, the lifetime must be valid for the static lifetime... +note: ...so that reference does not outlive borrowed content + --> $DIR/static_ref.rs:4:1 + | +4 | #[pyfunction] + | ^^^^^^^^^^^^^ + = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)