diff --git a/guide/src/ecosystem/logging.md b/guide/src/ecosystem/logging.md index ae5676c5554..c3e566b937e 100644 --- a/guide/src/ecosystem/logging.md +++ b/guide/src/ecosystem/logging.md @@ -28,7 +28,7 @@ mod my_module { } #[pymodule_init] - fn init(m: &Bound<'_, PyModule>) -> PyResult<()> { + fn init() { // A good place to install the Rust -> Python logger. pyo3_log::init(); } diff --git a/guide/src/module.md b/guide/src/module.md index eab280f2536..476cec0196a 100644 --- a/guide/src/module.md +++ b/guide/src/module.md @@ -174,3 +174,39 @@ mod my_extension { } # } ``` + +The module argument may be omitted if the initialization does not need it, for example when it only installs some global state. +The return type may then be omitted too: + +```rust,no_run +# mod procedural_module_no_arg_test { +#[pyo3::pymodule] +mod my_extension { + #[pymodule_init] + fn init() { + // Arbitrary code which does not touch the module + } +} +# } +``` + +A `Python<'_>` marker may be taken as the first argument, either on its own or followed by the module. +This suits an initializer which needs the interpreter but not the module itself: + +```rust,no_run +# mod procedural_module_py_arg_test { +#[pyo3::pymodule] +mod my_extension { + use pyo3::prelude::*; + + #[pymodule_init] + fn init(py: Python<'_>) -> PyResult<()> { + // Arbitrary code which needs the interpreter but not the module + py.import("decimal")?; + Ok(()) + } +} +# } +``` + +Prefer a form which does not take the module where possible: an initializer which is not handed the module does not add attributes to it, so [type stub generation](type-stub.md) can keep describing the module in full. diff --git a/guide/src/type-stub.md b/guide/src/type-stub.md index 2a4eb8d266d..cee684446aa 100644 --- a/guide/src/type-stub.md +++ b/guide/src/type-stub.md @@ -89,3 +89,5 @@ PyO3 also provides the smaller `pyo3-introspection` binary that allows to genera - `FromPyObject::INPUT_TYPE` and `IntoPyObject::OUTPUT_TYPE` must be implemented for PyO3 to get the proper input/output type annotations to use. - PyO3 is not able to introspect the content of `#[pymodule]` and `#[pymodule_init]` functions. If they are present, the module is tagged as incomplete using a fake `def __getattr__(name: str) -> Incomplete: ...` function [following best practices](https://typing.python.org/en/latest/guides/writing_stubs.html#incomplete-stubs). + A `#[pymodule_init]` function [declared without the module argument](module.md#procedural-initialization) is exempt: it is not handed the module, so the module is taken to be complete. + A `Python<'_>` marker on its own does not count as the module argument. diff --git a/newsfragments/6271.added.1.md b/newsfragments/6271.added.1.md new file mode 100644 index 00000000000..84d287c68c2 --- /dev/null +++ b/newsfragments/6271.added.1.md @@ -0,0 +1,2 @@ +A `#[pymodule_init]` function may now return `()` instead of `PyResult<()>`, which is useful for initialisers which cannot fail. +The return type is now checked: an initialiser returning a `Result` with a non-unit `Ok` value used to compile, with that value silently discarded, and is now rejected. diff --git a/newsfragments/6271.added.md b/newsfragments/6271.added.md new file mode 100644 index 00000000000..c36bef2287d --- /dev/null +++ b/newsfragments/6271.added.md @@ -0,0 +1 @@ +`#[pymodule_init]` may now be written without the module argument, and may take a `Python<'_>` marker in front of it or instead of it. An initialiser which is not handed the module leaves it complete, so with `experimental-inspect` the module is no longer tagged incomplete and its stubs no longer get a `def __getattr__(name: str) -> Incomplete: ...` catch-all. diff --git a/pyo3-macros-backend/src/module.rs b/pyo3-macros-backend/src/module.rs index bf307df9550..682d692516a 100644 --- a/pyo3-macros-backend/src/module.rs +++ b/pyo3-macros-backend/src/module.rs @@ -13,12 +13,14 @@ use crate::{ }, combine_errors::CombineErrors, get_doc, + method::FnArg, pyclass::PyClassPyO3Option, pyfunction::{impl_wrap_pyfunction, PyFunctionOptions}, + pymethod::split_off_python_arg, utils::{has_attribute, has_attribute_with_namespace, Ctx, IdentOrStr, PythonDoc}, }; use proc_macro2::{Span, TokenStream}; -use quote::{quote, ToTokens}; +use quote::{quote, quote_spanned, ToTokens}; use std::ffi::CString; use syn::LitCStr; use syn::{ @@ -164,6 +166,9 @@ pub fn pymodule_module_impl( } let mut pymodule_init = None; + // An initialiser which receives the module can add attributes the macro cannot see; one which + // does not is what lets the module stay complete for introspection. + let mut pymodule_init_takes_module = false; let mut module_consts = Vec::new(); let mut module_consts_cfg_attrs = Vec::new(); @@ -196,7 +201,29 @@ pub fn pymodule_module_impl( item_fn.span() => "`#[pyfunction]` cannot be used alongside `#[pymodule_init]`" ); ensure_spanned!(pymodule_init.is_none(), item_fn.span() => "only one `#[pymodule_init]` may be specified"); - pymodule_init = Some(quote! { #ident(module)?; }); + let ident = ident.clone(); + let sig_span = item_fn.sig.span(); + let return_span = item_fn.sig.output.span(); + let args: Vec<_> = item_fn + .sig + .inputs + .iter_mut() + .map(FnArg::parse) + .try_combine_syn_errors()?; + let (py_arg, args) = split_off_python_arg(&args); + ensure_spanned!( + args.len() <= 1, + sig_span => "`#[pymodule_init]` takes an optional `Python` argument followed by an optional module argument" + ); + pymodule_init_takes_module = !args.is_empty(); + let call_args = py_arg + .map(|_| quote! { module.py() }) + .into_iter() + .chain(pymodule_init_takes_module.then(|| quote! { module })); + let pyo3_path = pyo3_path.to_tokens_spanned(return_span); + pymodule_init = Some(quote_spanned! { return_span => + #pyo3_path::impl_::pymodule::PyModuleInitResult::into_result(#ident(#(#call_args),*))?; + }); } else if has_attribute(&item_fn.attrs, "pyfunction") || has_attribute_with_namespace( &item_fn.attrs, @@ -382,7 +409,7 @@ pub fn pymodule_module_impl( &module_items, &module_items_cfg_attrs, doc.as_ref(), - pymodule_init.is_some(), + pymodule_init_takes_module, ); #[cfg(not(feature = "experimental-inspect"))] let introspection = quote! {}; diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs index 683e03ad898..23ab7df5efb 100644 --- a/pyo3-macros-backend/src/pymethod.rs +++ b/pyo3-macros-backend/src/pymethod.rs @@ -1036,7 +1036,9 @@ fn impl_call_deleter( } /// Split an argument of pyo3::Python from the front of the arg list, if present -fn split_off_python_arg<'a, 'b>(args: &'a [FnArg<'b>]) -> (Option<&'a PyArg<'b>>, &'a [FnArg<'b>]) { +pub(crate) fn split_off_python_arg<'a, 'b>( + args: &'a [FnArg<'b>], +) -> (Option<&'a PyArg<'b>>, &'a [FnArg<'b>]) { match args { [FnArg::Py(py), args @ ..] => (Some(py), args), args => (None, args), diff --git a/pytests/src/othermod.rs b/pytests/src/othermod.rs index 1c3c768e342..797afa24cb0 100644 --- a/pytests/src/othermod.rs +++ b/pytests/src/othermod.rs @@ -36,4 +36,9 @@ pub mod othermod { pub const USIZE_MIN: usize = usize::MIN; #[pymodule_export] pub const USIZE_MAX: usize = usize::MAX; + + // An initialiser without the module argument leaves the module complete for introspection: + // `stubs/othermod.pyi` has no `__getattr__` catch-all. + #[pymodule_init] + fn init() {} } diff --git a/src/impl_/pymodule.rs b/src/impl_/pymodule.rs index 8c20828b632..ac101c850b9 100644 --- a/src/impl_/pymodule.rs +++ b/src/impl_/pymodule.rs @@ -41,7 +41,7 @@ use crate::{ ffi, impl_::pyfunction::PyFunctionDef, types::{PyModule, PyModuleMethods}, - Bound, PyClass, PyResult, PyTypeInfo, + Bound, PyClass, PyErr, PyResult, PyTypeInfo, }; use crate::{ sync::PyOnceLock, @@ -485,6 +485,30 @@ unsafe impl Sync for PyModuleSlots {} #[cfg(not(all(Py_LIMITED_API, Py_GIL_DISABLED)))] unsafe impl Sync for PyModuleDefSlots {} +/// Used to accept either `()` or `Result<(), E>` from a `#[pymodule_init]` function. +#[diagnostic::on_unimplemented( + message = "`{Self}` is not a suitable return value for `#[pymodule_init]` functions", + note = "`#[pymodule_init]` functions may return `()` or `Result<(), E>` where `PyErr: From`" +)] +pub trait PyModuleInitResult { + fn into_result(self) -> PyResult<()>; +} + +impl PyModuleInitResult for () { + fn into_result(self) -> PyResult<()> { + Ok(()) + } +} + +impl PyModuleInitResult for Result<(), E> +where + PyErr: From, +{ + fn into_result(self) -> PyResult<()> { + self.map_err(PyErr::from) + } +} + /// Trait to add an element (class, function...) to a module. /// /// Currently only implemented for classes. diff --git a/tests/test_declarative_module.rs b/tests/test_declarative_module.rs index e0d77f69e97..638d81cab7c 100644 --- a/tests/test_declarative_module.rs +++ b/tests/test_declarative_module.rs @@ -1,5 +1,6 @@ #![cfg(feature = "macros")] +use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; use std::sync::OnceLock; use pyo3::create_exception; @@ -271,3 +272,120 @@ fn test_inner_module_full_path() { py_assert!(py, m, "m.full_path_inner"); }) } + +static NO_ARG_INIT_RAN: AtomicBool = AtomicBool::new(false); + +#[pymodule] +mod module_with_no_arg_init { + use super::NO_ARG_INIT_RAN; + use pyo3::prelude::*; + use std::sync::atomic::Ordering; + + #[pyfunction] + fn triple(x: usize) -> usize { + x * 3 + } + + #[pymodule_init] + #[expect(clippy::unnecessary_wraps)] + fn init() -> PyResult<()> { + NO_ARG_INIT_RAN.store(true, Ordering::SeqCst); + Ok(()) + } +} + +#[test] +fn test_pymodule_init_without_module() { + Python::attach(|py| { + let m = pyo3::wrap_pymodule!(module_with_no_arg_init)(py); + let m = m.bind(py); + py_assert!(py, m, "m.triple(3) == 9"); + assert!(NO_ARG_INIT_RAN.load(Ordering::SeqCst)); + }) +} + +static UNIT_INIT_RAN: AtomicBool = AtomicBool::new(false); + +#[pymodule] +mod module_with_unit_init { + use super::UNIT_INIT_RAN; + use pyo3::prelude::*; + use std::sync::atomic::Ordering; + + #[pyfunction] + fn quadruple(x: usize) -> usize { + x * 4 + } + + #[pymodule_init] + fn init() { + UNIT_INIT_RAN.store(true, Ordering::SeqCst); + } +} + +#[test] +fn test_pymodule_init_returning_unit() { + Python::attach(|py| { + let m = pyo3::wrap_pymodule!(module_with_unit_init)(py); + let m = m.bind(py); + py_assert!(py, m, "m.quadruple(3) == 12"); + assert!(UNIT_INIT_RAN.load(Ordering::SeqCst)); + }) +} + +static PY_INIT_VERSION_MAJOR: AtomicU8 = AtomicU8::new(0); + +#[pymodule] +mod module_with_py_init { + use super::PY_INIT_VERSION_MAJOR; + use pyo3::prelude::*; + use std::sync::atomic::Ordering; + + #[pyfunction] + fn quintuple(x: usize) -> usize { + x * 5 + } + + #[pymodule_init] + fn init(py: Python<'_>) { + PY_INIT_VERSION_MAJOR.store(py.version_info().major, Ordering::SeqCst); + } +} + +#[test] +fn test_pymodule_init_with_only_python() { + Python::attach(|py| { + let m = pyo3::wrap_pymodule!(module_with_py_init)(py); + let m = m.bind(py); + py_assert!(py, m, "m.quintuple(3) == 15"); + assert_eq!( + PY_INIT_VERSION_MAJOR.load(Ordering::SeqCst), + py.version_info().major + ); + }) +} + +#[pymodule] +mod module_with_py_and_module_init { + use pyo3::prelude::*; + + #[pyfunction] + fn sextuple(x: usize) -> usize { + x * 6 + } + + #[pymodule_init] + fn init(py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> { + m.add("added_by_init", pyo3::types::PyString::new(py, "hello")) + } +} + +#[test] +fn test_pymodule_init_with_python_and_module() { + Python::attach(|py| { + let m = pyo3::wrap_pymodule!(module_with_py_and_module_init)(py); + let m = m.bind(py); + py_assert!(py, m, "m.sextuple(3) == 18"); + py_assert!(py, m, "m.added_by_init == 'hello'"); + }) +} diff --git a/tests/ui/invalid_pymodule_init_args.rs b/tests/ui/invalid_pymodule_init_args.rs new file mode 100644 index 00000000000..4fcb2238547 --- /dev/null +++ b/tests/ui/invalid_pymodule_init_args.rs @@ -0,0 +1,25 @@ +use pyo3::prelude::*; + +#[pymodule] +mod module { + use pyo3::prelude::*; + + #[pymodule_init] + fn init(_m: &Bound<'_, PyModule>, _extra: usize) -> PyResult<()> { +//~^ ERROR: `#[pymodule_init]` takes an optional `Python` argument followed by an optional module argument + Ok(()) + } +} + +#[pymodule] +mod module_with_python_last { + use pyo3::prelude::*; + + #[pymodule_init] + fn init(_m: &Bound<'_, PyModule>, _py: Python<'_>) -> PyResult<()> { +//~^ ERROR: `#[pymodule_init]` takes an optional `Python` argument followed by an optional module argument + Ok(()) + } +} + +fn main() {} diff --git a/tests/ui/invalid_pymodule_init_args.stderr b/tests/ui/invalid_pymodule_init_args.stderr new file mode 100644 index 00000000000..bcb3a7a250f --- /dev/null +++ b/tests/ui/invalid_pymodule_init_args.stderr @@ -0,0 +1,13 @@ +error: `#[pymodule_init]` takes an optional `Python` argument followed by an optional module argument + --> tests/ui/invalid_pymodule_init_args.rs:8:5 + | +8 | fn init(_m: &Bound<'_, PyModule>, _extra: usize) -> PyResult<()> { + | ^^ + +error: `#[pymodule_init]` takes an optional `Python` argument followed by an optional module argument + --> tests/ui/invalid_pymodule_init_args.rs:19:5 + | +19 | fn init(_m: &Bound<'_, PyModule>, _py: Python<'_>) -> PyResult<()> { + | ^^ + +error: aborting due to 2 previous errors diff --git a/tests/ui/invalid_pymodule_init_pyfunction.rs b/tests/ui/invalid_pymodule_init_pyfunction.rs new file mode 100644 index 00000000000..bfc54069e17 --- /dev/null +++ b/tests/ui/invalid_pymodule_init_pyfunction.rs @@ -0,0 +1,15 @@ +use pyo3::prelude::*; + +#[pymodule] +mod module { + use pyo3::prelude::*; + + #[pymodule_init] + #[pyfunction] +//~^ ERROR: `#[pyfunction]` cannot be used alongside `#[pymodule_init]` + fn init(_m: &Bound<'_, PyModule>) -> PyResult<()> { + Ok(()) + } +} + +fn main() {} diff --git a/tests/ui/invalid_pymodule_init_pyfunction.stderr b/tests/ui/invalid_pymodule_init_pyfunction.stderr new file mode 100644 index 00000000000..fe5d4657972 --- /dev/null +++ b/tests/ui/invalid_pymodule_init_pyfunction.stderr @@ -0,0 +1,7 @@ +error: `#[pyfunction]` cannot be used alongside `#[pymodule_init]` + --> tests/ui/invalid_pymodule_init_pyfunction.rs:8:5 + | +8 | #[pyfunction] + | ^ + +error: aborting due to 1 previous error diff --git a/tests/ui/invalid_pymodule_init_return.rs b/tests/ui/invalid_pymodule_init_return.rs new file mode 100644 index 00000000000..db9bf25921f --- /dev/null +++ b/tests/ui/invalid_pymodule_init_return.rs @@ -0,0 +1,23 @@ +use pyo3::prelude::*; + +#[pymodule] +mod module { + #[pymodule_init] + fn init() -> usize { + //~^ ERROR: `usize` is not a suitable return value for `#[pymodule_init]` functions + 0 + } +} + +#[pymodule] +mod module_result { + use pyo3::prelude::*; + + #[pymodule_init] + fn init() -> PyResult { + //~^ ERROR: `Result` is not a suitable return value for `#[pymodule_init]` functions + Ok(0) + } +} + +fn main() {} diff --git a/tests/ui/invalid_pymodule_init_return.stderr b/tests/ui/invalid_pymodule_init_return.stderr new file mode 100644 index 00000000000..6ab07e639c5 --- /dev/null +++ b/tests/ui/invalid_pymodule_init_return.stderr @@ -0,0 +1,42 @@ +error[E0277]: `usize` is not a suitable return value for `#[pymodule_init]` functions + --> tests/ui/invalid_pymodule_init_return.rs:6:8 + | + 6 | fn init() -> usize { + | ^^^^^^^- + | | | + | | required by a bound introduced by this call + | the trait `pyo3::impl_::pymodule::PyModuleInitResult` is not implemented for `usize` + | + = note: `#[pymodule_init]` functions may return `()` or `Result<(), E>` where `PyErr: From` +help: the following other types implement trait `pyo3::impl_::pymodule::PyModuleInitResult` + --> src/impl_/pymodule.rs + | + | impl PyModuleInitResult for () { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `()` +... + | / impl PyModuleInitResult for Result<(), E> + | | where + | | PyErr: From, + | |___________________^ `Result<(), E>` + +error[E0277]: `Result` is not a suitable return value for `#[pymodule_init]` functions + --> tests/ui/invalid_pymodule_init_return.rs:17:8 + | + 17 | fn init() -> PyResult { + | ^^^^^^^- + | | | + | | required by a bound introduced by this call + | the trait `pyo3::impl_::pymodule::PyModuleInitResult` is not implemented for `Result` + | + = note: `#[pymodule_init]` functions may return `()` or `Result<(), E>` where `PyErr: From` +help: the trait `pyo3::impl_::pymodule::PyModuleInitResult` is implemented for `Result<(), E>` + --> src/impl_/pymodule.rs + | + | / impl PyModuleInitResult for Result<(), E> + | | where + | | PyErr: From, + | |___________________^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`.