Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: inline handle_panic into macro output #2158

Merged
merged 1 commit into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
24 changes: 16 additions & 8 deletions pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}))
}
}
}
Expand All @@ -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
})
}))
}
}
}
Expand All @@ -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
})
}))
}
}
}
Expand All @@ -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)
})
}))
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions pyo3-macros-backend/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
18 changes: 12 additions & 6 deletions pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}),
Expand Down Expand Up @@ -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
}),
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion src/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
75 changes: 45 additions & 30 deletions src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
16 changes: 12 additions & 4 deletions src/impl_/pymodule.rs
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down Expand Up @@ -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())
}),
)
}
}

Expand Down
26 changes: 6 additions & 20 deletions tests/ui/static_ref.stderr
Original file line number Diff line number Diff line change
@@ -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)