From 747d791f1f614f0f18eb856b2a0ddb600a63d949 Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Tue, 25 Oct 2022 07:22:36 +0100 Subject: [PATCH] introduce trampolines for methods (#2705) --- newsfragments/2705.changed.md | 1 + pyo3-macros-backend/src/method.rs | 122 +++++++++++------ pyo3-macros-backend/src/module.rs | 2 +- pyo3-macros-backend/src/pymethod.rs | 141 +++++++++++++------ src/callback.rs | 95 +------------ src/impl_.rs | 2 + src/impl_/pyclass.rs | 123 ++++++++--------- src/impl_/pymodule.rs | 131 ++++++++---------- src/impl_/trampoline.rs | 203 ++++++++++++++++++++++++++++ src/pycell.rs | 23 ++-- src/pyclass.rs | 4 +- src/types/function.rs | 22 +-- tests/ui/static_ref.stderr | 11 +- 13 files changed, 532 insertions(+), 348 deletions(-) create mode 100644 newsfragments/2705.changed.md create mode 100644 src/impl_/trampoline.rs diff --git a/newsfragments/2705.changed.md b/newsfragments/2705.changed.md new file mode 100644 index 00000000..8365fc01 --- /dev/null +++ b/newsfragments/2705.changed.md @@ -0,0 +1 @@ +Change `#[pyfunction]` and `#[pymethods]` to use a common call "trampoline" to slightly reduce generated code size and compile times. diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs index c120b202..3165c5c9 100644 --- a/pyo3-macros-backend/src/method.rs +++ b/pyo3-macros-backend/src/method.rs @@ -481,18 +481,13 @@ impl<'a> FnSpec<'a> { CallingConvention::Noargs => { let call = rust_call(vec![]); quote! { - unsafe extern "C" fn #ident ( + unsafe fn #ident<'py>( + #py: _pyo3::Python<'py>, _slf: *mut _pyo3::ffi::PyObject, - _args: *mut _pyo3::ffi::PyObject, - ) -> *mut _pyo3::ffi::PyObject - { + ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { #deprecations - 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 - #call - })) + #self_conversion + #call } } } @@ -500,20 +495,17 @@ impl<'a> FnSpec<'a> { let (arg_convert, args) = impl_arg_params(self, cls, &py, true)?; let call = rust_call(args); quote! { - unsafe extern "C" fn #ident ( + unsafe fn #ident<'py>( + #py: _pyo3::Python<'py>, _slf: *mut _pyo3::ffi::PyObject, _args: *const *mut _pyo3::ffi::PyObject, _nargs: _pyo3::ffi::Py_ssize_t, - _kwnames: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject - { + _kwnames: *mut _pyo3::ffi::PyObject + ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { #deprecations - 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 - #call - })) + #self_conversion + #arg_convert + #call } } } @@ -521,19 +513,16 @@ impl<'a> FnSpec<'a> { let (arg_convert, args) = impl_arg_params(self, cls, &py, false)?; let call = rust_call(args); quote! { - unsafe extern "C" fn #ident ( + unsafe fn #ident<'py>( + #py: _pyo3::Python<'py>, _slf: *mut _pyo3::ffi::PyObject, _args: *mut _pyo3::ffi::PyObject, - _kwargs: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject - { + _kwargs: *mut _pyo3::ffi::PyObject + ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { #deprecations - 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 - #call - })) + #self_conversion + #arg_convert + #call } } } @@ -541,21 +530,19 @@ impl<'a> FnSpec<'a> { let (arg_convert, args) = impl_arg_params(self, cls, &py, false)?; let call = quote! { #rust_name(#(#args),*) }; quote! { - unsafe extern "C" fn #ident ( + unsafe fn #ident( + #py: _pyo3::Python<'_>, subtype: *mut _pyo3::ffi::PyTypeObject, _args: *mut _pyo3::ffi::PyObject, - _kwargs: *mut _pyo3::ffi::PyObject) -> *mut _pyo3::ffi::PyObject - { + _kwargs: *mut _pyo3::ffi::PyObject + ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { + use _pyo3::callback::IntoPyCallbackOutput; #deprecations - use _pyo3::{callback::IntoPyCallbackOutput, pyclass_init::PyObjectInit}; - 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 = #call; - let initializer: _pyo3::PyClassInitializer::<#cls> = result.convert(#py)?; - initializer.into_new_object(#py, subtype) - })) + #arg_convert + let result = #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) } } } @@ -571,21 +558,66 @@ impl<'a> FnSpec<'a> { CallingConvention::Noargs => quote! { _pyo3::impl_::pymethods::PyMethodDef::noargs( #python_name, - _pyo3::impl_::pymethods::PyCFunction(#wrapper), + _pyo3::impl_::pymethods::PyCFunction({ + unsafe extern "C" fn trampoline( + _slf: *mut _pyo3::ffi::PyObject, + _args: *mut _pyo3::ffi::PyObject, + ) -> *mut _pyo3::ffi::PyObject + { + _pyo3::impl_::trampoline::noargs( + _slf, + _args, + #wrapper + ) + } + trampoline + }), #doc, ) }, CallingConvention::Fastcall => quote! { _pyo3::impl_::pymethods::PyMethodDef::fastcall_cfunction_with_keywords( #python_name, - _pyo3::impl_::pymethods::PyCFunctionFastWithKeywords(#wrapper), + _pyo3::impl_::pymethods::PyCFunctionFastWithKeywords({ + unsafe extern "C" fn trampoline( + _slf: *mut _pyo3::ffi::PyObject, + _args: *const *mut _pyo3::ffi::PyObject, + _nargs: _pyo3::ffi::Py_ssize_t, + _kwnames: *mut _pyo3::ffi::PyObject + ) -> *mut _pyo3::ffi::PyObject + { + _pyo3::impl_::trampoline::fastcall_with_keywords( + _slf, + _args, + _nargs, + _kwnames, + #wrapper + ) + } + trampoline + }), #doc, ) }, CallingConvention::Varargs => quote! { _pyo3::impl_::pymethods::PyMethodDef::cfunction_with_keywords( #python_name, - _pyo3::impl_::pymethods::PyCFunctionWithKeywords(#wrapper), + _pyo3::impl_::pymethods::PyCFunctionWithKeywords({ + unsafe extern "C" fn trampoline( + _slf: *mut _pyo3::ffi::PyObject, + _args: *mut _pyo3::ffi::PyObject, + _kwargs: *mut _pyo3::ffi::PyObject, + ) -> *mut _pyo3::ffi::PyObject + { + _pyo3::impl_::trampoline::cfunction_with_keywords( + _slf, + _args, + _kwargs, + #wrapper + ) + } + trampoline + }), #doc, ) }, diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 65027dd3..236aee8e 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -84,7 +84,7 @@ pub fn pymodule_impl( /// the module. #[export_name = #pyinit_symbol] pub unsafe extern "C" fn init() -> *mut #krate::ffi::PyObject { - DEF.module_init() + #krate::impl_::trampoline::module_init(|py| DEF.make_module(py)) } } diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index ba9b66c2..0e5b8f49 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -302,7 +302,22 @@ fn impl_py_method_def_new(cls: &syn::Type, spec: &FnSpec<'_>) -> Result *mut _pyo3::ffi::PyObject + { + _pyo3::impl_::trampoline::newfunc( + subtype, + args, + kwargs, + #cls::#wrapper_ident + ) + } + trampoline + } as _pyo3::ffi::newfunc as _ } }; Ok(MethodAndSlotDef { @@ -321,7 +336,22 @@ fn impl_call_slot(cls: &syn::Type, mut spec: FnSpec<'_>) -> Result *mut _pyo3::ffi::PyObject + { + _pyo3::impl_::trampoline::ternaryfunc( + slf, + args, + kwargs, + #cls::#wrapper_ident + ) + } + trampoline + } as _pyo3::ffi::ternaryfunc as _ } }; Ok(MethodAndSlotDef { @@ -484,24 +514,20 @@ pub fn impl_py_setter_def( }; let associated_method = quote! { - unsafe extern "C" fn #wrapper_ident( + unsafe fn #wrapper_ident( + _py: _pyo3::Python<'_>, _slf: *mut _pyo3::ffi::PyObject, _value: *mut _pyo3::ffi::PyObject, - _: *mut ::std::os::raw::c_void - ) -> ::std::os::raw::c_int { - 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) - .ok_or_else(|| { - _pyo3::exceptions::PyAttributeError::new_err("can't delete attribute") - })?; - let _val = _pyo3::FromPyObject::extract(_value)?; + ) -> _pyo3::PyResult<::std::os::raw::c_int> { + #slf + let _value = _py + .from_borrowed_ptr_or_opt(_value) + .ok_or_else(|| { + _pyo3::exceptions::PyAttributeError::new_err("can't delete attribute") + })?; + let _val = _pyo3::FromPyObject::extract(_value)?; - _pyo3::callback::convert(_py, #setter_impl) - })) + _pyo3::callback::convert(_py, #setter_impl) } }; @@ -510,7 +536,22 @@ pub fn impl_py_setter_def( #deprecations _pyo3::class::PySetterDef::new( #python_name, - _pyo3::impl_::pymethods::PySetter(#cls::#wrapper_ident), + _pyo3::impl_::pymethods::PySetter({ + unsafe extern "C" fn trampoline( + slf: *mut _pyo3::ffi::PyObject, + value: *mut _pyo3::ffi::PyObject, + closure: *mut ::std::os::raw::c_void, + ) -> ::std::os::raw::c_int + { + _pyo3::impl_::trampoline::setter( + slf, + value, + closure, + #cls::#wrapper_ident + ) + } + trampoline + }), #doc ) }) @@ -555,7 +596,6 @@ pub fn impl_py_getter_def( .. } => { // named struct field - //quote!(_slf.#ident.clone()) quote!(::std::clone::Clone::clone(&(_slf.#ident))) } PropertyType::Descriptor { field_index, .. } => { @@ -608,17 +648,13 @@ pub fn impl_py_getter_def( }; let associated_method = quote! { - unsafe extern "C" fn #wrapper_ident( - _slf: *mut _pyo3::ffi::PyObject, - _: *mut ::std::os::raw::c_void - ) -> *mut _pyo3::ffi::PyObject { - 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 item = #getter_impl; - #conversion - })) + unsafe fn #wrapper_ident( + _py: _pyo3::Python<'_>, + _slf: *mut _pyo3::ffi::PyObject + ) -> _pyo3::PyResult<*mut _pyo3::ffi::PyObject> { + #slf + let item = #getter_impl; + #conversion } }; @@ -627,7 +663,20 @@ pub fn impl_py_getter_def( #deprecations _pyo3::class::PyGetterDef::new( #python_name, - _pyo3::impl_::pymethods::PyGetter(#cls::#wrapper_ident), + _pyo3::impl_::pymethods::PyGetter({ + unsafe extern "C" fn trampoline( + slf: *mut _pyo3::ffi::PyObject, + closure: *mut ::std::os::raw::c_void, + ) -> *mut _pyo3::ffi::PyObject + { + _pyo3::impl_::trampoline::getter( + slf, + closure, + #cls::#wrapper_ident + ) + } + trampoline + }), #doc ) }) @@ -1050,21 +1099,33 @@ impl SlotDef { return_mode.as_ref(), )?; let associated_method = quote! { - unsafe extern "C" fn #wrapper_ident(_raw_slf: *mut _pyo3::ffi::PyObject, #(#arg_idents: #arg_types),*) -> #ret_ty { + unsafe fn #wrapper_ident( + #py: _pyo3::Python<'_>, + _raw_slf: *mut _pyo3::ffi::PyObject, + #(#arg_idents: #arg_types),* + ) -> _pyo3::PyResult<#ret_ty> { let _slf = _raw_slf; - 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 - })) + #body } }; - let slot_def = quote! { + let slot_def = quote! {{ + unsafe extern "C" fn trampoline( + _slf: *mut _pyo3::ffi::PyObject, + #(#arg_idents: #arg_types),* + ) -> #ret_ty + { + _pyo3::impl_::trampoline:: #func_ty ( + _slf, + #(#arg_idents,)* + #cls::#wrapper_ident + ) + } + _pyo3::ffi::PyType_Slot { slot: _pyo3::ffi::#slot, - pfunc: #cls::#wrapper_ident as _pyo3::ffi::#func_ty as _ + pfunc: trampoline as _pyo3::ffi::#func_ty as _ } - }; + }}; Ok(MethodAndSlotDef { associated_method, slot_def, diff --git a/src/callback.rs b/src/callback.rs index 457c9a53..588639bb 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -5,14 +5,10 @@ use crate::err::{PyErr, PyResult}; use crate::exceptions::PyOverflowError; use crate::ffi::{self, Py_hash_t}; -use crate::impl_::panic::PanicTrap; -use crate::panic::PanicException; -use crate::{GILPool, IntoPyPointer}; +use crate::IntoPyPointer; use crate::{IntoPy, PyObject, Python}; -use std::any::Any; +use std::isize; use std::os::raw::c_int; -use std::panic::UnwindSafe; -use std::{isize, panic}; /// A type which can be the return type of a python C-API callback pub trait PyCallbackOutput: Copy { @@ -187,90 +183,3 @@ where { value.convert(py) } - -/// Use this macro for all internal callback functions which Python will call. -/// -/// It sets up the GILPool and converts the output into a Python object. It also restores -/// any python error returned as an Err variant from the body. -/// -/// Finally, any panics inside the callback body will be caught and translated into PanicExceptions. -/// -/// # Safety -/// This macro assumes the GIL is held. (It makes use of unsafe code, so usage of it is only -/// possible inside unsafe blocks.) -#[doc(hidden)] -#[macro_export] -macro_rules! callback_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 -/// conversion to be done manually in the case where lifetimes might otherwise cause issue. -/// -/// For example this pyfunction: -/// -/// ```no_compile -/// fn foo(&self) -> &Bar { -/// &self.bar -/// } -/// ``` -/// -/// It is wrapped in proc macros with handle_panic like so: -/// -/// ```no_compile -/// pyo3::callback::handle_panic(|_py| { -/// let _slf = #slf; -/// pyo3::callback::convert(_py, #foo) -/// }) -/// ``` -/// -/// If callback_body was used instead: -/// -/// ```no_compile -/// pyo3::callback_body!(py, { -/// let _slf = #slf; -/// #foo -/// }) -/// ``` -/// -/// 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)] -#[inline] -pub unsafe fn handle_panic(body: F) -> R -where - F: for<'py> FnOnce(Python<'py>) -> PyResult + UnwindSafe, - R: PyCallbackOutput, -{ - let trap = PanicTrap::new("uncaught panic at ffi boundary"); - let pool = GILPool::new(); - let py = pool.python(); - let out = panic_result_into_callback_output( - py, - panic::catch_unwind(move || -> PyResult<_> { body(py) }), - ); - trap.disarm(); - out -} - -/// 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( - py: Python<'_>, - panic_result: Result, Box>, -) -> R -where - R: PyCallbackOutput, -{ - let py_err = match panic_result { - Ok(Ok(value)) => return value, - Ok(Err(py_err)) => py_err, - Err(payload) => PanicException::from_panic_payload(payload), - }; - py_err.restore(py); - R::ERR_VALUE -} diff --git a/src/impl_.rs b/src/impl_.rs index 6b4db2b8..e0c06c5a 100644 --- a/src/impl_.rs +++ b/src/impl_.rs @@ -17,3 +17,5 @@ pub mod pyclass; pub mod pyfunction; pub mod pymethods; pub mod pymodule; +#[doc(hidden)] +pub mod trampoline; diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs index d0724a04..b7d40011 100644 --- a/src/impl_/pyclass.rs +++ b/src/impl_/pyclass.rs @@ -313,29 +313,24 @@ macro_rules! generate_pyclass_getattro_slot { _slf: *mut $crate::ffi::PyObject, attr: *mut $crate::ffi::PyObject, ) -> *mut $crate::ffi::PyObject { - use ::std::result::Result::*; - use $crate::impl_::pyclass::*; - 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(); + $crate::impl_::trampoline::getattrofunc(_slf, attr, |py, _slf, attr| { + use ::std::result::Result::*; + use $crate::impl_::pyclass::*; + let collector = PyClassImplCollector::<$cls>::new(); - // Strategy: - // - Try __getattribute__ first. Its default is PyObject_GenericGetAttr. - // - If it returns a result, use it. - // - If it fails with AttributeError, try __getattr__. - // - If it fails otherwise, reraise. - match collector.__getattribute__(py, _slf, attr) { - Ok(obj) => Ok(obj), - Err(e) if e.is_instance_of::<$crate::exceptions::PyAttributeError>(py) => { - collector.__getattr__(py, _slf, attr) - } - Err(e) => Err(e), + // Strategy: + // - Try __getattribute__ first. Its default is PyObject_GenericGetAttr. + // - If it returns a result, use it. + // - If it fails with AttributeError, try __getattr__. + // - If it fails otherwise, reraise. + match collector.__getattribute__(py, _slf, attr) { + Ok(obj) => Ok(obj), + Err(e) if e.is_instance_of::<$crate::exceptions::PyAttributeError>(py) => { + collector.__getattr__(py, _slf, attr) } - }), - ) + Err(e) => Err(e), + } + }) } $crate::ffi::PyType_Slot { slot: $crate::ffi::Py_tp_getattro, @@ -402,21 +397,21 @@ macro_rules! define_pyclass_setattr_slot { attr: *mut $crate::ffi::PyObject, value: *mut $crate::ffi::PyObject, ) -> ::std::os::raw::c_int { - use ::std::option::Option::*; - use $crate::callback::IntoPyCallbackOutput; - use $crate::impl_::pyclass::*; - let gil = $crate::GILPool::new(); - let py = gil.python(); - $crate::callback::panic_result_into_callback_output( - py, - ::std::panic::catch_unwind(move || -> $crate::PyResult<_> { + $crate::impl_::trampoline::setattrofunc( + _slf, + attr, + value, + |py, _slf, attr, value| { + use ::std::option::Option::*; + use $crate::callback::IntoPyCallbackOutput; + use $crate::impl_::pyclass::*; 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 { @@ -517,22 +512,17 @@ macro_rules! define_pyclass_binary_operator_slot { _slf: *mut $crate::ffi::PyObject, _other: *mut $crate::ffi::PyObject, ) -> *mut $crate::ffi::PyObject { - 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::impl_::trampoline::binaryfunc(_slf, _other, |py, _slf, _other| { + 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, @@ -715,22 +705,17 @@ macro_rules! generate_pyclass_pow_slot { _other: *mut $crate::ffi::PyObject, _mod: *mut $crate::ffi::PyObject, ) -> *mut $crate::ffi::PyObject { - 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::impl_::trampoline::ternaryfunc(_slf, _other, _mod, |py, _slf, _other, _mod| { + 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, @@ -943,7 +928,19 @@ impl PyClassBaseType for T { /// Implementation of tp_dealloc for all pyclasses pub(crate) unsafe extern "C" fn tp_dealloc(obj: *mut ffi::PyObject) { - crate::callback_body!(py, T::Layout::tp_dealloc(obj, py)) + /// A wrapper because PyCellLayout::tp_dealloc currently takes the py argument last + /// (which is different to the rest of the trampolines which take py first) + #[inline] + #[allow(clippy::unnecessary_wraps)] + unsafe fn trampoline_dealloc_wrapper( + py: Python<'_>, + slf: *mut ffi::PyObject, + ) -> PyResult<()> { + T::Layout::tp_dealloc(slf, py); + Ok(()) + } + // TODO change argument order in PyCellLayout::tp_dealloc so this wrapper isn't needed. + crate::impl_::trampoline::dealloc(obj, trampoline_dealloc_wrapper::) } pub(crate) unsafe extern "C" fn get_sequence_item_from_mapping( diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index 28996b02..20a076a5 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -5,10 +5,7 @@ use std::{ sync::atomic::{self, AtomicBool}, }; -use crate::{ - callback::panic_result_into_callback_output, exceptions::PyImportError, ffi, types::PyModule, - GILPool, IntoPyPointer, Py, PyObject, PyResult, Python, -}; +use crate::{exceptions::PyImportError, ffi, types::PyModule, Py, PyResult, Python}; /// `Sync` wrapper of `ffi::PyModuleDef`. pub struct ModuleDef { @@ -58,7 +55,22 @@ impl ModuleDef { } } /// Builds a module using user given initializer. Used for [`#[pymodule]`][crate::pymodule]. - pub fn make_module(&'static self, py: Python<'_>) -> PyResult { + pub fn make_module(&'static self, py: Python<'_>) -> PyResult> { + #[cfg(all(PyPy, not(Py_3_8)))] + { + const PYPY_GOOD_VERSION: [u8; 3] = [7, 3, 8]; + let version = py + .import("sys")? + .getattr("implementation")? + .getattr("version")?; + if version.lt(crate::types::PyTuple::new(py, &PYPY_GOOD_VERSION))? { + let warn = py.import("warnings")?.getattr("warn")?; + warn.call1(( + "PyPy 3.7 versions older than 7.3.8 are known to have binary \ + compatibility issues which may cause segfaults. Please upgrade.", + ))?; + } + } let module = unsafe { Py::::from_owned_ptr_or_err(py, ffi::PyModule_Create(self.ffi_def.get()))? }; @@ -68,37 +80,7 @@ impl ModuleDef { )); } (self.initializer.0)(py, module.as_ref(py))?; - Ok(module.into()) - } - /// Implementation of `PyInit_foo` functions generated in [`#[pymodule]`][crate::pymodule].. - /// - /// # Safety - /// The Python GIL must be held. - pub unsafe fn module_init(&'static self) -> *mut ffi::PyObject { - 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<_> { - #[cfg(all(PyPy, not(Py_3_8)))] - { - const PYPY_GOOD_VERSION: [u8; 3] = [7, 3, 8]; - let version = py - .import("sys")? - .getattr("implementation")? - .getattr("version")?; - if version.lt(crate::types::PyTuple::new(py, &PYPY_GOOD_VERSION))? { - let warn = py.import("warnings")?.getattr("warn")?; - warn.call1(( - "PyPy 3.7 versions older than 7.3.8 are known to have binary \ - compatibility issues which may cause segfaults. Please upgrade.", - ))?; - } - } - Ok(unwind_safe_self.make_module(py)?.into_ptr()) - }), - ) + Ok(module) } } @@ -112,46 +94,43 @@ mod tests { #[test] fn module_init() { - unsafe { - static MODULE_DEF: ModuleDef = unsafe { - ModuleDef::new( - "test_module\0", - "some doc\0", - ModuleInitializer(|_, m| { - m.add("SOME_CONSTANT", 42)?; - Ok(()) - }), - ) - }; - Python::with_gil(|py| { - let module = MODULE_DEF.module_init(); - let pymodule: &PyModule = py.from_owned_ptr(module); - assert_eq!( - pymodule - .getattr("__name__") - .unwrap() - .extract::<&str>() - .unwrap(), - "test_module", - ); - assert_eq!( - pymodule - .getattr("__doc__") - .unwrap() - .extract::<&str>() - .unwrap(), - "some doc", - ); - assert_eq!( - pymodule - .getattr("SOME_CONSTANT") - .unwrap() - .extract::() - .unwrap(), - 42, - ); - }) - } + static MODULE_DEF: ModuleDef = unsafe { + ModuleDef::new( + "test_module\0", + "some doc\0", + ModuleInitializer(|_, m| { + m.add("SOME_CONSTANT", 42)?; + Ok(()) + }), + ) + }; + Python::with_gil(|py| { + let module = MODULE_DEF.make_module(py).unwrap().into_ref(py); + assert_eq!( + module + .getattr("__name__") + .unwrap() + .extract::<&str>() + .unwrap(), + "test_module", + ); + assert_eq!( + module + .getattr("__doc__") + .unwrap() + .extract::<&str>() + .unwrap(), + "some doc", + ); + assert_eq!( + module + .getattr("SOME_CONSTANT") + .unwrap() + .extract::() + .unwrap(), + 42, + ); + }) } #[test] diff --git a/src/impl_/trampoline.rs b/src/impl_/trampoline.rs new file mode 100644 index 00000000..44227fa7 --- /dev/null +++ b/src/impl_/trampoline.rs @@ -0,0 +1,203 @@ +//! Trampolines for various pyfunction and pymethod implementations. +//! +//! They exist to monomorphise std::panic::catch_unwind once into PyO3, rather than inline in every +//! function, thus saving a huge amount of compile-time complexity. + +use std::{ + any::Any, + os::raw::{c_int, c_void}, + panic::{self, UnwindSafe}, +}; + +use crate::{ + callback::PyCallbackOutput, ffi, impl_::panic::PanicTrap, methods::IPowModulo, + panic::PanicException, types::PyModule, GILPool, IntoPyPointer, Py, PyResult, Python, +}; + +#[inline] +pub unsafe fn module_init( + f: for<'py> unsafe fn(Python<'py>) -> PyResult>, +) -> *mut ffi::PyObject { + trampoline_inner(|py| f(py).map(|module| module.into_ptr())) +} + +#[inline] +pub unsafe fn noargs( + slf: *mut ffi::PyObject, + args: *mut ffi::PyObject, + f: for<'py> unsafe fn(Python<'py>, *mut ffi::PyObject) -> PyResult<*mut ffi::PyObject>, +) -> *mut ffi::PyObject { + debug_assert!(args.is_null()); + trampoline_inner(|py| f(py, slf)) +} + +macro_rules! trampoline { + (pub fn $name:ident($($arg_names:ident: $arg_types:ty),* $(,)?) -> $ret:ty;) => { + #[inline] + pub unsafe fn $name( + $($arg_names: $arg_types,)* + f: for<'py> unsafe fn (Python<'py>, $($arg_types),*) -> PyResult<$ret>, + ) -> $ret { + trampoline_inner(|py| f(py, $($arg_names,)*)) + } + } +} + +macro_rules! trampolines { + ($(pub fn $name:ident($($arg_names:ident: $arg_types:ty),* $(,)?) -> $ret:ty);* ;) => { + $(trampoline!(pub fn $name($($arg_names: $arg_types),*) -> $ret;));*; + } +} + +trampolines!( + pub fn fastcall_with_keywords( + slf: *mut ffi::PyObject, + args: *const *mut ffi::PyObject, + nargs: ffi::Py_ssize_t, + kwnames: *mut ffi::PyObject, + ) -> *mut ffi::PyObject; + + pub fn cfunction_with_keywords( + slf: *mut ffi::PyObject, + args: *mut ffi::PyObject, + kwargs: *mut ffi::PyObject, + ) -> *mut ffi::PyObject; +); + +#[inline] +pub unsafe fn getter( + slf: *mut ffi::PyObject, + closure: *mut c_void, + f: for<'py> unsafe fn(Python<'py>, *mut ffi::PyObject) -> PyResult<*mut ffi::PyObject>, +) -> *mut ffi::PyObject { + // PyO3 doesn't use the closure argument at present. + debug_assert!(closure.is_null()); + trampoline_inner(|py| f(py, slf)) +} + +#[inline] +pub unsafe fn setter( + slf: *mut ffi::PyObject, + value: *mut ffi::PyObject, + closure: *mut c_void, + f: for<'py> unsafe fn(Python<'py>, *mut ffi::PyObject, *mut ffi::PyObject) -> PyResult, +) -> c_int { + // PyO3 doesn't use the closure argument at present. + debug_assert!(closure.is_null()); + trampoline_inner(|py| f(py, slf, value)) +} + +// Trampolines used by slot methods +trampolines!( + pub fn getattrofunc(slf: *mut ffi::PyObject, attr: *mut ffi::PyObject) -> *mut ffi::PyObject; + + pub fn setattrofunc( + slf: *mut ffi::PyObject, + attr: *mut ffi::PyObject, + value: *mut ffi::PyObject, + ) -> c_int; + + pub fn binaryfunc(slf: *mut ffi::PyObject, arg1: *mut ffi::PyObject) -> *mut ffi::PyObject; + + pub fn descrgetfunc( + slf: *mut ffi::PyObject, + arg1: *mut ffi::PyObject, + arg2: *mut ffi::PyObject, + ) -> *mut ffi::PyObject; + + pub fn getiterfunc(slf: *mut ffi::PyObject) -> *mut ffi::PyObject; + + pub fn hashfunc(slf: *mut ffi::PyObject) -> ffi::Py_hash_t; + + pub fn inquiry(slf: *mut ffi::PyObject) -> c_int; + + pub fn iternextfunc(slf: *mut ffi::PyObject) -> *mut ffi::PyObject; + + pub fn lenfunc(slf: *mut ffi::PyObject) -> ffi::Py_ssize_t; + + pub fn newfunc( + subtype: *mut ffi::PyTypeObject, + args: *mut ffi::PyObject, + kwargs: *mut ffi::PyObject, + ) -> *mut ffi::PyObject; + + pub fn objobjproc(slf: *mut ffi::PyObject, arg1: *mut ffi::PyObject) -> c_int; + + pub fn reprfunc(slf: *mut ffi::PyObject) -> *mut ffi::PyObject; + + pub fn richcmpfunc( + slf: *mut ffi::PyObject, + other: *mut ffi::PyObject, + op: c_int, + ) -> *mut ffi::PyObject; + + pub fn ssizeargfunc(arg1: *mut ffi::PyObject, arg2: ffi::Py_ssize_t) -> *mut ffi::PyObject; + + pub fn ternaryfunc( + slf: *mut ffi::PyObject, + arg1: *mut ffi::PyObject, + arg2: *mut ffi::PyObject, + ) -> *mut ffi::PyObject; + + pub fn unaryfunc(slf: *mut ffi::PyObject) -> *mut ffi::PyObject; + + pub fn dealloc(slf: *mut ffi::PyObject) -> (); +); + +#[cfg(any(not(Py_LIMITED_API), Py_3_11))] +trampolines! { + pub fn getbufferproc(slf: *mut ffi::PyObject, buf: *mut ffi::Py_buffer, flags: c_int) -> c_int; + + pub fn releasebufferproc(slf: *mut ffi::PyObject, buf: *mut ffi::Py_buffer) -> (); +} + +// Ipowfunc is a unique case where PyO3 has its own type +// to workaround a problem on 3.7 (see IPowModulo type definition). +// Once 3.7 support dropped can just remove this. +trampoline!( + pub fn ipowfunc( + arg1: *mut ffi::PyObject, + arg2: *mut ffi::PyObject, + arg3: IPowModulo, + ) -> *mut ffi::PyObject; +); + +/// Implementation of trampoline functions, which sets up a GILPool and calls F. +/// +/// Panics during execution are trapped so that they don't propagate through any +/// outer FFI boundary. +#[inline] +pub(crate) fn trampoline_inner(body: F) -> R +where + F: for<'py> FnOnce(Python<'py>) -> PyResult + UnwindSafe, + R: PyCallbackOutput, +{ + let trap = PanicTrap::new("uncaught panic at ffi boundary"); + let pool = unsafe { GILPool::new() }; + let py = pool.python(); + let out = panic_result_into_callback_output( + py, + panic::catch_unwind(move || -> PyResult<_> { body(py) }), + ); + trap.disarm(); + out +} + +/// 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. +#[inline] +fn panic_result_into_callback_output( + py: Python<'_>, + panic_result: Result, Box>, +) -> R +where + R: PyCallbackOutput, +{ + let py_err = match panic_result { + Ok(Ok(value)) => return value, + Ok(Err(py_err)) => py_err, + Err(payload) => PanicException::from_panic_payload(payload), + }; + py_err.restore(py); + R::ERR_VALUE +} diff --git a/src/pycell.rs b/src/pycell.rs index 2dd6e4f7..e3d26c37 100644 --- a/src/pycell.rs +++ b/src/pycell.rs @@ -55,25 +55,20 @@ //! # } //! # } //! # -//! // This function is exported to Python. +//! // The function which is exported to Python looks roughly like the following //! unsafe extern "C" fn __pymethod_increment__( //! _slf: *mut pyo3::ffi::PyObject, //! _args: *mut pyo3::ffi::PyObject, //! ) -> *mut pyo3::ffi::PyObject { //! use :: pyo3 as _pyo3; -//! let gil = _pyo3::GILPool::new(); -//! let _py = gil.python(); -//! _pyo3::callback::panic_result_into_callback_output( -//! _py, -//! ::std::panic::catch_unwind(move || -> _pyo3::PyResult<_> { -//! let _cell = _py -//! .from_borrowed_ptr::<_pyo3::PyAny>(_slf) -//! .downcast::<_pyo3::PyCell>()?; -//! let mut _ref = _cell.try_borrow_mut()?; -//! let _slf: &mut Number = &mut *_ref; -//! _pyo3::callback::convert(_py, Number::increment(_slf)) -//! }), -//! ) +//! _pyo3::impl_::trampoline::noargs(_slf, _args, |py, _slf| { +//! let _cell = py +//! .from_borrowed_ptr::<_pyo3::PyAny>(_slf) +//! .downcast::<_pyo3::PyCell>()?; +//! let mut _ref = _cell.try_borrow_mut()?; +//! let _slf: &mut Number = &mut *_ref; +//! _pyo3::callback::convert(py, Number::increment(_slf)) +//! }) //! } //! ``` //! diff --git a/src/pyclass.rs b/src/pyclass.rs index d82cdd73..235d0185 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -600,8 +600,8 @@ pub(crate) unsafe extern "C" fn no_constructor_defined( _args: *mut ffi::PyObject, _kwds: *mut ffi::PyObject, ) -> *mut ffi::PyObject { - crate::callback_body!(py, { - Err::<(), _>(crate::exceptions::PyTypeError::new_err( + crate::impl_::trampoline::trampoline_inner(|_| { + Err(crate::exceptions::PyTypeError::new_err( "No constructor defined", )) }) diff --git a/src/types/function.rs b/src/types/function.rs index d01b7dda..bfd55a63 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -27,14 +27,20 @@ where F: Fn(&types::PyTuple, Option<&types::PyDict>) -> R + Send + 'static, R: crate::callback::IntoPyCallbackOutput<*mut ffi::PyObject>, { - crate::callback_body!(py, { - let boxed_fn: &F = - &*(ffi::PyCapsule_GetPointer(capsule_ptr, CLOSURE_CAPSULE_NAME.as_ptr() as *const _) - as *mut F); - let args = py.from_borrowed_ptr::(args); - let kwargs = py.from_borrowed_ptr_or_opt::(kwargs); - boxed_fn(args, kwargs) - }) + crate::impl_::trampoline::cfunction_with_keywords( + capsule_ptr, + args, + kwargs, + |py, capsule_ptr, args, kwargs| { + let boxed_fn: &F = &*(ffi::PyCapsule_GetPointer( + capsule_ptr, + CLOSURE_CAPSULE_NAME.as_ptr() as *const _, + ) as *mut F); + let args = py.from_borrowed_ptr::(args); + let kwargs = py.from_borrowed_ptr_or_opt::(kwargs); + crate::callback::convert(py, boxed_fn(args, kwargs)) + }, + ) } unsafe extern "C" fn drop_closure(capsule_ptr: *mut ffi::PyObject) diff --git a/tests/ui/static_ref.stderr b/tests/ui/static_ref.stderr index f087904c..2dd3342e 100644 --- a/tests/ui/static_ref.stderr +++ b/tests/ui/static_ref.stderr @@ -1,11 +1,10 @@ -error[E0597]: `gil` does not live long enough +error: lifetime may 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` + | ^^^^^^^^^^^^^ + | | + | lifetime `'py` defined here + | cast requires that `'py` must outlive `'static` | = note: this error originates in the attribute macro `pyfunction` (in Nightly builds, run with -Z macro-backtrace for more info)