Merge pull request #2158 from davidhewitt/inline-handle-panic

refactor: inline handle_panic into macro output

CHANGELOG.md

@@ -48,7 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `__getitem__`, `__setitem__` and `__delitem__` in `#[pymethods]` now implement both a Python mapping and sequence by default. [#2065](https://github.com/PyO3/pyo3/pull/2065)
 - Improve performance and error messages for `#[derive(FromPyObject)]` for enums. [#2068](https://github.com/PyO3/pyo3/pull/2068)
 - Reduce generated LLVM code size (to improve compile times) for:
-  - internal `handle_panic` helper [#2074](https://github.com/PyO3/pyo3/pull/2074)
+  - internal `handle_panic` helper [#2074](https://github.com/PyO3/pyo3/pull/2074) [#2158](https://github.com/PyO3/pyo3/pull/2158)
   - `#[pyfunction]` and `#[pymethods]` argument extraction [#2075](https://github.com/PyO3/pyo3/pull/2075) [#2085](https://github.com/PyO3/pyo3/pull/2085)
   - `#[pyclass]` type object creation [#2076](https://github.com/PyO3/pyo3/pull/2076) [#2081](https://github.com/PyO3/pyo3/pull/2081) [#2157](https://github.com/PyO3/pyo3/pull/2157)
 - `__ipow__` now supports modulo argument on Python 3.8+. [#2083](https://github.com/PyO3/pyo3/pull/2083)

pyo3-macros-backend/src/method.rs

@@ -490,10 +490,12 @@ impl<'a> FnSpec<'a> {
                     {
                         use #krate as _pyo3;
                         #deprecations
-                        _pyo3::callback::handle_panic(|#py| {
+                        let gil = _pyo3::GILPool::new();
+                        let #py = gil.python();
+                        _pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
                             #self_conversion
                             #rust_call
-                        })
+                        }))
                     }
                 }
             }
@@ -508,11 +510,13 @@ impl<'a> FnSpec<'a> {
                     {
                         use #krate as _pyo3;
                         #deprecations
-                        _pyo3::callback::handle_panic(|#py| {
+                        let gil = _pyo3::GILPool::new();
+                        let #py = gil.python();
+                        _pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
                             #self_conversion
                             #arg_convert
                             #rust_call
-                        })
+                        }))
                     }
                 }
             }
@@ -526,11 +530,13 @@ impl<'a> FnSpec<'a> {
                     {
                         use #krate as _pyo3;
                         #deprecations
-                        _pyo3::callback::handle_panic(|#py| {
+                        let gil = _pyo3::GILPool::new();
+                        let #py = gil.python();
+                        _pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
                             #self_conversion
                             #arg_convert
                             #rust_call
-                        })
+                        }))
                     }
                 }
             }
@@ -546,13 +552,15 @@ impl<'a> FnSpec<'a> {
                         use #krate as _pyo3;
                         #deprecations
                         use _pyo3::callback::IntoPyCallbackOutput;
-                        _pyo3::callback::handle_panic(|#py| {
+                        let gil = _pyo3::GILPool::new();
+                        let #py = gil.python();
+                        _pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
                             #arg_convert
                             let result = #rust_call;
                             let initializer: _pyo3::PyClassInitializer::<#cls> = result.convert(#py)?;
                             let cell = initializer.create_cell_from_subtype(#py, subtype)?;
                             ::std::result::Result::Ok(cell as *mut _pyo3::ffi::PyObject)
-                        })
+                        }))
                     }
                 }
             }

pyo3-macros-backend/src/module.rs

@@ -80,8 +80,7 @@ pub fn pymodule_impl(
         /// This autogenerated function is called by the python interpreter when importing
         /// the module.
         pub unsafe extern "C" fn #cb_name() -> *mut #krate::ffi::PyObject {
-            use #krate::{self as _pyo3, IntoPyPointer};
-            _pyo3::callback::handle_panic(|_py| ::std::result::Result::Ok(#module_def_name.make_module(_py)?.into_ptr()))
+            unsafe { #module_def_name.module_init() }
         }
 
         #[doc(hidden)]

pyo3-macros-backend/src/pymethod.rs

@@ -304,7 +304,9 @@ pub fn impl_py_setter_def(cls: &syn::Type, property_type: PropertyType) -> Resul
                         _value: *mut _pyo3::ffi::PyObject,
                         _: *mut ::std::os::raw::c_void
                     ) -> ::std::os::raw::c_int {
-                        _pyo3::callback::handle_panic(|_py| {
+                        let gil = _pyo3::GILPool::new();
+                        let _py = gil.python();
+                        _pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
                             #slf
                             let _value = _py
                                 .from_borrowed_ptr_or_opt(_value)
@@ -314,7 +316,7 @@ pub fn impl_py_setter_def(cls: &syn::Type, property_type: PropertyType) -> Resul
                             let _val = _pyo3::FromPyObject::extract(_value)?;
 
                             _pyo3::callback::convert(_py, #setter_impl)
-                        })
+                        }))
                     }
                     __wrap
                 }),
