From 9db0ea924c91a3605b2a8fdd9e11c3d8f5ce8bd2 Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Sun, 25 Feb 2024 13:27:16 +0000 Subject: [PATCH 1/8] Support Bound in pymodule and pyfunction macros Co-authored-by: David Hewitt --- guide/src/module.md | 2 +- guide/src/python_from_rust.md | 2 +- pyo3-macros-backend/src/module.rs | 11 +++-- pyo3-macros-backend/src/pyfunction.rs | 6 +-- src/derive_utils.rs | 44 +++++++++++++++++- src/impl_.rs | 1 - src/impl_/pyfunction.rs | 10 ----- src/impl_/pymodule.rs | 23 ++++++++++ src/macros.rs | 33 +++++++++++++- src/prelude.rs | 2 +- src/types/function.rs | 64 ++++++++++++++++++++++----- src/types/module.rs | 21 +++++++-- tests/test_module.rs | 13 +++--- 13 files changed, 186 insertions(+), 46 deletions(-) delete mode 100644 src/impl_/pyfunction.rs diff --git a/guide/src/module.md b/guide/src/module.md index 789c3d91ccb..91a3d9b9730 100644 --- a/guide/src/module.md +++ b/guide/src/module.md @@ -79,7 +79,7 @@ fn parent_module(py: Python<'_>, m: &PyModule) -> PyResult<()> { fn register_child_module(py: Python<'_>, parent_module: &PyModule) -> PyResult<()> { let child_module = PyModule::new_bound(py, "child_module")?; - child_module.add_function(&wrap_pyfunction!(func, child_module.as_gil_ref())?.as_borrowed())?; + child_module.add_function(wrap_pyfunction_bound!(func, &child_module)?)?; parent_module.add_submodule(child_module.as_gil_ref())?; Ok(()) } diff --git a/guide/src/python_from_rust.md b/guide/src/python_from_rust.md index 973f997cbf8..cb9c50919b6 100644 --- a/guide/src/python_from_rust.md +++ b/guide/src/python_from_rust.md @@ -311,7 +311,7 @@ fn main() -> PyResult<()> { Python::with_gil(|py| { // Create new module let foo_module = PyModule::new_bound(py, "foo")?; - foo_module.add_function(&wrap_pyfunction!(add_one, foo_module.as_gil_ref())?.as_borrowed())?; + foo_module.add_function(wrap_pyfunction_bound!(add_one, &foo_module)?)?; // Import and get sys.modules let sys = PyModule::import_bound(py, "sys")?; diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 6907e484f71..016ba732aee 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -194,7 +194,7 @@ pub fn pymodule_module_impl(mut module: syn::ItemMod) -> Result { /// module pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result { let options = PyModuleOptions::from_attrs(&mut function.attrs)?; - process_functions_in_module(&options, &mut function)?; + process_functions_in_module(&mut function)?; let krate = get_pyo3_crate(&options.krate); let ident = &function.sig.ident; let vis = &function.vis; @@ -215,13 +215,13 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result use #krate::impl_::pymodule as impl_; fn __pyo3_pymodule(module: &#krate::Bound<'_, #krate::types::PyModule>) -> #krate::PyResult<()> { - #ident(module.py(), module.as_gil_ref()) + #ident(module.py(), ::std::convert::Into::into(impl_::BoundModule(module))) } impl #ident::MakeDef { const fn make_def() -> impl_::ModuleDef { + const INITIALIZER: impl_::ModuleInitializer = impl_::ModuleInitializer(__pyo3_pymodule); unsafe { - const INITIALIZER: impl_::ModuleInitializer = impl_::ModuleInitializer(__pyo3_pymodule); impl_::ModuleDef::new( #ident::__PYO3_NAME, #doc, @@ -260,9 +260,8 @@ fn module_initialization(options: PyModuleOptions, ident: &syn::Ident) -> TokenS } /// Finds and takes care of the #[pyfn(...)] in `#[pymodule]` -fn process_functions_in_module(options: &PyModuleOptions, func: &mut syn::ItemFn) -> Result<()> { +fn process_functions_in_module(func: &mut syn::ItemFn) -> Result<()> { let mut stmts: Vec = Vec::new(); - let krate = get_pyo3_crate(&options.krate); for mut stmt in func.block.stmts.drain(..) { if let syn::Stmt::Item(Item::Fn(func)) = &mut stmt { @@ -272,7 +271,7 @@ fn process_functions_in_module(options: &PyModuleOptions, func: &mut syn::ItemFn let name = &func.sig.ident; let statements: Vec = syn::parse_quote! { #wrapped_function - #module_name.add_function(#krate::impl_::pyfunction::_wrap_pyfunction(&#name::DEF, #module_name)?)?; + #module_name.add_function(#module_name.wrap_pyfunction(&#name::DEF)?)?; }; stmts.extend(statements); } diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs index 7b48585cddc..4155d6cd0f0 100644 --- a/pyo3-macros-backend/src/pyfunction.rs +++ b/pyo3-macros-backend/src/pyfunction.rs @@ -268,12 +268,12 @@ pub fn impl_wrap_pyfunction( #[doc(hidden)] #vis mod #name { pub(crate) struct MakeDef; - pub const DEF: #krate::impl_::pyfunction::PyMethodDef = MakeDef::DEF; + pub const DEF: #krate::impl_::pymethods::PyMethodDef = MakeDef::DEF; pub fn add_to_module(module: &#krate::Bound<'_, #krate::types::PyModule>) -> #krate::PyResult<()> { use #krate::prelude::PyModuleMethods; use ::std::convert::Into; - module.add_function(&#krate::types::PyCFunction::internal_new(&DEF, module.as_gil_ref().into())?) + module.add_function(#krate::types::PyCFunction::internal_new(&DEF, module.as_gil_ref().into())?) } } @@ -284,7 +284,7 @@ pub fn impl_wrap_pyfunction( const _: () = { use #krate as _pyo3; impl #name::MakeDef { - const DEF: #krate::impl_::pyfunction::PyMethodDef = #methoddef; + const DEF: #krate::impl_::pymethods::PyMethodDef = #methoddef; } #[allow(non_snake_case)] diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 4ccb38f901b..fdf5a142e56 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -1,6 +1,10 @@ //! Functionality for the code generated by the derive backend -use crate::{types::PyModule, Python}; +use crate::impl_::pymethods::PyMethodDef; +use crate::{ + types::{PyCFunction, PyModule}, + Bound, PyResult, Python, +}; /// Enum to abstract over the arguments of Python function wrappers. pub enum PyFunctionArguments<'a> { @@ -18,6 +22,10 @@ impl<'a> PyFunctionArguments<'a> { } } } + + pub fn wrap_pyfunction(self, method_def: &'a PyMethodDef) -> PyResult<&PyCFunction> { + Ok(PyCFunction::internal_new(method_def, self)?.into_gil_ref()) + } } impl<'a> From> for PyFunctionArguments<'a> { @@ -31,3 +39,37 @@ impl<'a> From<&'a PyModule> for PyFunctionArguments<'a> { PyFunctionArguments::PyModule(module) } } + +/// Enum to abstract over the arguments of Python function wrappers. +pub enum PyFunctionArgumentsBound<'a, 'py> { + Python(Python<'py>), + PyModule(&'a Bound<'py, PyModule>), +} + +impl<'a, 'py> PyFunctionArgumentsBound<'a, 'py> { + pub fn into_py_and_maybe_module(self) -> (Python<'py>, Option<&'a Bound<'py, PyModule>>) { + match self { + PyFunctionArgumentsBound::Python(py) => (py, None), + PyFunctionArgumentsBound::PyModule(module) => { + let py = module.py(); + (py, Some(module)) + } + } + } + + pub fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult> { + PyCFunction::internal_new_bound(method_def, self) + } +} + +impl<'a, 'py> From> for PyFunctionArgumentsBound<'a, 'py> { + fn from(py: Python<'py>) -> PyFunctionArgumentsBound<'a, 'py> { + PyFunctionArgumentsBound::Python(py) + } +} + +impl<'a, 'py> From<&'a Bound<'py, PyModule>> for PyFunctionArgumentsBound<'a, 'py> { + fn from(module: &'a Bound<'py, PyModule>) -> PyFunctionArgumentsBound<'a, 'py> { + PyFunctionArgumentsBound::PyModule(module) + } +} diff --git a/src/impl_.rs b/src/impl_.rs index 77f9ff4ea1f..14832e6cd66 100644 --- a/src/impl_.rs +++ b/src/impl_.rs @@ -16,7 +16,6 @@ pub(crate) mod not_send; pub mod panic; pub mod pycell; pub mod pyclass; -pub mod pyfunction; pub mod pymethods; pub mod pymodule; #[doc(hidden)] diff --git a/src/impl_/pyfunction.rs b/src/impl_/pyfunction.rs deleted file mode 100644 index 464beeb8ce5..00000000000 --- a/src/impl_/pyfunction.rs +++ /dev/null @@ -1,10 +0,0 @@ -use crate::{derive_utils::PyFunctionArguments, types::PyCFunction, PyResult}; - -pub use crate::impl_::pymethods::PyMethodDef; - -pub fn _wrap_pyfunction<'a>( - method_def: &PyMethodDef, - py_or_module: impl Into>, -) -> PyResult<&'a PyCFunction> { - PyCFunction::internal_new(method_def, py_or_module.into()).map(|x| x.into_gil_ref()) -} diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index 9fff799c37b..562b3d14c6e 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -147,6 +147,29 @@ impl PyAddToModule for T { } } +pub struct BoundModule<'a, 'py>(pub &'a Bound<'py, PyModule>); + +impl<'a> From> for &'a PyModule { + #[inline] + fn from(bound: BoundModule<'a, 'a>) -> Self { + bound.0.as_gil_ref() + } +} + +impl<'a, 'py> From> for &'a Bound<'py, PyModule> { + #[inline] + fn from(bound: BoundModule<'a, 'py>) -> Self { + bound.0 + } +} + +impl From> for Py { + #[inline] + fn from(bound: BoundModule<'_, '_>) -> Self { + bound.0.clone().unbind() + } +} + #[cfg(test)] mod tests { use std::sync::atomic::{AtomicBool, Ordering}; diff --git a/src/macros.rs b/src/macros.rs index 9b0d2816882..4ad5a71055a 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -126,13 +126,42 @@ macro_rules! py_run_impl { macro_rules! wrap_pyfunction { ($function:path) => { &|py_or_module| { + use $crate::derive_utils::PyFunctionArguments; use $function as wrapped_pyfunction; - $crate::impl_::pyfunction::_wrap_pyfunction(&wrapped_pyfunction::DEF, py_or_module) + let function_arguments: PyFunctionArguments<'_> = + ::std::convert::Into::into(py_or_module); + function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) } }; ($function:path, $py_or_module:expr) => {{ + use $crate::derive_utils::PyFunctionArguments; use $function as wrapped_pyfunction; - $crate::impl_::pyfunction::_wrap_pyfunction(&wrapped_pyfunction::DEF, $py_or_module) + let function_arguments: PyFunctionArguments<'_> = ::std::convert::Into::into($py_or_module); + function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) + }}; +} + +/// Wraps a Rust function annotated with [`#[pyfunction]`](macro@crate::pyfunction). +/// +/// This can be used with [`PyModule::add_function`](crate::types::PyModule::add_function) to add free +/// functions to a [`PyModule`](crate::types::PyModule) - see its documentation for more information. +#[macro_export] +macro_rules! wrap_pyfunction_bound { + ($function:path) => { + &|py_or_module| { + use $crate::derive_utils::PyFunctionArgumentsBound; + use $function as wrapped_pyfunction; + let function_arguments: PyFunctionArgumentsBound<'_, '_> = + ::std::convert::Into::into($py_or_module); + function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) + } + }; + ($function:path, $py_or_module:expr) => {{ + use $crate::derive_utils::PyFunctionArgumentsBound; + use $function as wrapped_pyfunction; + let function_arguments: PyFunctionArgumentsBound<'_, '_> = + ::std::convert::Into::into($py_or_module); + function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) }}; } diff --git a/src/prelude.rs b/src/prelude.rs index 1de7c3acd2d..dcf4fe71cdd 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -23,7 +23,7 @@ pub use crate::PyNativeType; pub use pyo3_macros::{pyclass, pyfunction, pymethods, pymodule, FromPyObject}; #[cfg(feature = "macros")] -pub use crate::wrap_pyfunction; +pub use crate::{wrap_pyfunction, wrap_pyfunction_bound}; pub use crate::types::any::PyAnyMethods; pub use crate::types::boolobject::PyBoolMethods; diff --git a/src/types/function.rs b/src/types/function.rs index 7cbb05e2a48..5e8eda1ada5 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -1,8 +1,9 @@ -use crate::derive_utils::PyFunctionArguments; +use crate::derive_utils::{PyFunctionArguments, PyFunctionArgumentsBound}; use crate::ffi_ptr_ext::FfiPtrExt; use crate::methods::PyMethodDefDestructor; use crate::py_result_ext::PyResultExt; use crate::types::capsule::PyCapsuleMethods; +use crate::types::module::PyModuleMethods; use crate::{ ffi, impl_::pymethods::{self, PyMethodDef}, @@ -33,17 +34,25 @@ impl PyCFunction { doc: &'static str, py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a Self> { - Self::new_with_keywords_bound(fun, name, doc, py_or_module).map(Bound::into_gil_ref) + Self::internal_new( + &PyMethodDef::cfunction_with_keywords( + name, + pymethods::PyCFunctionWithKeywords(fun), + doc, + ), + py_or_module, + ) + .map(Bound::into_gil_ref) } /// Create a new built-in function with keywords (*args and/or **kwargs). - pub fn new_with_keywords_bound<'a>( + pub fn new_with_keywords_bound<'py>( fun: ffi::PyCFunctionWithKeywords, name: &'static str, doc: &'static str, - py_or_module: PyFunctionArguments<'a>, - ) -> PyResult> { - Self::internal_new( + py_or_module: PyFunctionArgumentsBound<'_, 'py>, + ) -> PyResult> { + Self::internal_new_bound( &PyMethodDef::cfunction_with_keywords( name, pymethods::PyCFunctionWithKeywords(fun), @@ -67,17 +76,21 @@ impl PyCFunction { doc: &'static str, py_or_module: PyFunctionArguments<'a>, ) -> PyResult<&'a Self> { - Self::new_bound(fun, name, doc, py_or_module).map(Bound::into_gil_ref) + Self::internal_new( + &PyMethodDef::noargs(name, pymethods::PyCFunction(fun), doc), + py_or_module, + ) + .map(Bound::into_gil_ref) } /// Create a new built-in function which takes no arguments. - pub fn new_bound<'a>( + pub fn new_bound<'py>( fun: ffi::PyCFunction, name: &'static str, doc: &'static str, - py_or_module: PyFunctionArguments<'a>, - ) -> PyResult> { - Self::internal_new( + py_or_module: PyFunctionArgumentsBound<'_, 'py>, + ) -> PyResult> { + Self::internal_new_bound( &PyMethodDef::noargs(name, pymethods::PyCFunction(fun), doc), py_or_module, ) @@ -189,6 +202,35 @@ impl PyCFunction { .downcast_into_unchecked() } } + + #[doc(hidden)] + pub(crate) fn internal_new_bound<'py>( + method_def: &PyMethodDef, + py_or_module: PyFunctionArgumentsBound<'_, 'py>, + ) -> PyResult> { + let (py, module) = py_or_module.into_py_and_maybe_module(); + let (mod_ptr, module_name): (_, Option>) = if let Some(m) = module { + let mod_ptr = m.as_ptr(); + (mod_ptr, Some(m.name()?.into_py(py))) + } else { + (std::ptr::null_mut(), None) + }; + let (def, destructor) = method_def.as_method_def()?; + + // FIXME: stop leaking the def and destructor + let def = Box::into_raw(Box::new(def)); + std::mem::forget(destructor); + + let module_name_ptr = module_name + .as_ref() + .map_or(std::ptr::null_mut(), Py::as_ptr); + + unsafe { + ffi::PyCFunction_NewEx(def, mod_ptr, module_name_ptr) + .assume_owned_or_err(py) + .downcast_into_unchecked() + } + } } fn closure_capsule_name() -> &'static CStr { diff --git a/src/types/module.rs b/src/types/module.rs index 245d38d9e08..42e27afa77b 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -1,6 +1,8 @@ use crate::callback::IntoPyCallbackOutput; +use crate::derive_utils::{PyFunctionArguments, PyFunctionArgumentsBound}; use crate::err::{PyErr, PyResult}; use crate::ffi_ptr_ext::FfiPtrExt; +use crate::impl_::pymethods::PyMethodDef; use crate::py_result_ext::PyResultExt; use crate::pyclass::PyClass; use crate::types::{ @@ -399,7 +401,13 @@ impl PyModule { /// [1]: crate::prelude::pyfunction /// [2]: crate::wrap_pyfunction pub fn add_function<'a>(&'a self, fun: &'a PyCFunction) -> PyResult<()> { - self.as_borrowed().add_function(&fun.as_borrowed()) + let name = fun.getattr(__name__(self.py()))?.extract()?; + self.add(name, fun) + } + + #[doc(hidden)] + pub fn wrap_pyfunction<'a>(&'a self, method_def: &'a PyMethodDef) -> PyResult<&PyCFunction> { + PyFunctionArguments::PyModule(self).wrap_pyfunction(method_def) } } @@ -590,7 +598,10 @@ pub trait PyModuleMethods<'py> { /// /// [1]: crate::prelude::pyfunction /// [2]: crate::wrap_pyfunction - fn add_function(&self, fun: &Bound<'_, PyCFunction>) -> PyResult<()>; + fn add_function(&self, fun: Bound<'_, PyCFunction>) -> PyResult<()>; + + #[doc(hidden)] + fn wrap_pyfunction(&self, method_def: &PyMethodDef) -> PyResult>; } impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> { @@ -700,10 +711,14 @@ impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> { self.add(name, module) } - fn add_function(&self, fun: &Bound<'_, PyCFunction>) -> PyResult<()> { + fn add_function(&self, fun: Bound<'_, PyCFunction>) -> PyResult<()> { let name = fun.getattr(__name__(self.py()))?; self.add(name.downcast_into::()?, fun) } + + fn wrap_pyfunction(&self, method_def: &PyMethodDef) -> PyResult> { + PyFunctionArgumentsBound::PyModule(self).wrap_pyfunction(method_def) + } } fn __all__(py: Python<'_>) -> &Bound<'_, PyString> { diff --git a/tests/test_module.rs b/tests/test_module.rs index 9d14f243d50..726f68ae588 100644 --- a/tests/test_module.rs +++ b/tests/test_module.rs @@ -240,7 +240,7 @@ fn subfunction() -> String { } fn submodule(module: &Bound<'_, PyModule>) -> PyResult<()> { - module.add_function(&wrap_pyfunction!(subfunction, module.as_gil_ref())?.as_borrowed())?; + module.add_function(wrap_pyfunction_bound!(subfunction, module)?)?; Ok(()) } @@ -295,18 +295,19 @@ fn test_module_nesting() { // Test that argument parsing specification works for pyfunctions #[pyfunction(signature = (a=5, *args))] -fn ext_vararg_fn(py: Python<'_>, a: i32, args: &PyTuple) -> PyObject { - [a.to_object(py), args.into()].to_object(py) +fn ext_vararg_fn(py: Python<'_>, a: i32, args: &Bound<'_, PyTuple>) -> PyObject { + [a.to_object(py), args.into_py(py)].to_object(py) } #[pymodule] -fn vararg_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> { +fn vararg_module(_py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> { #[pyfn(m, signature = (a=5, *args))] - fn int_vararg_fn(py: Python<'_>, a: i32, args: &PyTuple) -> PyObject { + fn int_vararg_fn(py: Python<'_>, a: i32, args: &Bound<'_, PyTuple>) -> PyObject { ext_vararg_fn(py, a, args) } - m.add_function(wrap_pyfunction!(ext_vararg_fn, m)?).unwrap(); + m.add_function(wrap_pyfunction_bound!(ext_vararg_fn, m)?) + .unwrap(); Ok(()) } From a05342e184f08977e44eedbbbb25790b7fc38851 Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Mon, 26 Feb 2024 21:12:52 +0000 Subject: [PATCH 2/8] Remove spurious $ character Co-authored-by: Matthew Neeley --- src/macros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/macros.rs b/src/macros.rs index 4ad5a71055a..448a6506488 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -152,7 +152,7 @@ macro_rules! wrap_pyfunction_bound { use $crate::derive_utils::PyFunctionArgumentsBound; use $function as wrapped_pyfunction; let function_arguments: PyFunctionArgumentsBound<'_, '_> = - ::std::convert::Into::into($py_or_module); + ::std::convert::Into::into(py_or_module); function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) } }; From e310b4a795564c9c688787b98503b30b9fcbfa91 Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Mon, 26 Feb 2024 23:35:36 +0000 Subject: [PATCH 3/8] Rework PyCFunction::new_bound signatures This allows us to remove the awkward `PyFunctionArgumentsBound` enum. --- src/derive_utils.rs | 36 +----------------------------------- src/macros.rs | 14 ++++---------- src/types/function.rs | 20 ++++++++++++-------- src/types/module.rs | 4 ++-- tests/test_pyfunction.rs | 6 ++++-- 5 files changed, 23 insertions(+), 57 deletions(-) diff --git a/src/derive_utils.rs b/src/derive_utils.rs index fdf5a142e56..09433427966 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -3,7 +3,7 @@ use crate::impl_::pymethods::PyMethodDef; use crate::{ types::{PyCFunction, PyModule}, - Bound, PyResult, Python, + PyResult, Python, }; /// Enum to abstract over the arguments of Python function wrappers. @@ -39,37 +39,3 @@ impl<'a> From<&'a PyModule> for PyFunctionArguments<'a> { PyFunctionArguments::PyModule(module) } } - -/// Enum to abstract over the arguments of Python function wrappers. -pub enum PyFunctionArgumentsBound<'a, 'py> { - Python(Python<'py>), - PyModule(&'a Bound<'py, PyModule>), -} - -impl<'a, 'py> PyFunctionArgumentsBound<'a, 'py> { - pub fn into_py_and_maybe_module(self) -> (Python<'py>, Option<&'a Bound<'py, PyModule>>) { - match self { - PyFunctionArgumentsBound::Python(py) => (py, None), - PyFunctionArgumentsBound::PyModule(module) => { - let py = module.py(); - (py, Some(module)) - } - } - } - - pub fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult> { - PyCFunction::internal_new_bound(method_def, self) - } -} - -impl<'a, 'py> From> for PyFunctionArgumentsBound<'a, 'py> { - fn from(py: Python<'py>) -> PyFunctionArgumentsBound<'a, 'py> { - PyFunctionArgumentsBound::Python(py) - } -} - -impl<'a, 'py> From<&'a Bound<'py, PyModule>> for PyFunctionArgumentsBound<'a, 'py> { - fn from(module: &'a Bound<'py, PyModule>) -> PyFunctionArgumentsBound<'a, 'py> { - PyFunctionArgumentsBound::PyModule(module) - } -} diff --git a/src/macros.rs b/src/macros.rs index 448a6506488..b510643c86f 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -148,20 +148,14 @@ macro_rules! wrap_pyfunction { #[macro_export] macro_rules! wrap_pyfunction_bound { ($function:path) => { - &|py_or_module| { - use $crate::derive_utils::PyFunctionArgumentsBound; + &|module| { use $function as wrapped_pyfunction; - let function_arguments: PyFunctionArgumentsBound<'_, '_> = - ::std::convert::Into::into(py_or_module); - function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) + module.wrap_pyfunction(&wrapped_pyfunction::DEF) } }; - ($function:path, $py_or_module:expr) => {{ - use $crate::derive_utils::PyFunctionArgumentsBound; + ($function:path, $module:expr) => {{ use $function as wrapped_pyfunction; - let function_arguments: PyFunctionArgumentsBound<'_, '_> = - ::std::convert::Into::into($py_or_module); - function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) + $module.wrap_pyfunction(&wrapped_pyfunction::DEF) }}; } diff --git a/src/types/function.rs b/src/types/function.rs index 5e8eda1ada5..75173d2aadb 100644 --- a/src/types/function.rs +++ b/src/types/function.rs @@ -1,4 +1,4 @@ -use crate::derive_utils::{PyFunctionArguments, PyFunctionArgumentsBound}; +use crate::derive_utils::PyFunctionArguments; use crate::ffi_ptr_ext::FfiPtrExt; use crate::methods::PyMethodDefDestructor; use crate::py_result_ext::PyResultExt; @@ -7,7 +7,7 @@ use crate::types::module::PyModuleMethods; use crate::{ ffi, impl_::pymethods::{self, PyMethodDef}, - types::{PyCapsule, PyDict, PyString, PyTuple}, + types::{PyCapsule, PyDict, PyModule, PyString, PyTuple}, }; use crate::{Bound, IntoPy, Py, PyAny, PyResult, Python}; use std::cell::UnsafeCell; @@ -47,18 +47,20 @@ impl PyCFunction { /// Create a new built-in function with keywords (*args and/or **kwargs). pub fn new_with_keywords_bound<'py>( + py: Python<'py>, fun: ffi::PyCFunctionWithKeywords, name: &'static str, doc: &'static str, - py_or_module: PyFunctionArgumentsBound<'_, 'py>, + module: Option<&Bound<'py, PyModule>>, ) -> PyResult> { Self::internal_new_bound( + py, &PyMethodDef::cfunction_with_keywords( name, pymethods::PyCFunctionWithKeywords(fun), doc, ), - py_or_module, + module, ) } @@ -85,14 +87,16 @@ impl PyCFunction { /// Create a new built-in function which takes no arguments. pub fn new_bound<'py>( + py: Python<'py>, fun: ffi::PyCFunction, name: &'static str, doc: &'static str, - py_or_module: PyFunctionArgumentsBound<'_, 'py>, + module: Option<&Bound<'py, PyModule>>, ) -> PyResult> { Self::internal_new_bound( + py, &PyMethodDef::noargs(name, pymethods::PyCFunction(fun), doc), - py_or_module, + module, ) } @@ -205,10 +209,10 @@ impl PyCFunction { #[doc(hidden)] pub(crate) fn internal_new_bound<'py>( + py: Python<'py>, method_def: &PyMethodDef, - py_or_module: PyFunctionArgumentsBound<'_, 'py>, + module: Option<&Bound<'py, PyModule>>, ) -> PyResult> { - let (py, module) = py_or_module.into_py_and_maybe_module(); let (mod_ptr, module_name): (_, Option>) = if let Some(m) = module { let mod_ptr = m.as_ptr(); (mod_ptr, Some(m.name()?.into_py(py))) diff --git a/src/types/module.rs b/src/types/module.rs index 42e27afa77b..b8f3ecb53a9 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -1,5 +1,5 @@ use crate::callback::IntoPyCallbackOutput; -use crate::derive_utils::{PyFunctionArguments, PyFunctionArgumentsBound}; +use crate::derive_utils::PyFunctionArguments; use crate::err::{PyErr, PyResult}; use crate::ffi_ptr_ext::FfiPtrExt; use crate::impl_::pymethods::PyMethodDef; @@ -717,7 +717,7 @@ impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> { } fn wrap_pyfunction(&self, method_def: &PyMethodDef) -> PyResult> { - PyFunctionArgumentsBound::PyModule(self).wrap_pyfunction(method_def) + PyCFunction::internal_new_bound(self.py(), method_def, Some(self)) } } diff --git a/tests/test_pyfunction.rs b/tests/test_pyfunction.rs index 14748418687..838f7353737 100644 --- a/tests/test_pyfunction.rs +++ b/tests/test_pyfunction.rs @@ -324,10 +324,11 @@ fn test_pycfunction_new() { } let py_fn = PyCFunction::new_bound( + py, c_fn, "py_fn", "py_fn for test (this is the docstring)", - py.into(), + None, ) .unwrap(); @@ -381,10 +382,11 @@ fn test_pycfunction_new_with_keywords() { } let py_fn = PyCFunction::new_with_keywords_bound( + py, c_fn, "py_fn", "py_fn for test (this is the docstring)", - py.into(), + None, ) .unwrap(); From b67f93f4b8c1a4ce98ea8f950e5e9d2e19fec1d3 Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Mon, 26 Feb 2024 23:52:08 +0000 Subject: [PATCH 4/8] Use BoundRef instead of BoundModule --- pyo3-macros-backend/src/module.rs | 3 ++- src/impl_/pymodule.rs | 23 ----------------------- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 016ba732aee..2be76db9dbb 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -213,9 +213,10 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result // inside a function body) const _: () = { use #krate::impl_::pymodule as impl_; + use #krate::impl_::pymethods::BoundRef; fn __pyo3_pymodule(module: &#krate::Bound<'_, #krate::types::PyModule>) -> #krate::PyResult<()> { - #ident(module.py(), ::std::convert::Into::into(impl_::BoundModule(module))) + #ident(module.py(), ::std::convert::Into::into(BoundRef(module))) } impl #ident::MakeDef { diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index 562b3d14c6e..9fff799c37b 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -147,29 +147,6 @@ impl PyAddToModule for T { } } -pub struct BoundModule<'a, 'py>(pub &'a Bound<'py, PyModule>); - -impl<'a> From> for &'a PyModule { - #[inline] - fn from(bound: BoundModule<'a, 'a>) -> Self { - bound.0.as_gil_ref() - } -} - -impl<'a, 'py> From> for &'a Bound<'py, PyModule> { - #[inline] - fn from(bound: BoundModule<'a, 'py>) -> Self { - bound.0 - } -} - -impl From> for Py { - #[inline] - fn from(bound: BoundModule<'_, '_>) -> Self { - bound.0.clone().unbind() - } -} - #[cfg(test)] mod tests { use std::sync::atomic::{AtomicBool, Ordering}; From 90a1efe1fc7b23a6f62733fbc98fe50a23a46b48 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 27 Feb 2024 07:52:29 +0000 Subject: [PATCH 5/8] support argument deduction for `wrap_pyfunction_bound!` --- pyo3-macros-backend/src/module.rs | 13 +++-- src/derive_utils.rs | 10 +--- src/impl_.rs | 1 + src/impl_/pyfunction.rs | 72 +++++++++++++++++++++++++ src/macros.rs | 17 +++--- src/types/module.rs | 14 ----- tests/test_wrap_pyfunction_deduction.rs | 9 ++++ 7 files changed, 98 insertions(+), 38 deletions(-) create mode 100644 src/impl_/pyfunction.rs diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 2be76db9dbb..461ca0b586d 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -194,7 +194,7 @@ pub fn pymodule_module_impl(mut module: syn::ItemMod) -> Result { /// module pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result { let options = PyModuleOptions::from_attrs(&mut function.attrs)?; - process_functions_in_module(&mut function)?; + process_functions_in_module(&options, &mut function)?; let krate = get_pyo3_crate(&options.krate); let ident = &function.sig.ident; let vis = &function.vis; @@ -261,8 +261,13 @@ fn module_initialization(options: PyModuleOptions, ident: &syn::Ident) -> TokenS } /// Finds and takes care of the #[pyfn(...)] in `#[pymodule]` -fn process_functions_in_module(func: &mut syn::ItemFn) -> Result<()> { - let mut stmts: Vec = Vec::new(); +fn process_functions_in_module(options: &PyModuleOptions, func: &mut syn::ItemFn) -> Result<()> { + let krate = get_pyo3_crate(&options.krate); + + let mut stmts: Vec = vec![syn::parse_quote!( + #[allow(unknown_lints, unused_imports, redundant_imports)] + use #krate::{PyNativeType, types::PyModuleMethods}; + )]; for mut stmt in func.block.stmts.drain(..) { if let syn::Stmt::Item(Item::Fn(func)) = &mut stmt { @@ -272,7 +277,7 @@ fn process_functions_in_module(func: &mut syn::ItemFn) -> Result<()> { let name = &func.sig.ident; let statements: Vec = syn::parse_quote! { #wrapped_function - #module_name.add_function(#module_name.wrap_pyfunction(&#name::DEF)?)?; + #module_name.as_borrowed().add_function(#krate::wrap_pyfunction_bound!(#name, #module_name.as_borrowed())?)?; }; stmts.extend(statements); } diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 09433427966..4ccb38f901b 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -1,10 +1,6 @@ //! Functionality for the code generated by the derive backend -use crate::impl_::pymethods::PyMethodDef; -use crate::{ - types::{PyCFunction, PyModule}, - PyResult, Python, -}; +use crate::{types::PyModule, Python}; /// Enum to abstract over the arguments of Python function wrappers. pub enum PyFunctionArguments<'a> { @@ -22,10 +18,6 @@ impl<'a> PyFunctionArguments<'a> { } } } - - pub fn wrap_pyfunction(self, method_def: &'a PyMethodDef) -> PyResult<&PyCFunction> { - Ok(PyCFunction::internal_new(method_def, self)?.into_gil_ref()) - } } impl<'a> From> for PyFunctionArguments<'a> { diff --git a/src/impl_.rs b/src/impl_.rs index 14832e6cd66..77f9ff4ea1f 100644 --- a/src/impl_.rs +++ b/src/impl_.rs @@ -16,6 +16,7 @@ pub(crate) mod not_send; pub mod panic; pub mod pycell; pub mod pyclass; +pub mod pyfunction; pub mod pymethods; pub mod pymodule; #[doc(hidden)] diff --git a/src/impl_/pyfunction.rs b/src/impl_/pyfunction.rs new file mode 100644 index 00000000000..7e0f1f1d311 --- /dev/null +++ b/src/impl_/pyfunction.rs @@ -0,0 +1,72 @@ +use crate::{ + derive_utils::PyFunctionArguments, + types::{PyCFunction, PyModule}, + Borrowed, Bound, PyResult, Python, +}; + +pub use crate::impl_::pymethods::PyMethodDef; + +pub fn wrap_pyfunction<'a>( + method_def: &PyMethodDef, + py_or_module: impl Into>, +) -> PyResult<&'a PyCFunction> { + PyCFunction::internal_new(method_def, py_or_module.into()).map(|x| x.into_gil_ref()) +} + +/// Trait to enable the use of `wrap_pyfunction` with both `Python` and `PyModule`. +pub trait WrapPyFunctionArg<'py> { + fn py(&self) -> Python<'py>; + fn module(&self) -> Option<&Bound<'py, PyModule>>; +} + +impl<'py> WrapPyFunctionArg<'py> for Bound<'py, PyModule> { + fn py(&self) -> Python<'py> { + Bound::py(self) + } + fn module(&self) -> Option<&Bound<'py, PyModule>> { + Some(self) + } +} + +impl<'py> WrapPyFunctionArg<'py> for &'_ Bound<'py, PyModule> { + fn py(&self) -> Python<'py> { + Bound::py(self) + } + fn module(&self) -> Option<&Bound<'py, PyModule>> { + Some(self) + } +} + +impl<'py> WrapPyFunctionArg<'py> for Borrowed<'_, 'py, PyModule> { + fn py(&self) -> Python<'py> { + Bound::py(self) + } + fn module(&self) -> Option<&Bound<'py, PyModule>> { + Some(self) + } +} + +impl<'py> WrapPyFunctionArg<'py> for &'_ Borrowed<'_, 'py, PyModule> { + fn py(&self) -> Python<'py> { + Bound::py(self) + } + fn module(&self) -> Option<&Bound<'py, PyModule>> { + Some(self) + } +} + +impl<'py> WrapPyFunctionArg<'py> for Python<'py> { + fn py(&self) -> Python<'py> { + *self + } + fn module(&self) -> Option<&Bound<'py, PyModule>> { + None + } +} + +pub fn wrap_pyfunction_bound<'py>( + method_def: &PyMethodDef, + arg: impl WrapPyFunctionArg<'py>, +) -> PyResult> { + PyCFunction::internal_new_bound(arg.py(), method_def, arg.module()) +} diff --git a/src/macros.rs b/src/macros.rs index b510643c86f..568da2c186d 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -126,18 +126,13 @@ macro_rules! py_run_impl { macro_rules! wrap_pyfunction { ($function:path) => { &|py_or_module| { - use $crate::derive_utils::PyFunctionArguments; use $function as wrapped_pyfunction; - let function_arguments: PyFunctionArguments<'_> = - ::std::convert::Into::into(py_or_module); - function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) + $crate::impl_::pyfunction::wrap_pyfunction(&wrapped_pyfunction::DEF, py_or_module) } }; ($function:path, $py_or_module:expr) => {{ - use $crate::derive_utils::PyFunctionArguments; use $function as wrapped_pyfunction; - let function_arguments: PyFunctionArguments<'_> = ::std::convert::Into::into($py_or_module); - function_arguments.wrap_pyfunction(&wrapped_pyfunction::DEF) + $crate::impl_::pyfunction::wrap_pyfunction(&wrapped_pyfunction::DEF, $py_or_module) }}; } @@ -148,14 +143,14 @@ macro_rules! wrap_pyfunction { #[macro_export] macro_rules! wrap_pyfunction_bound { ($function:path) => { - &|module| { + &|py_or_module| { use $function as wrapped_pyfunction; - module.wrap_pyfunction(&wrapped_pyfunction::DEF) + $crate::impl_::pyfunction::wrap_pyfunction_bound(&wrapped_pyfunction::DEF, py_or_module) } }; - ($function:path, $module:expr) => {{ + ($function:path, $py_or_module:expr) => {{ use $function as wrapped_pyfunction; - $module.wrap_pyfunction(&wrapped_pyfunction::DEF) + $crate::impl_::pyfunction::wrap_pyfunction_bound(&wrapped_pyfunction::DEF, $py_or_module) }}; } diff --git a/src/types/module.rs b/src/types/module.rs index b8f3ecb53a9..75fc6625846 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -1,8 +1,6 @@ use crate::callback::IntoPyCallbackOutput; -use crate::derive_utils::PyFunctionArguments; use crate::err::{PyErr, PyResult}; use crate::ffi_ptr_ext::FfiPtrExt; -use crate::impl_::pymethods::PyMethodDef; use crate::py_result_ext::PyResultExt; use crate::pyclass::PyClass; use crate::types::{ @@ -404,11 +402,6 @@ impl PyModule { let name = fun.getattr(__name__(self.py()))?.extract()?; self.add(name, fun) } - - #[doc(hidden)] - pub fn wrap_pyfunction<'a>(&'a self, method_def: &'a PyMethodDef) -> PyResult<&PyCFunction> { - PyFunctionArguments::PyModule(self).wrap_pyfunction(method_def) - } } /// Implementation of functionality for [`PyModule`]. @@ -599,9 +592,6 @@ pub trait PyModuleMethods<'py> { /// [1]: crate::prelude::pyfunction /// [2]: crate::wrap_pyfunction fn add_function(&self, fun: Bound<'_, PyCFunction>) -> PyResult<()>; - - #[doc(hidden)] - fn wrap_pyfunction(&self, method_def: &PyMethodDef) -> PyResult>; } impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> { @@ -715,10 +705,6 @@ impl<'py> PyModuleMethods<'py> for Bound<'py, PyModule> { let name = fun.getattr(__name__(self.py()))?; self.add(name.downcast_into::()?, fun) } - - fn wrap_pyfunction(&self, method_def: &PyMethodDef) -> PyResult> { - PyCFunction::internal_new_bound(self.py(), method_def, Some(self)) - } } fn __all__(py: Python<'_>) -> &Bound<'_, PyString> { diff --git a/tests/test_wrap_pyfunction_deduction.rs b/tests/test_wrap_pyfunction_deduction.rs index 8723ad43fbd..52c9adcb6d7 100644 --- a/tests/test_wrap_pyfunction_deduction.rs +++ b/tests/test_wrap_pyfunction_deduction.rs @@ -13,3 +13,12 @@ pub fn add_wrapped(wrapper: &impl Fn(Python<'_>) -> PyResult<&PyCFunction>) { fn wrap_pyfunction_deduction() { add_wrapped(wrap_pyfunction!(f)); } + +pub fn add_wrapped_bound(wrapper: &impl Fn(Python<'_>) -> PyResult>) { + let _ = wrapper; +} + +#[test] +fn wrap_pyfunction_deduction_bound() { + add_wrapped_bound(wrap_pyfunction_bound!(f)); +} From 4af75fe088e4068b9d6fd17bb33ffda098c65b9e Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Tue, 27 Feb 2024 08:20:59 +0000 Subject: [PATCH 6/8] support `wrap_pyfunction!` with `Bound` input/output --- src/impl_/pyfunction.rs | 84 ++++++++++++++++++++--------------------- src/macros.rs | 31 ++++++++++++--- tests/test_module.rs | 2 +- 3 files changed, 67 insertions(+), 50 deletions(-) diff --git a/src/impl_/pyfunction.rs b/src/impl_/pyfunction.rs index 7e0f1f1d311..531e0c93192 100644 --- a/src/impl_/pyfunction.rs +++ b/src/impl_/pyfunction.rs @@ -1,72 +1,68 @@ use crate::{ - derive_utils::PyFunctionArguments, types::{PyCFunction, PyModule}, Borrowed, Bound, PyResult, Python, }; pub use crate::impl_::pymethods::PyMethodDef; -pub fn wrap_pyfunction<'a>( - method_def: &PyMethodDef, - py_or_module: impl Into>, -) -> PyResult<&'a PyCFunction> { - PyCFunction::internal_new(method_def, py_or_module.into()).map(|x| x.into_gil_ref()) +/// Trait to enable the use of `wrap_pyfunction` with both `Python` and `PyModule`, +/// and also to infer the return type of either `&'py PyCFunction` or `Bound<'py, PyCFunction>`. +pub trait WrapPyFunctionArg<'py, T> { + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult; } -/// Trait to enable the use of `wrap_pyfunction` with both `Python` and `PyModule`. -pub trait WrapPyFunctionArg<'py> { - fn py(&self) -> Python<'py>; - fn module(&self) -> Option<&Bound<'py, PyModule>>; +impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for Bound<'py, PyModule> { + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult> { + PyCFunction::internal_new_bound(self.py(), method_def, Some(&self)) + } } -impl<'py> WrapPyFunctionArg<'py> for Bound<'py, PyModule> { - fn py(&self) -> Python<'py> { - Bound::py(self) - } - fn module(&self) -> Option<&Bound<'py, PyModule>> { - Some(self) +impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for &'_ Bound<'py, PyModule> { + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult> { + PyCFunction::internal_new_bound(self.py(), method_def, Some(self)) } } -impl<'py> WrapPyFunctionArg<'py> for &'_ Bound<'py, PyModule> { - fn py(&self) -> Python<'py> { - Bound::py(self) - } - fn module(&self) -> Option<&Bound<'py, PyModule>> { - Some(self) +impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for Borrowed<'_, 'py, PyModule> { + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult> { + PyCFunction::internal_new_bound(self.py(), method_def, Some(&self)) } } -impl<'py> WrapPyFunctionArg<'py> for Borrowed<'_, 'py, PyModule> { - fn py(&self) -> Python<'py> { - Bound::py(self) - } - fn module(&self) -> Option<&Bound<'py, PyModule>> { - Some(self) +impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for &'_ Borrowed<'_, 'py, PyModule> { + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult> { + PyCFunction::internal_new_bound(self.py(), method_def, Some(self)) } } -impl<'py> WrapPyFunctionArg<'py> for &'_ Borrowed<'_, 'py, PyModule> { - fn py(&self) -> Python<'py> { - Bound::py(self) - } - fn module(&self) -> Option<&Bound<'py, PyModule>> { - Some(self) +// For Python<'py>, only the GIL Ref form exists to avoid causing type inference to kick in. +// The `wrap_pyfunction_bound!` macro is needed for the Bound form. +impl<'py> WrapPyFunctionArg<'py, &'py PyCFunction> for Python<'py> { + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<&'py PyCFunction> { + PyCFunction::internal_new(method_def, self.into()).map(Bound::into_gil_ref) } } -impl<'py> WrapPyFunctionArg<'py> for Python<'py> { - fn py(&self) -> Python<'py> { - *self +impl<'py> WrapPyFunctionArg<'py, &'py PyCFunction> for &'py PyModule { + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult<&'py PyCFunction> { + PyCFunction::internal_new(method_def, self.into()).map(Bound::into_gil_ref) } - fn module(&self) -> Option<&Bound<'py, PyModule>> { - None +} + +/// Helper for `wrap_pyfunction_bound!` to guarantee return type of `Bound<'py, PyCFunction>`. +pub struct OnlyBound(pub T); + +impl<'py, T> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for OnlyBound +where + T: WrapPyFunctionArg<'py, Bound<'py, PyCFunction>>, +{ + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult> { + WrapPyFunctionArg::wrap_pyfunction(self.0, method_def) } } -pub fn wrap_pyfunction_bound<'py>( - method_def: &PyMethodDef, - arg: impl WrapPyFunctionArg<'py>, -) -> PyResult> { - PyCFunction::internal_new_bound(arg.py(), method_def, arg.module()) +impl<'py> WrapPyFunctionArg<'py, Bound<'py, PyCFunction>> for OnlyBound> { + fn wrap_pyfunction(self, method_def: &PyMethodDef) -> PyResult> { + PyCFunction::internal_new_bound(self.0, method_def, None) + } } diff --git a/src/macros.rs b/src/macros.rs index 568da2c186d..c1fb0b8fd41 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -121,18 +121,33 @@ macro_rules! py_run_impl { /// Wraps a Rust function annotated with [`#[pyfunction]`](macro@crate::pyfunction). /// /// This can be used with [`PyModule::add_function`](crate::types::PyModule::add_function) to add free -/// functions to a [`PyModule`](crate::types::PyModule) - see its documentation for more information. +/// functions to a [`PyModule`](crate::types::PyModule) - see its documentation for more +/// information. +/// +/// During the migration from the GIL Ref API to the Bound API, the return type of this macro will +/// be either the `&'py PyModule` GIL Ref or `Bound<'py, PyModule>` according to the second +/// argument. +/// +/// For backwards compatibility, if the second argument is `Python<'py>` then the return type will +/// be `&'py PyModule` GIL Ref. To get `Bound<'py, PyModule>`, use the [`wrap_pyfunction_bound!`] +/// macro instead. #[macro_export] macro_rules! wrap_pyfunction { ($function:path) => { &|py_or_module| { use $function as wrapped_pyfunction; - $crate::impl_::pyfunction::wrap_pyfunction(&wrapped_pyfunction::DEF, py_or_module) + $crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction( + py_or_module, + &wrapped_pyfunction::DEF, + ) } }; ($function:path, $py_or_module:expr) => {{ use $function as wrapped_pyfunction; - $crate::impl_::pyfunction::wrap_pyfunction(&wrapped_pyfunction::DEF, $py_or_module) + $crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction( + $py_or_module, + &wrapped_pyfunction::DEF, + ) }}; } @@ -145,12 +160,18 @@ macro_rules! wrap_pyfunction_bound { ($function:path) => { &|py_or_module| { use $function as wrapped_pyfunction; - $crate::impl_::pyfunction::wrap_pyfunction_bound(&wrapped_pyfunction::DEF, py_or_module) + $crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction( + $crate::impl_::pyfunction::OnlyBound(py_or_module), + &wrapped_pyfunction::DEF, + ) } }; ($function:path, $py_or_module:expr) => {{ use $function as wrapped_pyfunction; - $crate::impl_::pyfunction::wrap_pyfunction_bound(&wrapped_pyfunction::DEF, $py_or_module) + $crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction( + $crate::impl_::pyfunction::OnlyBound($py_or_module), + &wrapped_pyfunction::DEF, + ) }}; } diff --git a/tests/test_module.rs b/tests/test_module.rs index 726f68ae588..c5be730548a 100644 --- a/tests/test_module.rs +++ b/tests/test_module.rs @@ -411,7 +411,7 @@ fn pyfunction_with_pass_module_in_attribute(module: &PyModule) -> PyResult<&str> } #[pymodule] -fn module_with_functions_with_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> { +fn module_with_functions_with_module(_py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_function(wrap_pyfunction!(pyfunction_with_module, m)?)?; m.add_function(wrap_pyfunction!(pyfunction_with_module_gil_ref, m)?)?; m.add_function(wrap_pyfunction!(pyfunction_with_module_owned, m)?)?; From df1da21e7280d4101cc21ad97df93b5b9e3a5aed Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Tue, 27 Feb 2024 09:42:17 +0000 Subject: [PATCH 7/8] Fix docs link to `wrap_pyfunction_bound!` --- src/macros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/macros.rs b/src/macros.rs index c1fb0b8fd41..33f378e7b83 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -129,7 +129,7 @@ macro_rules! py_run_impl { /// argument. /// /// For backwards compatibility, if the second argument is `Python<'py>` then the return type will -/// be `&'py PyModule` GIL Ref. To get `Bound<'py, PyModule>`, use the [`wrap_pyfunction_bound!`] +/// be `&'py PyModule` GIL Ref. To get `Bound<'py, PyModule>`, use the [`crate::wrap_pyfunction_bound!`] /// macro instead. #[macro_export] macro_rules! wrap_pyfunction { From f7a4e825bad6c5a047019aca495ed4cf9f29f193 Mon Sep 17 00:00:00 2001 From: Lily Foote Date: Tue, 27 Feb 2024 10:25:12 +0000 Subject: [PATCH 8/8] Revert back to wrap_pyfunction! --- guide/src/module.md | 2 +- guide/src/python_from_rust.md | 2 +- pyo3-macros-backend/src/module.rs | 2 +- tests/test_module.rs | 5 ++--- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/guide/src/module.md b/guide/src/module.md index 91a3d9b9730..53c390bec06 100644 --- a/guide/src/module.md +++ b/guide/src/module.md @@ -79,7 +79,7 @@ fn parent_module(py: Python<'_>, m: &PyModule) -> PyResult<()> { fn register_child_module(py: Python<'_>, parent_module: &PyModule) -> PyResult<()> { let child_module = PyModule::new_bound(py, "child_module")?; - child_module.add_function(wrap_pyfunction_bound!(func, &child_module)?)?; + child_module.add_function(wrap_pyfunction!(func, &child_module)?)?; parent_module.add_submodule(child_module.as_gil_ref())?; Ok(()) } diff --git a/guide/src/python_from_rust.md b/guide/src/python_from_rust.md index cb9c50919b6..b51bbe592eb 100644 --- a/guide/src/python_from_rust.md +++ b/guide/src/python_from_rust.md @@ -311,7 +311,7 @@ fn main() -> PyResult<()> { Python::with_gil(|py| { // Create new module let foo_module = PyModule::new_bound(py, "foo")?; - foo_module.add_function(wrap_pyfunction_bound!(add_one, &foo_module)?)?; + foo_module.add_function(wrap_pyfunction!(add_one, &foo_module)?)?; // Import and get sys.modules let sys = PyModule::import_bound(py, "sys")?; diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index 461ca0b586d..0c83e842433 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -277,7 +277,7 @@ fn process_functions_in_module(options: &PyModuleOptions, func: &mut syn::ItemFn let name = &func.sig.ident; let statements: Vec = syn::parse_quote! { #wrapped_function - #module_name.as_borrowed().add_function(#krate::wrap_pyfunction_bound!(#name, #module_name.as_borrowed())?)?; + #module_name.as_borrowed().add_function(#krate::wrap_pyfunction!(#name, #module_name.as_borrowed())?)?; }; stmts.extend(statements); } diff --git a/tests/test_module.rs b/tests/test_module.rs index c5be730548a..5763043e57d 100644 --- a/tests/test_module.rs +++ b/tests/test_module.rs @@ -240,7 +240,7 @@ fn subfunction() -> String { } fn submodule(module: &Bound<'_, PyModule>) -> PyResult<()> { - module.add_function(wrap_pyfunction_bound!(subfunction, module)?)?; + module.add_function(wrap_pyfunction!(subfunction, module)?)?; Ok(()) } @@ -306,8 +306,7 @@ fn vararg_module(_py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> { ext_vararg_fn(py, a, args) } - m.add_function(wrap_pyfunction_bound!(ext_vararg_fn, m)?) - .unwrap(); + m.add_function(wrap_pyfunction!(ext_vararg_fn, m)?).unwrap(); Ok(()) }