refactor to remove add_to_module functions from generated code (#4009)

* refactor to remove add_to_module functions from generated code

* mrsv, newsfragment

* clippy

newsfragments/4009.fixed.md

@@ -0,0 +1 @@
+Fix some uncovered code blocks emitted by `#[pymodule]`, `#[pyfunction]` and `#[pyclass]` macros.

pyo3-macros-backend/src/module.rs

@@ -267,7 +267,7 @@ pub fn pymodule_module_impl(mut module: syn::ItemMod) -> Result<TokenStream> {
                 use #pyo3_path::impl_::pymodule::PyAddToModule;
                 #(
                     #(#module_items_cfg_attrs)*
-                    #module_items::add_to_module(module)?;
+                    #module_items::_PYO3_DEF.add_to_module(module)?;
                 )*
                 #pymodule_init
                 Ok(())
@@ -358,21 +358,19 @@ fn module_initialization(options: PyModuleOptions, ident: &syn::Ident) -> TokenS
     let pyinit_symbol = format!("PyInit_{}", name);
 
     quote! {
+        #[doc(hidden)]
         pub const __PYO3_NAME: &'static str = concat!(stringify!(#name), "\0");
 
         pub(super) struct MakeDef;
-        pub static DEF: #pyo3_path::impl_::pymodule::ModuleDef = MakeDef::make_def();
-
-        pub fn add_to_module(module: &#pyo3_path::Bound<'_, #pyo3_path::types::PyModule>) -> #pyo3_path::PyResult<()> {
-            use #pyo3_path::prelude::PyModuleMethods;
-            module.add_submodule(DEF.make_module(module.py())?.bind(module.py()))
-        }
+        #[doc(hidden)]
+        pub static _PYO3_DEF: #pyo3_path::impl_::pymodule::ModuleDef = MakeDef::make_def();
 
         /// This autogenerated function is called by the python interpreter when importing
         /// the module.
+        #[doc(hidden)]
         #[export_name = #pyinit_symbol]
         pub unsafe extern "C" fn __pyo3_init() -> *mut #pyo3_path::ffi::PyObject {
-            #pyo3_path::impl_::trampoline::module_init(|py| DEF.make_module(py))
+            #pyo3_path::impl_::trampoline::module_init(|py| _PYO3_DEF.make_module(py))
         }
     }
 }

pyo3-macros-backend/src/pyclass.rs

@@ -1620,11 +1620,9 @@ impl<'a> PyClassImplsBuilder<'a> {
         let Ctx { pyo3_path } = ctx;
         let cls = self.cls;
         quote! {
-            impl #pyo3_path::impl_::pymodule::PyAddToModule for #cls {
-                fn add_to_module(module: &#pyo3_path::Bound<'_, #pyo3_path::types::PyModule>) -> #pyo3_path::PyResult<()> {
-                    use #pyo3_path::types::PyModuleMethods;
-                    module.add_class::<Self>()
-                }
+            impl #cls {
+                #[doc(hidden)]
+                const _PYO3_DEF: #pyo3_path::impl_::pymodule::AddClassToModule<Self> = #pyo3_path::impl_::pymodule::AddClassToModule::new();
             }
         }
     }

pyo3-macros-backend/src/pyfunction.rs

