fixes #4285 -- allow full-path to pymodule with nested declarative modules #4288
Conversation
There was a problem hiding this comment.
Thanks, this is definitely worth solving.
I'd prefer a more complete solution. I think also this needs similar treatment for #[pyclass] and #[pyfunction]?
I wonder whether a more thorough alternative solution would be to immediately parse and expand inner #[pyclass] / #[pyfunction] / #[pymodule] macros. I don't know if there's any implications of expansion ordering related to that.
Actually now that I say this - I guess declarative modules will fail to see a structure like this already:
#[pymodule]
mod foo {
macro_rules! class {
($name:ident) {
#[pyclass]
struct $name { }
}
}
class!(Foo);
class!(Bar);
}
pyo3-macros-backend/src/module.rs
Outdated
| item.span() => "`#[pymodule_export]` may only be used on `use` statements" | ||
| ); | ||
| if has_attribute(&item_mod.attrs, "pymodule") { | ||
| if has_attribute(&item_mod.attrs, &["pymodule"]) | ||
| || has_attribute(&item_mod.attrs, &["pyo3", "prelude", "pymodule"]) |
There was a problem hiding this comment.
We should probably use ctx.pyo3_path instead of pyo3 hard-coded here.
pyo3::pymodule is also a valid path to the macro, so we should allow that.
There was a problem hiding this comment.
Hmm, moved too fast, it's not clear what the best way to get an &str out of the syn::Path is. This may need more involved surgery.
|
Hmm, how would expanding the macros eagerly work? Handling the variety of different forms a user can write this in is... expansive. (A user can |
alex
force-pushed
the
full-path-pymodule
branch
3 times, most recently
from
e81419b to
0676a10
Compare
|
Are there other pieces you'd like here besides expanding to pyfunction/pyclass? |
|
I think that's just it, I see you solved the The eager expansion / unlimited forms of |
|
Will add support for those after work today. Thanks for the review
…On Tue, Jul 2, 2024, 9:15 AM David Hewitt ***@***.***> wrote:
I think that's just it, I see you solved the Path complication above 👍
The eager expansion / unlimited forms of use imports can be problems for
the future.
—
Reply to this email directly, view it on GitHub
<#4288 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAGBGKNVKA5AIDT53ZPC3ZKKRWPAVCNFSM6AAAAABJ5BGK2CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBTGEZDSMZWGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Ok, added! |
fixes #4285