Use PyModExport and PyABIInfo APIs in pymodule implementation #5753
Conversation
src/impl_/pymodule.rs
Outdated
| #[cfg(not(Py_3_15))] | ||
| { | ||
| unsafe { *self.ffi_def.get() }.m_slots | ||
| } |
There was a problem hiding this comment.
I thought it was easier to read if I repeat myself a bit in the Py_3_15 and not(Py_3_15) blocks. Let me know if anyone would prefer to see me collapse this a bit.
| pub const fn with_name(self, name: &'static CStr) -> Self { | ||
| #[cfg(Py_3_15)] | ||
| { | ||
| self.push(ffi::Py_mod_name, name.as_ptr() as *mut c_void) |
There was a problem hiding this comment.
Does that static lifetime on the name mean this is safe? Or does this function need to be unsafe?
There was a problem hiding this comment.
I think it's safe. The name should either be leaked (and there will be an unsafe at this stage) or stored as a constant in the binary.
| pub const fn with_abi_info(self) -> Self { | ||
| #[cfg(Py_3_15)] | ||
| { | ||
| ffi::PyABIInfo_VAR!(ABI_INFO); |
There was a problem hiding this comment.
is it OK to define ABI_INFO inline in a macro like this?
|
|
||
| #[test] | ||
| #[should_panic] | ||
| fn test_module_slots_builder_overflow() { |
There was a problem hiding this comment.
I deleted this test because I got rid of the const generic parameter.
ngoldbaum
added
the
CI-no-fail-fast
Merging this PR will degrade performance by 27.24%
Performance Changes
Comparing 
|
|
Tests are passing now but I don't understand why the benchmark is failing or why the code coverage test is failing. Are the 3.15 test runners not uploading coverage reports, perhaps? I'm going to add a changelog entry and mark this ready for review. |
|
@bschoenmaeckers do you happen to have any context about why my changes to type initialization are impacting the benchmark you added last week? |
It has been jumping up and down since rust 1.93. I'm not sure what is going on. So it it safe to ignore for now |
3 participants
Fixes #5644
The first two commits fix some errors from #5746 that were missed in review.
The bulk of the changes are in the third commit, which updates the pymodule implementation to use the new PEP 793 APIs.
After that there are some commits for issues I encountered in CI.
Implementation notes
My original goal was to avoid defining
PyModuleDefobjects directly on the 3.15 and newer. It turns out the inittab tests need the legacy init hook, so I still need to unconditionally define aPyModuleDef. When we add support for a opaque PyObject ABI, we'll need to disableappend_to_inittabon those builds.I also had to work around a limitation of using both the PyModuleDef and slots, see this discourse post for more context. I'm asking in the discourse post whether it makes sense to relax the check in CPython that I'm working around by defining both
SLOTSandSLOTS_MINIMALin the pymodule macro implementation.With the new APIs in 3.15, I'm then able to set up modules and submodules without any reference to a module object.
I replaced the const generic parameter on
PyModuleSlotsandPyModuleSlotsBuilderbecause we statically know how many slots PyO3 knows how to define, so it's not necessary for users of the API to keep track. See the logic defining theMAX_SLOTSvariable and how it's used in the implementation.