deprecate &PyModule as #[pymodule] argument type#3936
Conversation
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Ah, nice solution! I think that's a workable approach.
A compile error test would be sufficient. Maybe add to tests/ui/deprecations.rs?
pyo3-macros-backend/src/module.rs
Outdated
| #(#attrs)* | ||
| #vis #sig { | ||
| #(#extractors)* | ||
| #block | ||
| } |
There was a problem hiding this comment.
Rather than splitting up the function it might be possible to insert these into the front of function.block.stmts.
pyo3-macros-backend/src/module.rs
Outdated
| if let syn::Pat::Ident(pat_ident) = &*pat_type.pat { | ||
| let ident = &pat_ident.ident; | ||
| return Some(quote_spanned! { pat_type.span() => { | ||
| let (_, e) = #pyo3_path::impl_::pymethods::inspect_type(#ident); |
There was a problem hiding this comment.
Might need to assign the argument back out:
| let (_, e) = #pyo3_path::impl_::pymethods::inspect_type(#ident); | |
| let (#ident, e) = #pyo3_path::impl_::pymethods::inspect_type(#ident); |
(I think this would matter for edge cases where the module is using Bound<'_, PyModule> or Py<PyModule> for some reason, and inspect_type would transfer ownership rather than copy.)
|
I think this is ready now, and it actually found a few placed that we haven't updated yet, including two doc tests 🎉 |
Icxolu
force-pushed
the
deprecate-pymodule-gil-ref
branch
from
b40e530 to
a85157e
Compare
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Great, many thanks!
I'm sure there are quite a few more GIL Refs sneaking around various edges of the codebase... Hopefully we can eliminate a few more from the docs before 0.21 final. The road to 0.22 is littered with removing most of the rest of them PyO3's test suite, I think 😂
2 participants
This uses the framework of #3847 to deprecate
&PyModuleas#[pymodule]argument type.This is done by modifying the original module function on macro expansion and injection of the gil-ref extractor with deref specialization. This can (and should) be reverted once the gil-refs are fully removed.
I assume this will need some tests, I will add those if we want go with this