allow #[pymodule(...)] to accept all relevant #[pyo3(...)] options (#4330)

Author: davidhewitt

examples/getitem/src/lib.rs

@@ -75,8 +75,7 @@ impl ExampleContainer {
     }
 }
 
-#[pymodule]
-#[pyo3(name = "getitem")]
+#[pymodule(name = "getitem")]
 fn example(m: &Bound<'_, PyModule>) -> PyResult<()> {
     // ? -https://github.com/PyO3/maturin/issues/475
     m.add_class::<ExampleContainer>()?;

guide/src/module.md

@@ -31,8 +31,7 @@ fn double(x: usize) -> usize {
     x * 2
 }
 
-#[pymodule]
-#[pyo3(name = "custom_name")]
+#[pymodule(name = "custom_name")]
 fn my_extension(m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add_function(wrap_pyfunction!(double, m)?)
 }

newsfragments/4330.changed.md

@@ -0,0 +1 @@
+`#[pymodule(...)]` now directly accepts all relevant `#[pyo3(...)]` options.

pyo3-macros-backend/src/module.rs

@@ -2,8 +2,8 @@
 
 use crate::{
     attributes::{
-        self, take_attributes, take_pyo3_options, CrateAttribute, ModuleAttribute, NameAttribute,
-        SubmoduleAttribute,
+        self, kw, take_attributes, take_pyo3_options, CrateAttribute, ModuleAttribute,
+        NameAttribute, SubmoduleAttribute,
     },
     get_doc,
     pyclass::PyClassPyO3Option,
@@ -16,7 +16,7 @@ use std::ffi::CString;
 use syn::{
     ext::IdentExt,
     parse::{Parse, ParseStream},
-    parse_quote,
+    parse_quote, parse_quote_spanned,
     punctuated::Punctuated,
     spanned::Spanned,
     token::Comma,
@@ -26,105 +26,89 @@ use syn::{
 #[derive(Default)]
 pub struct PyModuleOptions {
     krate: Option<CrateAttribute>,
-    name: Option<syn::Ident>,
+    name: Option<NameAttribute>,
     module: Option<ModuleAttribute>,
-    is_submodule: bool,
+    submodule: Option<kw::submodule>,
 }
 
-impl PyModuleOptions {
-    pub fn from_attrs(attrs: &mut Vec<syn::Attribute>) -> Result<Self> {
+impl Parse for PyModuleOptions {
+    fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
         let mut options: PyModuleOptions = Default::default();
 
-        for option in take_pyo3_options(attrs)? {
-            match option {
-                PyModulePyO3Option::Name(name) => options.set_name(name.value.0)?,
-                PyModulePyO3Option::Crate(path) => options.set_crate(path)?,
-                PyModulePyO3Option::Module(module) => options.set_module(module)?,
-                PyModulePyO3Option::Submodule(submod) => options.set_submodule(submod)?,
-            }
-        }
+        options.add_attributes(
+            Punctuated::<PyModulePyO3Option, syn::Token![,]>::parse_terminated(input)?,
+        )?;
 
         Ok(options)
     }
+}
 
-    fn set_name(&mut self, name: syn::Ident) -> Result<()> {
-        ensure_spanned!(
-            self.name.is_none(),
-            name.span() => "`name` may only be specified once"
-        );
-
-        self.name = Some(name);
-        Ok(())
-    }
-
-    fn set_crate(&mut self, path: CrateAttribute) -> Result<()> {
-        ensure_spanned!(
-            self.krate.is_none(),
-            path.span() => "`crate` may only be specified once"
-        );
-
-        self.krate = Some(path);
-        Ok(())
-    }
-
-    fn set_module(&mut self, name: ModuleAttribute) -> Result<()> {
-        ensure_spanned!(
-            self.module.is_none(),
-            name.span() => "`module` may only be specified once"
-        );
-
-        self.module = Some(name);
-        Ok(())
+impl PyModuleOptions {
+    fn take_pyo3_options(&mut self, attrs: &mut Vec<syn::Attribute>) -> Result<()> {
+        self.add_attributes(take_pyo3_options(attrs)?)
     }
 
-    fn set_submodule(&mut self, submod: SubmoduleAttribute) -> Result<()> {
-        ensure_spanned!(
-            !self.is_submodule,
-            submod.span() => "`submodule` may only be specified once (it is implicitly always specified for nested modules)"
-        );
-
-        self.is_submodule = true;
+    fn add_attributes(
+        &mut self,
+        attrs: impl IntoIterator<Item = PyModulePyO3Option>,
+    ) -> Result<()> {
+        macro_rules! set_option {
+            ($key:ident $(, $extra:literal)?) => {
+                {
+                    ensure_spanned!(
+                        self.$key.is_none(),
+                        $key.span() => concat!("`", stringify!($key), "` may only be specified once" $(, $extra)?)
+                    );
+                    self.$key = Some($key);
+                }
+            };
+        }
+        for attr in attrs {
+            match attr {
+                PyModulePyO3Option::Crate(krate) => set_option!(krate),
+                PyModulePyO3Option::Name(name) => set_option!(name),
+                PyModulePyO3Option::Module(module) => set_option!(module),
+                PyModulePyO3Option::Submodule(submodule) => set_option!(
+                    submodule,
+                    " (it is implicitly always specified for nested modules)"
+                ),
+            }
+        }
         Ok(())
     }
 }
 
 pub fn pymodule_module_impl(
-    mut module: syn::ItemMod,
-    mut is_submodule: bool,
+    module: &mut syn::ItemMod,
+    mut options: PyModuleOptions,
 ) -> Result<TokenStream> {
     let syn::ItemMod {
         attrs,
         vis,
         unsafety: _,
         ident,
-        mod_token: _,
+        mod_token,
         content,
         semi: _,
-    } = &mut module;
+    } = module;
     let items = if let Some((_, items)) = content {
         items
     } else {
-        bail_spanned!(module.span() => "`#[pymodule]` can only be used on inline modules")
+        bail_spanned!(mod_token.span() => "`#[pymodule]` can only be used on inline modules")
     };
-    let options = PyModuleOptions::from_attrs(attrs)?;
+    options.take_pyo3_options(attrs)?;
     let ctx = &Ctx::new(&options.krate, None);
     let Ctx { pyo3_path, .. } = ctx;
     let doc = get_doc(attrs, None, ctx);
-    let name = options.name.unwrap_or_else(|| ident.unraw());
+    let name = options
+        .name
+        .map_or_else(|| ident.unraw(), |name| name.value.0);
     let full_name = if let Some(module) = &options.module {
         format!("{}.{}", module.value.value(), name)
     } else {
         name.to_string()
     };
 
-    is_submodule = match (is_submodule, options.is_submodule) {
-        (true, true) => {
-            bail_spanned!(module.span() => "`submodule` may only be specified once (it is implicitly always specified for nested modules)")
-        }
-        (false, false) => false,
-        (true, false) | (false, true) => true,
-    };
-
     let mut module_items = Vec::new();
     let mut module_items_cfg_attrs = Vec::new();
 
@@ -280,7 +264,9 @@ pub fn pymodule_module_impl(
                     )? {
                         set_module_attribute(&mut item_mod.attrs, &full_name);
                     }
-                    item_mod.attrs.push(parse_quote!(#[pyo3(submodule)]));
+                    item_mod
+                        .attrs
+                        .push(parse_quote_spanned!(item_mod.mod_token.span()=> #[pyo3(submodule)]));
                 }
             }
             Item::ForeignMod(item) => {
@@ -358,10 +344,11 @@ pub fn pymodule_module_impl(
             )
         }
     }};
-    let initialization = module_initialization(&name, ctx, module_def, is_submodule);
+    let initialization = module_initialization(&name, ctx, module_def, options.submodule.is_some());
+
     Ok(quote!(
         #(#attrs)*
-        #vis mod #ident {
+        #vis #mod_token #ident {
             #(#items)*
 
             #initialization
@@ -381,13 +368,18 @@ pub fn pymodule_module_impl(
 
 /// Generates the function that is called by the python interpreter to initialize the native
 /// module
-pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result<TokenStream> {
-    let options = PyModuleOptions::from_attrs(&mut function.attrs)?;
-    process_functions_in_module(&options, &mut function)?;
+pub fn pymodule_function_impl(
+    function: &mut syn::ItemFn,
+    mut options: PyModuleOptions,
+) -> Result<TokenStream> {
+    options.take_pyo3_options(&mut function.attrs)?;
+    process_functions_in_module(&options, function)?;
     let ctx = &Ctx::new(&options.krate, None);
     let Ctx { pyo3_path, .. } = ctx;
     let ident = &function.sig.ident;
-    let name = options.name.unwrap_or_else(|| ident.unraw());
+    let name = options
+        .name
+        .map_or_else(|| ident.unraw(), |name| name.value.0);
     let vis = &function.vis;
     let doc = get_doc(&function.attrs, None, ctx);
 
@@ -402,7 +394,6 @@ pub fn pymodule_function_impl(mut function: syn::ItemFn) -> Result<TokenStream>
         .push(quote!(::std::convert::Into::into(#pyo3_path::impl_::pymethods::BoundRef(module))));
 
     Ok(quote! {
-        #function
         #[doc(hidden)]
         #vis mod #ident {
             #initialization

pyo3-macros/src/lib.rs

@@ -3,14 +3,14 @@
 
 #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
 use proc_macro::TokenStream;
-use proc_macro2::{Span, TokenStream as TokenStream2};
+use proc_macro2::TokenStream as TokenStream2;
 use pyo3_macros_backend::{
     build_derive_from_pyobject, build_py_class, build_py_enum, build_py_function, build_py_methods,
     pymodule_function_impl, pymodule_module_impl, PyClassArgs, PyClassMethodsType,
-    PyFunctionOptions,
+    PyFunctionOptions, PyModuleOptions,
 };
 use quote::quote;
-use syn::{parse::Nothing, parse_macro_input, Item};
+use syn::{parse_macro_input, Item};
 
 /// A proc macro used to implement Python modules.
 ///
@@ -24,6 +24,9 @@ use syn::{parse::Nothing, parse_macro_input, Item};
 /// |  Annotation  |  Description |
 /// | :-  | :- |
 /// | `#[pyo3(name = "...")]` | Defines the name of the module in Python. |
+/// | `#[pyo3(submodule)]`    | Skips adding a `PyInit_` FFI symbol to the compiled binary. |
+/// | `#[pyo3(module = "...")]` | Defines the Python `dotted.path` to the parent module for use in introspection. |
+/// | `#[pyo3(crate = "pyo3")]` | Defines the path to PyO3 to use code generated by the macro. |
 ///
 /// For more on creating Python modules see the [module section of the guide][1].
 ///
@@ -35,32 +38,29 @@ use syn::{parse::Nothing, parse_macro_input, Item};
 #[doc = concat!("[1]: https://pyo3.rs/v", env!("CARGO_PKG_VERSION"), "/module.html")]
 #[proc_macro_attribute]
 pub fn pymodule(args: TokenStream, input: TokenStream) -> TokenStream {
-    match parse_macro_input!(input as Item) {
+    let options = parse_macro_input!(args as PyModuleOptions);
+
+    let mut ast = parse_macro_input!(input as Item);
+    let expanded = match &mut ast {
         Item::Mod(module) => {
-            let is_submodule = match parse_macro_input!(args as Option<syn::Ident>) {
-                Some(i) if i == "submodule" => true,
-                Some(_) => {
-                    return syn::Error::new(
-                        Span::call_site(),
-                        "#[pymodule] only accepts submodule as an argument",
-                    )
-                    .into_compile_error()
-                    .into();
-                }
-                None => false,
-            };
-            pymodule_module_impl(module, is_submodule)
-        }
-        Item::Fn(function) => {
-            parse_macro_input!(args as Nothing);
-            pymodule_function_impl(function)
+            match pymodule_module_impl(module, options) {
+                // #[pymodule] on a module will rebuild the original ast, so we don't emit it here
+                Ok(expanded) => return expanded.into(),
+                Err(e) => Err(e),
+            }
         }
+        Item::Fn(function) => pymodule_function_impl(function, options),
         unsupported => Err(syn::Error::new_spanned(
             unsupported,
             "#[pymodule] only supports modules and functions.",
         )),
     }
-    .unwrap_or_compile_error()
+    .unwrap_or_compile_error();
+
+    quote!(
+        #ast
+        #expanded
+    )
     .into()
 }
 

tests/test_declarative_module.rs

@@ -49,8 +49,7 @@ create_exception!(
     "Some description."
 );
 
-#[pymodule]
-#[pyo3(submodule)]
+#[pymodule(submodule)]
 mod external_submodule {}
 
 /// A module written using declarative syntax.
@@ -144,8 +143,7 @@ mod declarative_submodule {
     use super::{double, double_value};
 }
 
-#[pymodule]
-#[pyo3(name = "declarative_module_renamed")]
+#[pymodule(name = "declarative_module_renamed")]
 mod declarative_module2 {
     #[pymodule_export]
     use super::double;

tests/test_module.rs

@@ -138,8 +138,7 @@ fn test_module_with_explicit_py_arg() {
     });
 }
 
-#[pymodule]
-#[pyo3(name = "other_name")]
+#[pymodule(name = "other_name")]
 fn some_name(m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add("other_name", "other_name")?;
     Ok(())

tests/ui/duplicate_pymodule_submodule.stderr

@@ -4,8 +4,14 @@ error: `submodule` may only be specified once (it is implicitly always specified
 4 |     mod submod {}
   |     ^^^
 
-error[E0433]: failed to resolve: use of undeclared crate or module `submod`
- --> tests/ui/duplicate_pymodule_submodule.rs:4:6
+error[E0425]: cannot find value `_PYO3_DEF` in module `submod`
+ --> tests/ui/duplicate_pymodule_submodule.rs:1:1
+  |
+1 | #[pyo3::pymodule]
+  | ^^^^^^^^^^^^^^^^^ not found in `submod`
+  |
+  = note: this error originates in the attribute macro `pyo3::pymodule` (in Nightly builds, run with -Z macro-backtrace for more info)
+help: consider importing this static
+  |
+3 +     use crate::mymodule::_PYO3_DEF;
   |
-4 |     mod submod {}
-  |         ^^^^^^ use of undeclared crate or module `submod`

tests/ui/empty.rs

@@ -0,0 +1 @@
+// see invalid_pymodule_in_root.rs

tests/ui/invalid_pymodule_args.stderr

@@ -1,4 +1,4 @@
-error: unexpected token
+error: expected one of: `name`, `crate`, `module`, `submodule`
  --> tests/ui/invalid_pymodule_args.rs:3:12
   |
 3 | #[pymodule(some_arg)]

tests/ui/invalid_pymodule_glob.rs

@@ -1,3 +1,5 @@
+#![allow(unused_imports)]
+
 use pyo3::prelude::*;
 
 #[pyfunction]

tests/ui/invalid_pymodule_glob.stderr

@@ -1,5 +1,5 @@
 error: #[pymodule] cannot import glob statements
-  --> tests/ui/invalid_pymodule_glob.rs:11:16
+  --> tests/ui/invalid_pymodule_glob.rs:13:16
    |
-11 |     use super::*;
+13 |     use super::*;
    |                ^