Merge pull request #1455 from PyO3/generate-less-code

Generate less code
PyO3 · Mar 1, 2021 · 67b1a61 · 67b1a61
2 parents f9ad119 + 19bcce5
commit 67b1a61
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 55 deletions.
diff --git 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::<pyo3::types::PyModule>(_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::<pyo3::types::PyTuple>(_args);
                 let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);

diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs
@@ -428,8 +428,8 @@ fn impl_class(
 
             fn for_each_method_def(visitor: impl FnMut(&pyo3::class::PyMethodDefType)) {
                 use pyo3::class::impl_::*;
-                let collector = PyClassImplCollector::<#cls>::new();
-                pyo3::inventory::iter::<<#cls as pyo3::class::methods::HasMethodsInventory>::Methods>
+                let collector = PyClassImplCollector::<Self>::new();
+                pyo3::inventory::iter::<<Self as pyo3::class::methods::HasMethodsInventory>::Methods>
                     .into_iter()
                     .flat_map(pyo3::class::methods::PyMethodsInventory::get)
                     .chain(collector.object_protocol_methods())
@@ -442,19 +442,19 @@ fn impl_class(
             }
             fn get_new() -> Option<pyo3::ffi::newfunc> {
                 use pyo3::class::impl_::*;
-                let collector = PyClassImplCollector::<#cls>::new();
+                let collector = PyClassImplCollector::<Self>::new();
                 collector.new_impl()
             }
             fn get_call() -> Option<pyo3::ffi::PyCFunctionWithKeywords> {
                 use pyo3::class::impl_::*;
-                let collector = PyClassImplCollector::<#cls>::new();
+                let collector = PyClassImplCollector::<Self>::new();
                 collector.call_impl()
             }
 
             fn for_each_proto_slot(visitor: impl FnMut(&pyo3::ffi::PyType_Slot)) {
                 // Implementation which uses dtolnay specialization to load all slots.
                 use pyo3::class::impl_::*;
-                let collector = PyClassImplCollector::<#cls>::new();
+                let collector = PyClassImplCollector::<Self>::new();
                 collector.object_protocol_slots()
                     .iter()
                     .chain(collector.number_protocol_slots())
@@ -470,7 +470,7 @@ fn impl_class(
 
             fn get_buffer() -> Option<&'static pyo3::class::impl_::PyBufferProcs> {
                 use pyo3::class::impl_::*;
-                let collector = PyClassImplCollector::<#cls>::new();
+                let collector = PyClassImplCollector::<Self>::new();
                 collector.buffer_procs()
             }
         }

diff --git 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::<pyo3::types::PyTuple>(_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::<pyo3::types::PyTuple>(_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::<pyo3::types::PyTuple>(_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<syn::Ident> = 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::<pyo3::types::PyTuple>(_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<syn::Ident> = 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::<pyo3::types::PyTuple>(_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::<pyo3::types::PyAny>(_value);
                 let _val = pyo3::FromPyObject::extract(_value)?;
@@ -392,7 +392,7 @@ pub fn get_arg_names(spec: &FnSpec) -> Vec<syn::Ident> {
 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
@@ -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,12 +211,12 @@ 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)
+///     pyo3::callback::convert(_py, #foo)
 /// })
 /// ```
 ///
@@ -231,33 +232,32 @@ 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<F, R>(body: F) -> R
+where
+    F: FnOnce(Python) -> crate::PyResult<R> + UnwindSafe,
+    R: PyCallbackOutput,
+{
+    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::<String>() {
-                    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
@@ -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)