diff --git a/CHANGELOG.md b/CHANGELOG.md index ed34b7cb1e7..48c65c6eeb2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `PyNumberProtocol::__complex__` and `PyNumberProtocol::__round__` - `PyAsyncProtocol::__aenter__` and `PyAsyncProtocol::__aexit__` - Deprecate `#[name = "..."]` attributes in favor of `#[pyo3(name = "...")]`. [#1567](https://github.com/PyO3/pyo3/pull/1567) +- Improve compilation times for projects using PyO3 [#1604](https://github.com/PyO3/pyo3/pull/1604) ### Removed - Remove deprecated exception names `BaseException` etc. [#1426](https://github.com/PyO3/pyo3/pull/1426) diff --git a/guide/src/class.md b/guide/src/class.md index 6b164ab1713..569fd717d22 100644 --- a/guide/src/class.md +++ b/guide/src/class.md @@ -757,7 +757,7 @@ impl pyo3::class::impl_::PyClassImpl for MyClass { type BaseType = PyAny; type ThreadChecker = pyo3::class::impl_::ThreadCheckerStub; - fn for_each_method_def(visitor: impl FnMut(&pyo3::class::PyMethodDefType)) { + fn for_each_method_def(visitor: &mut dyn FnMut(&pyo3::class::PyMethodDefType)) { use pyo3::class::impl_::*; let collector = PyClassImplCollector::::new(); collector.py_methods().iter() @@ -780,7 +780,7 @@ impl pyo3::class::impl_::PyClassImpl for MyClass { let collector = PyClassImplCollector::::new(); collector.call_impl() } - fn for_each_proto_slot(visitor: impl FnMut(&pyo3::ffi::PyType_Slot)) { + fn for_each_proto_slot(visitor: &mut dyn FnMut(&pyo3::ffi::PyType_Slot)) { // Implementation which uses dtolnay specialization to load all slots. use pyo3::class::impl_::*; let collector = PyClassImplCollector::::new(); diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs index 4d568d79c71..0a5ab8c1cde 100644 --- a/pyo3-macros-backend/src/pyclass.rs +++ b/pyo3-macros-backend/src/pyclass.rs @@ -436,7 +436,7 @@ fn impl_class( type BaseType = #base; type ThreadChecker = #thread_checker; - fn for_each_method_def(visitor: impl FnMut(&pyo3::class::PyMethodDefType)) { + fn for_each_method_def(visitor: &mut dyn FnMut(&pyo3::class::PyMethodDefType)) { use pyo3::class::impl_::*; let collector = PyClassImplCollector::::new(); #iter_py_methods @@ -460,7 +460,7 @@ fn impl_class( collector.call_impl() } - fn for_each_proto_slot(visitor: impl FnMut(&pyo3::ffi::PyType_Slot)) { + fn for_each_proto_slot(visitor: &mut dyn FnMut(&pyo3::ffi::PyType_Slot)) { // Implementation which uses dtolnay specialization to load all slots. use pyo3::class::impl_::*; let collector = PyClassImplCollector::::new(); diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index afe8d89721c..e27a1d04f6d 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -487,19 +487,28 @@ fn impl_arg_param( let arg_value = quote_arg_span!(#args_array[#option_pos]); *option_pos += 1; - let default = match (spec.default_value(name), arg.optional.is_some()) { - (Some(default), true) if default.to_string() != "None" => { - quote_arg_span! { Some(#default) } - } - (Some(default), _) => quote_arg_span! { #default }, - (None, true) => quote_arg_span! { None }, - (None, false) => quote_arg_span! { panic!("Failed to extract required method argument") }, - }; - let extract = if let Some(FromPyWithAttribute(expr_path)) = &arg.attrs.from_py_with { - quote_arg_span! { #expr_path(_obj).map_err(#transform_error)?} + quote_arg_span! { #expr_path(_obj).map_err(#transform_error) } } else { - quote_arg_span! { _obj.extract().map_err(#transform_error)?} + quote_arg_span! { _obj.extract().map_err(#transform_error) } + }; + + let arg_value_or_default = match (spec.default_value(name), arg.optional.is_some()) { + (Some(default), true) if default.to_string() != "None" => { + quote_arg_span! { #arg_value.map_or_else(|| Ok(Some(#default)), |_obj| #extract)? } + } + (Some(default), _) => { + quote_arg_span! { #arg_value.map_or_else(|| Ok(#default), |_obj| #extract)? } + } + (None, true) => quote_arg_span! { #arg_value.map_or(Ok(None), |_obj| #extract)? }, + (None, false) => { + quote_arg_span! { + { + let _obj = #arg_value.expect("Failed to extract required method argument"); + #extract? + } + } + } }; return if let syn::Type::Reference(tref) = arg.optional.as_ref().unwrap_or(&ty) { @@ -523,18 +532,12 @@ fn impl_arg_param( }; Ok(quote_arg_span! { - let #mut_ _tmp: #target_ty = match #arg_value { - Some(_obj) => #extract, - None => #default, - }; + let #mut_ _tmp: #target_ty = #arg_value_or_default; let #arg_name = #borrow_tmp; }) } else { Ok(quote_arg_span! { - let #arg_name = match #arg_value { - Some(_obj) => #extract, - None => #default, - }; + let #arg_name = #arg_value_or_default; }) }; diff --git a/src/callback.rs b/src/callback.rs index d81fdccc07f..e162394a49c 100644 --- a/src/callback.rs +++ b/src/callback.rs @@ -5,11 +5,13 @@ use crate::err::{PyErr, PyResult}; use crate::exceptions::PyOverflowError; use crate::ffi::{self, Py_hash_t}; -use crate::IntoPyPointer; +use crate::panic::PanicException; +use crate::{GILPool, IntoPyPointer}; use crate::{IntoPy, PyObject, Python}; -use std::isize; +use std::any::Any; use std::os::raw::c_int; -use std::panic::UnwindSafe; +use std::panic::{AssertUnwindSafe, UnwindSafe}; +use std::{isize, panic}; /// A type which can be the return type of a python C-API callback pub trait PyCallbackOutput: Copy { @@ -173,15 +175,6 @@ where value.convert(py) } -#[doc(hidden)] -#[inline] -pub fn callback_error() -> T -where - T: PyCallbackOutput, -{ - T::ERR_VALUE -} - /// 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 @@ -234,30 +227,33 @@ macro_rules! callback_body { #[doc(hidden)] pub unsafe fn handle_panic(body: F) -> R where - F: FnOnce(Python) -> crate::PyResult + UnwindSafe, + F: FnOnce(Python) -> PyResult + 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::() { - 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(),))) - } else { - Err(crate::panic::PanicException::new_err(( - "panic from Rust code", - ))) - } - } - }; + let pool = GILPool::new(); + let unwind_safe_py = AssertUnwindSafe(pool.python()); + let panic_result = panic::catch_unwind(move || -> PyResult<_> { + let py = *unwind_safe_py; + body(py) + }); + + panic_result_into_callback_output(pool.python(), panic_result) +} + +fn panic_result_into_callback_output( + py: Python, + panic_result: Result, Box>, +) -> R +where + R: PyCallbackOutput, +{ + let py_result = match panic_result { + Ok(py_result) => py_result, + Err(payload) => Err(PanicException::from_panic_payload(payload)), + }; - result.unwrap_or_else(|e| { - e.restore(pool.python()); - crate::callback::callback_error() + py_result.unwrap_or_else(|py_err| { + py_err.restore(py); + R::ERR_VALUE }) } diff --git a/src/class/impl_.rs b/src/class/impl_.rs index 3113be2bab5..a4b282ee44a 100644 --- a/src/class/impl_.rs +++ b/src/class/impl_.rs @@ -65,14 +65,14 @@ pub trait PyClassImpl: Sized { /// can be accessed by multiple threads by `threading` module. type ThreadChecker: PyClassThreadChecker; - fn for_each_method_def(_visitor: impl FnMut(&PyMethodDefType)) {} + fn for_each_method_def(_visitor: &mut dyn FnMut(&PyMethodDefType)) {} fn get_new() -> Option { None } fn get_call() -> Option { None } - fn for_each_proto_slot(_visitor: impl FnMut(&ffi::PyType_Slot)) {} + fn for_each_proto_slot(_visitor: &mut dyn FnMut(&ffi::PyType_Slot)) {} fn get_buffer() -> Option<&'static PyBufferProcs> { None } diff --git a/src/panic.rs b/src/panic.rs index 4884278d4c8..d140ac8304b 100644 --- a/src/panic.rs +++ b/src/panic.rs @@ -1,4 +1,6 @@ use crate::exceptions::PyBaseException; +use crate::PyErr; +use std::any::Any; pyo3_exception!( " @@ -11,3 +13,16 @@ pyo3_exception!( PanicException, PyBaseException ); + +impl PanicException { + // Try to format the error in the same way panic does + pub(crate) fn from_panic_payload(payload: Box) -> PyErr { + if let Some(string) = payload.downcast_ref::() { + Self::new_err((string.clone(),)) + } else if let Some(s) = payload.downcast_ref::<&str>() { + Self::new_err((s.to_string(),)) + } else { + Self::new_err(("panic from Rust code",)) + } + } +} diff --git a/src/pyclass.rs b/src/pyclass.rs index da2d3e2ac51..cae15cad025 100644 --- a/src/pyclass.rs +++ b/src/pyclass.rs @@ -206,20 +206,20 @@ where } // normal methods - let methods = py_class_method_defs::(); + let methods = py_class_method_defs(&T::for_each_method_def); if !methods.is_empty() { slots.push(ffi::Py_tp_methods, into_raw(methods)); } // properties - let props = py_class_properties::(); + let props = py_class_properties(T::Dict::IS_DUMMY, &T::for_each_method_def); if !props.is_empty() { slots.push(ffi::Py_tp_getset, into_raw(props)); } // protocol methods let mut has_gc_methods = false; - T::for_each_proto_slot(|slot| { + T::for_each_proto_slot(&mut |slot| { has_gc_methods |= slot.slot == ffi::Py_tp_clear; has_gc_methods |= slot.slot == ffi::Py_tp_traverse; slots.0.push(*slot); @@ -230,7 +230,7 @@ where name: get_type_name::(module_name)?, basicsize: std::mem::size_of::() as c_int, itemsize: 0, - flags: py_class_flags::(has_gc_methods), + flags: py_class_flags(has_gc_methods, T::IS_GC, T::IS_BASETYPE), slots: slots.0.as_mut_ptr(), }; @@ -299,22 +299,24 @@ fn tp_init_additional(type_object: *mut ffi::PyTypeObject) { #[cfg(any(Py_LIMITED_API, Py_3_10))] fn tp_init_additional(_type_object: *mut ffi::PyTypeObject) {} -fn py_class_flags(has_gc_methods: bool) -> c_uint { - let mut flags = if has_gc_methods || T::IS_GC { +fn py_class_flags(has_gc_methods: bool, is_gc: bool, is_basetype: bool) -> c_uint { + let mut flags = if has_gc_methods || is_gc { ffi::Py_TPFLAGS_DEFAULT | ffi::Py_TPFLAGS_HAVE_GC } else { ffi::Py_TPFLAGS_DEFAULT }; - if T::IS_BASETYPE { + if is_basetype { flags |= ffi::Py_TPFLAGS_BASETYPE; } flags.try_into().unwrap() } -fn py_class_method_defs() -> Vec { +fn py_class_method_defs( + for_each_method_def: &dyn Fn(&mut dyn FnMut(&PyMethodDefType)), +) -> Vec { let mut defs = Vec::new(); - T::for_each_method_def(|def| match def { + for_each_method_def(&mut |def| match def { PyMethodDefType::Method(def) | PyMethodDefType::Class(def) | PyMethodDefType::Static(def) => { @@ -381,10 +383,13 @@ const PY_GET_SET_DEF_INIT: ffi::PyGetSetDef = ffi::PyGetSetDef { }; #[allow(clippy::clippy::collapsible_if)] // for if cfg! -fn py_class_properties() -> Vec { +fn py_class_properties( + is_dummy: bool, + for_each_method_def: &dyn Fn(&mut dyn FnMut(&PyMethodDefType)), +) -> Vec { let mut defs = std::collections::HashMap::new(); - T::for_each_method_def(|def| match def { + for_each_method_def(&mut |def| match def { PyMethodDefType::Getter(getter) => { getter.copy_to(defs.entry(getter.name).or_insert(PY_GET_SET_DEF_INIT)); } @@ -398,7 +403,7 @@ fn py_class_properties() -> Vec { // PyPy doesn't automatically adds __dict__ getter / setter. // PyObject_GenericGetDict not in the limited API until Python 3.10. - push_dict_getset::(&mut props); + push_dict_getset(&mut props, is_dummy); if !props.is_empty() { props.push(unsafe { std::mem::zeroed() }); @@ -407,8 +412,8 @@ fn py_class_properties() -> Vec { } #[cfg(not(any(PyPy, all(Py_LIMITED_API, not(Py_3_10)))))] -fn push_dict_getset(props: &mut Vec) { - if !T::Dict::IS_DUMMY { +fn push_dict_getset(props: &mut Vec, is_dummy: bool) { + if !is_dummy { props.push(ffi::PyGetSetDef { name: "__dict__\0".as_ptr() as *mut c_char, get: Some(ffi::PyObject_GenericGetDict), @@ -420,4 +425,4 @@ fn push_dict_getset(props: &mut Vec) { } #[cfg(any(PyPy, all(Py_LIMITED_API, not(Py_3_10))))] -fn push_dict_getset(_: &mut Vec) {} +fn push_dict_getset(_: &mut Vec, _is_dummy: bool) {} diff --git a/src/type_object.rs b/src/type_object.rs index ebf7df8a028..cbc9e326510 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -104,6 +104,17 @@ impl LazyStaticType { }) }); + self.ensure_init(py, type_object, T::NAME, &T::for_each_method_def); + type_object + } + + fn ensure_init( + &self, + py: Python, + type_object: *mut ffi::PyTypeObject, + name: &str, + for_each_method_def: &dyn Fn(&mut dyn FnMut(&PyMethodDefType)), + ) { // We might want to fill the `tp_dict` with python instances of `T` // itself. In order to do so, we must first initialize the type object // with an empty `tp_dict`: now we can create instances of `T`. @@ -117,7 +128,7 @@ impl LazyStaticType { if self.tp_dict_filled.get(py).is_some() { // `tp_dict` is already filled: ok. - return type_object; + return; } { @@ -126,7 +137,7 @@ impl LazyStaticType { if threads.contains(&thread_id) { // Reentrant call: just return the type object, even if the // `tp_dict` is not filled yet. - return type_object; + return; } threads.push(thread_id); } @@ -136,7 +147,7 @@ impl LazyStaticType { // means that another thread can continue the initialization in the // meantime: at worst, we'll just make a useless computation. let mut items = vec![]; - T::for_each_method_def(|def| { + for_each_method_def(&mut |def| { if let PyMethodDefType::ClassAttribute(attr) = def { items.push(( extract_cstr_or_leak_cstring( @@ -162,10 +173,8 @@ impl LazyStaticType { if let Err(err) = result { err.clone_ref(py).print(py); - panic!("An error occured while initializing `{}.__dict__`", T::NAME); + panic!("An error occured while initializing `{}.__dict__`", name); } - - type_object } }