@@ -383,10 +385,12 @@ pub fn impl_py_getter_def(cls: &syn::Type, property_type: PropertyType) -> Resul
                         _slf: *mut _pyo3::ffi::PyObject,
                         _: *mut ::std::os::raw::c_void
                     ) -> *mut _pyo3::ffi::PyObject {
-                        _pyo3::callback::handle_panic(|_py| {
+                        let gil = _pyo3::GILPool::new();
+                        let _py = gil.python();
+                        _pyo3::callback::panic_result_into_callback_output(_py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
                             #slf
                             _pyo3::callback::convert(_py, #getter_impl)
-                        })
+                        }))
                     }
                     __wrap
                 }),
@@ -880,9 +884,11 @@ impl SlotDef {
             unsafe extern "C" fn __wrap(_raw_slf: *mut _pyo3::ffi::PyObject, #(#method_arguments),*) -> #ret_ty {
                 let _slf = _raw_slf;
                 #before_call_method
-                _pyo3::callback::handle_panic(|#py| {
+                let gil = _pyo3::GILPool::new();
+                let #py = gil.python();
+                _pyo3::callback::panic_result_into_callback_output(#py, ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> {
                     #body
-                })
+                }))
             }
             _pyo3::ffi::PyType_Slot {
                 slot: _pyo3::ffi::#slot,

src/callback.rs

@@ -245,7 +245,11 @@ where
     panic_result_into_callback_output(py, panic::catch_unwind(move || -> PyResult<_> { body(py) }))
 }
 
