Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically treat nested modules as submodules #4308

Merged
merged 1 commit into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions guide/src/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ For nested modules, the name of the parent module is automatically added.
In the following example, the `Unit` class will have for `module` `my_extension.submodule` because it is properly nested
but the `Ext` class will have for `module` the default `builtins` because it not nested.

You can provide the `submodule` argument to `pymodule()` for modules that are not top-level modules.
```rust
# mod declarative_module_module_attr_test {
use pyo3::prelude::*;
Expand All @@ -170,7 +169,7 @@ mod my_extension {
#[pymodule_export]
use super::Ext;

#[pymodule(submodule)]
#[pymodule]
mod submodule {
use super::*;
// This is a submodule
Expand All @@ -183,3 +182,4 @@ mod my_extension {
```
It is possible to customize the `module` value for a `#[pymodule]` with the `#[pyo3(module = "MY_MODULE")]` option.

You can provide the `submodule` argument to `pymodule()` for modules that are not top-level modules -- it is automatically set for modules nested inside of a `#[pymodule]`.
1 change: 1 addition & 0 deletions newsfragments/4308.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Nested declarative `#[pymodule]` are automatically treated as submodules (no `PyInit_` entrypoint is created)
12 changes: 10 additions & 2 deletions pyo3-macros-backend/src/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl PyModuleOptions {
fn set_submodule(&mut self, submod: SubmoduleAttribute) -> Result<()> {
ensure_spanned!(
!self.is_submodule,
submod.span() => "`submodule` may only be specified once"
submod.span() => "`submodule` may only be specified once (it is implicitly always specified for nested modules)"
);

self.is_submodule = true;
Expand Down Expand Up @@ -116,7 +116,14 @@ pub fn pymodule_module_impl(
} else {
name.to_string()
};
is_submodule = is_submodule || options.is_submodule;

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();
Expand Down Expand Up @@ -273,6 +280,7 @@ pub fn pymodule_module_impl(
)? {
set_module_attribute(&mut item_mod.attrs, &full_name);
}
item_mod.attrs.push(parse_quote!(#[pyo3(submodule)]));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs some additional tests.

If submodule is already present, will this error? Probably it will error for duplicate submodule setting already, the error message should ideally read "submodules of declarative modules don't need submodule attribute set".

We might want to allow for submodule = false or maybe #[pymodule(importable)] to allow turning this off. Maybe? (TBH I can't see a use case where users would want to nest declarative modules and then allow both to be importable as root modules too.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I cannot think of a valid use case for that scenario. If one does want it, it can be accomplished by declrating two top-level modules and exporting one into the other.

}
}
Item::ForeignMod(item) => {
Expand Down
1 change: 1 addition & 0 deletions tests/test_compile_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,5 @@ fn test_compile_errors() {
t.compile_fail("tests/ui/abi3_weakref.rs");
#[cfg(all(Py_LIMITED_API, not(Py_3_9)))]
t.compile_fail("tests/ui/abi3_dict.rs");
t.compile_fail("tests/ui/duplicate_pymodule_submodule.rs");
}
2 changes: 1 addition & 1 deletion tests/test_declarative_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ mod declarative_module {
}
}

#[pymodule(submodule)]
#[pymodule]
#[pyo3(module = "custom_root")]
mod inner_custom_root {
use super::*;
Expand Down
7 changes: 7 additions & 0 deletions tests/ui/duplicate_pymodule_submodule.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#[pyo3::pymodule]
mod mymodule {
#[pyo3::pymodule(submodule)]
mod submod {}
}

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/duplicate_pymodule_submodule.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: `submodule` may only be specified once (it is implicitly always specified for nested modules)
--> tests/ui/duplicate_pymodule_submodule.rs:4:2
|
4 | mod submod {}
| ^^^

error[E0433]: failed to resolve: use of undeclared crate or module `submod`
--> tests/ui/duplicate_pymodule_submodule.rs:4:6
|
4 | mod submod {}
| ^^^^^^ use of undeclared crate or module `submod`
Loading