@@ -269,13 +269,7 @@ pub fn impl_wrap_pyfunction(
         #[doc(hidden)]
         #vis mod #name {
             pub(crate) struct MakeDef;
-            pub const DEF: #pyo3_path::impl_::pymethods::PyMethodDef = MakeDef::DEF;
-
-            pub fn add_to_module(module: &#pyo3_path::Bound<'_, #pyo3_path::types::PyModule>) -> #pyo3_path::PyResult<()> {
-                use #pyo3_path::prelude::PyModuleMethods;
-                use ::std::convert::Into;
-                module.add_function(#pyo3_path::types::PyCFunction::internal_new(module.py(), &DEF, module.into())?)
-            }
+            pub const _PYO3_DEF: #pyo3_path::impl_::pymethods::PyMethodDef = MakeDef::_PYO3_DEF;
         }
 
         // Generate the definition inside an anonymous function in the same scope as the original function -
@@ -283,7 +277,7 @@ pub fn impl_wrap_pyfunction(
         // (and `super` doesn't always refer to the outer scope, e.g. if the `#[pyfunction] is
         // inside a function body)
         impl #name::MakeDef {
-            const DEF: #pyo3_path::impl_::pymethods::PyMethodDef = #methoddef;
+            const _PYO3_DEF: #pyo3_path::impl_::pymethods::PyMethodDef = #methoddef;
         }
 
         #[allow(non_snake_case)]

src/impl_/pymodule.rs

@@ -1,6 +1,6 @@
 //! Implementation details of `#[pymodule]` which need to be accessible from proc-macro generated code.
 
-use std::cell::UnsafeCell;
+use std::{cell::UnsafeCell, marker::PhantomData};
 
 #[cfg(all(
     not(any(PyPy, GraalPy)),
@@ -11,7 +11,12 @@ use portable_atomic::{AtomicI64, Ordering};
 
 #[cfg(not(any(PyPy, GraalPy)))]
 use crate::exceptions::PyImportError;
-use crate::{ffi, sync::GILOnceCell, types::PyModule, Bound, Py, PyResult, Python};
+use crate::{
+    ffi,
+    sync::GILOnceCell,
+    types::{PyCFunction, PyModule, PyModuleMethods},
+    Bound, Py, PyClass, PyMethodDef, PyResult, PyTypeInfo, Python,
+};
 
 /// `Sync` wrapper of `ffi::PyModuleDef`.
 pub struct ModuleDef {
@@ -149,7 +154,53 @@ impl ModuleDef {
 ///
 /// Currently only implemented for classes.
 pub trait PyAddToModule {
-    fn add_to_module(module: &Bound<'_, PyModule>) -> PyResult<()>;
+    fn add_to_module(&'static self, module: &Bound<'_, PyModule>) -> PyResult<()>;
+}
+
+/// For adding native types (non-pyclass) to a module.
+pub struct AddTypeToModule<T>(PhantomData<T>);
+
+impl<T> AddTypeToModule<T> {
+    #[allow(clippy::new_without_default)]
+    pub const fn new() -> Self {
+        AddTypeToModule(PhantomData)
+    }
+}
+
+impl<T: PyTypeInfo> PyAddToModule for AddTypeToModule<T> {
+    fn add_to_module(&'static self, module: &Bound<'_, PyModule>) -> PyResult<()> {
+        module.add(T::NAME, T::type_object_bound(module.py()))
+    }
+}
+
+/// For adding a class to a module.
+pub struct AddClassToModule<T>(PhantomData<T>);
+
+impl<T> AddClassToModule<T> {
+    #[allow(clippy::new_without_default)]
+    pub const fn new() -> Self {
+        AddClassToModule(PhantomData)
+    }
+}
+
+impl<T: PyClass> PyAddToModule for AddClassToModule<T> {
+    fn add_to_module(&'static self, module: &Bound<'_, PyModule>) -> PyResult<()> {
+        module.add_class::<T>()
+    }
+}
+
+/// For adding a function to a module.
+impl PyAddToModule for PyMethodDef {
+    fn add_to_module(&'static self, module: &Bound<'_, PyModule>) -> PyResult<()> {
+        module.add_function(PyCFunction::internal_new(module.py(), self, Some(module))?)
+    }
+}
+
+/// For adding a module to a module.
+impl PyAddToModule for ModuleDef {
+    fn add_to_module(&'static self, module: &Bound<'_, PyModule>) -> PyResult<()> {
+        module.add_submodule(self.make_module(module.py())?.bind(module.py()))
+    }
 }
 
 #[cfg(test)]

src/macros.rs

@@ -138,7 +138,7 @@ macro_rules! wrap_pyfunction {
             use $function as wrapped_pyfunction;
             $crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction(
                 py_or_module,
-                &wrapped_pyfunction::DEF,
+                &wrapped_pyfunction::_PYO3_DEF,
             )
         }
     };
@@ -150,7 +150,7 @@ macro_rules! wrap_pyfunction {
         check_gil_refs.is_python();
         $crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction(
             py_or_module,
-            &wrapped_pyfunction::DEF,
+            &wrapped_pyfunction::_PYO3_DEF,
         )
     }};
 }
@@ -166,7 +166,7 @@ macro_rules! wrap_pyfunction_bound {
             use $function as wrapped_pyfunction;
             $crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction(
                 $crate::impl_::pyfunction::OnlyBound(py_or_module),
-                &wrapped_pyfunction::DEF,
+                &wrapped_pyfunction::_PYO3_DEF,
             )
         }
     };
@@ -174,7 +174,7 @@ macro_rules! wrap_pyfunction_bound {
         use $function as wrapped_pyfunction;
         $crate::impl_::pyfunction::WrapPyFunctionArg::wrap_pyfunction(
             $crate::impl_::pyfunction::OnlyBound($py_or_module),
-            &wrapped_pyfunction::DEF,
+            &wrapped_pyfunction::_PYO3_DEF,
         )
     }};
 }
@@ -189,7 +189,7 @@ macro_rules! wrap_pymodule {
     ($module:path) => {
         &|py| {
             use $module as wrapped_pymodule;
-            wrapped_pymodule::DEF
+            wrapped_pymodule::_PYO3_DEF
                 .make_module(py)
                 .expect("failed to wrap pymodule")
         }

src/types/mod.rs

@@ -250,16 +250,9 @@ macro_rules! pyobject_native_type_info(
             )?
         }
 
-        impl<$($generics,)*> $crate::impl_::pymodule::PyAddToModule for $name {
-            fn add_to_module(
-                module: &$crate::Bound<'_, $crate::types::PyModule>,
-            ) -> $crate::PyResult<()> {
-                use $crate::types::PyModuleMethods;
-                module.add(
-                    <Self as $crate::PyTypeInfo>::NAME,
-                    <Self as $crate::PyTypeInfo>::type_object_bound(module.py()),
-                )
-            }
+        impl $name {
+            #[doc(hidden)]
+            const _PYO3_DEF: $crate::impl_::pymodule::AddTypeToModule<Self> = $crate::impl_::pymodule::AddTypeToModule::new();
         }
     };
 );

tests/test_declarative_module.rs

@@ -145,7 +145,7 @@ mod class_initialization_module {
 #[cfg(not(Py_LIMITED_API))]
 fn test_class_initialization_fails() {
     Python::with_gil(|py| {
-        let err = class_initialization_module::DEF
+        let err = class_initialization_module::_PYO3_DEF
             .make_module(py)
             .unwrap_err();
         assert_eq!(