-fn panic_result_into_callback_output<R>(
+/// Converts the output of std::panic::catch_unwind into a Python function output, either by raising a Python
+/// exception or by unwrapping the contained success output.
+#[doc(hidden)]
+#[inline]
+pub fn panic_result_into_callback_output<R>(
     py: Python,
     panic_result: Result<PyResult<R>, Box<dyn Any + Send + 'static>>,
 ) -> R

src/impl_/pyclass.rs

@@ -260,14 +260,19 @@ macro_rules! define_pyclass_setattr_slot {
                     use ::std::option::Option::*;
                     use $crate::callback::IntoPyCallbackOutput;
                     use $crate::impl_::pyclass::*;
-                    $crate::callback::handle_panic(|py| {
-                        let collector = PyClassImplCollector::<$cls>::new();
-                        if let Some(value) = ::std::ptr::NonNull::new(value) {
-                            collector.$set(py, _slf, attr, value).convert(py)
-                        } else {
-                            collector.$del(py, _slf, attr).convert(py)
-                        }
-                    })
+                    let gil = $crate::GILPool::new();
+                    let py = gil.python();
+                    $crate::callback::panic_result_into_callback_output(
+                        py,
+                        ::std::panic::catch_unwind(move || -> $crate::PyResult<_> {
+                            let collector = PyClassImplCollector::<$cls>::new();
+                            if let Some(value) = ::std::ptr::NonNull::new(value) {
+                                collector.$set(py, _slf, attr, value).convert(py)
+                            } else {
+                                collector.$del(py, _slf, attr).convert(py)
+                            }
+                        }),
+                    )
                 }
                 $crate::ffi::PyType_Slot {
                     slot: $crate::ffi::$slot,
@@ -367,17 +372,22 @@ macro_rules! define_pyclass_binary_operator_slot {
                     _slf: *mut $crate::ffi::PyObject,
                     _other: *mut $crate::ffi::PyObject,
                 ) -> *mut $crate::ffi::PyObject {
-                    $crate::callback::handle_panic(|py| {
-                        use $crate::impl_::pyclass::*;
-                        let collector = PyClassImplCollector::<$cls>::new();
-                        let lhs_result = collector.$lhs(py, _slf, _other)?;
-                        if lhs_result == $crate::ffi::Py_NotImplemented() {
-                            $crate::ffi::Py_DECREF(lhs_result);
-                            collector.$rhs(py, _other, _slf)
-                        } else {
-                            ::std::result::Result::Ok(lhs_result)
-                        }
-                    })
+                    let gil = $crate::GILPool::new();
+                    let py = gil.python();
+                    $crate::callback::panic_result_into_callback_output(
+                        py,
+                        ::std::panic::catch_unwind(move || -> $crate::PyResult<_> {
+                            use $crate::impl_::pyclass::*;
+                            let collector = PyClassImplCollector::<$cls>::new();
+                            let lhs_result = collector.$lhs(py, _slf, _other)?;
+                            if lhs_result == $crate::ffi::Py_NotImplemented() {
+                                $crate::ffi::Py_DECREF(lhs_result);
+                                collector.$rhs(py, _other, _slf)
+                            } else {
+                                ::std::result::Result::Ok(lhs_result)
+                            }
+                        }),
+                    )
                 }
                 $crate::ffi::PyType_Slot {
                     slot: $crate::ffi::$slot,
@@ -560,17 +570,22 @@ macro_rules! generate_pyclass_pow_slot {
             _other: *mut $crate::ffi::PyObject,
             _mod: *mut $crate::ffi::PyObject,
         ) -> *mut $crate::ffi::PyObject {
-            $crate::callback::handle_panic(|py| {
-                use $crate::impl_::pyclass::*;
-                let collector = PyClassImplCollector::<$cls>::new();
-                let lhs_result = collector.__pow__(py, _slf, _other, _mod)?;
-                if lhs_result == $crate::ffi::Py_NotImplemented() {
-                    $crate::ffi::Py_DECREF(lhs_result);
-                    collector.__rpow__(py, _other, _slf, _mod)
-                } else {
-                    ::std::result::Result::Ok(lhs_result)
-                }
-            })
+            let gil = $crate::GILPool::new();
+            let py = gil.python();
+            $crate::callback::panic_result_into_callback_output(
+                py,
+                ::std::panic::catch_unwind(move || -> $crate::PyResult<_> {
+                    use $crate::impl_::pyclass::*;
+                    let collector = PyClassImplCollector::<$cls>::new();
+                    let lhs_result = collector.__pow__(py, _slf, _other, _mod)?;
+                    if lhs_result == $crate::ffi::Py_NotImplemented() {
+                        $crate::ffi::Py_DECREF(lhs_result);
+                        collector.__rpow__(py, _other, _slf, _mod)
+                    } else {
+                        ::std::result::Result::Ok(lhs_result)
+                    }
+                }),
+            )
         }
         $crate::ffi::PyType_Slot {
             slot: $crate::ffi::Py_nb_power,

src/impl_/pymodule.rs

@@ -1,9 +1,10 @@
 //! Implementation details of `#[pymodule]` which need to be accessible from proc-macro generated code.
 
-use std::{cell::UnsafeCell, panic::AssertUnwindSafe};
+use std::cell::UnsafeCell;
 
 use crate::{
-    callback::handle_panic, ffi, types::PyModule, IntoPyPointer, Py, PyObject, PyResult, Python,
+    callback::panic_result_into_callback_output, ffi, types::PyModule, GILPool, IntoPyPointer, Py,
+    PyObject, PyResult, Python,
 };
 
 /// `Sync` wrapper of `ffi::PyModuleDef`.
@@ -64,8 +65,15 @@ impl ModuleDef {
     /// # Safety
     /// The Python GIL must be held.
     pub unsafe fn module_init(&'static self) -> *mut ffi::PyObject {
-        let unwind_safe_self = AssertUnwindSafe(self);
-        handle_panic(|py| Ok(unwind_safe_self.make_module(py)?.into_ptr()))
+        let pool = GILPool::new();
+        let py = pool.python();
+        let unwind_safe_self = std::panic::AssertUnwindSafe(self);
+        panic_result_into_callback_output(
+            py,
+            std::panic::catch_unwind(move || -> PyResult<_> {
+                Ok(unwind_safe_self.make_module(py)?.into_ptr())
+            }),
+        )
     }
 }
 

tests/ui/static_ref.stderr

@@ -1,25 +1,11 @@
-error[E0495]: cannot infer an appropriate lifetime for lifetime parameter `'py` due to conflicting requirements
+error[E0597]: `gil` does not live long enough
  --> tests/ui/static_ref.rs:4:1
   |
 4 | #[pyfunction]
-  | ^^^^^^^^^^^^^
+  | ^^^^^^^^^^^^-
+  | |           |
+  | |           `gil` dropped here while still borrowed
+  | borrowed value does not live long enough
+  | cast requires that `gil` is borrowed for `'static`
   |
-note: first, the lifetime cannot outlive the anonymous lifetime #1 defined here...
- --> tests/ui/static_ref.rs:4:1
-  |
-4 | #[pyfunction]
-  | ^^^^^^^^^^^^^
-note: ...so that the expression is assignable
- --> tests/ui/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
- --> tests/ui/static_ref.rs:4:1
-  |
-4 | #[pyfunction]
-  | ^^^^^^^^^^^^^
   = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)