Skip to content

Conversation

@encukou
Copy link
Member

@encukou encukou commented Jan 30, 2026

From feedback on the discussion thread:

When integrating slots-based module creation is with the inittab, which currently requires PyModuleDef, it would be convenient to reuse the the same slots array for the MethodDef.

This PR allows slots that match what's already present in the PyModuleDef.

cc @ngoldbaum


📚 Documentation preview 📚: https://cpython-previews--144340.org.readthedocs.build/

When integrating slots-based module creation is with the inittab,
which currently requires PyModuleDef, it would be convenient to
reuse the the same slots array for the MethodDef.

Allow slots that match what's already present in the PyModuleDef.
@ngoldbaum
Copy link
Contributor

ngoldbaum commented Jan 30, 2026

I can confirm that with this PR the PyO3 tests all pass on my for-petr branch, which uses the same slots array for both initialization hooks and unconditionally sets m_name and m_doc in PyModuleDef. It's fine for PyO3's purposes to have duplicate information with this check to make sure that the duplicate information matches exactly.

the module name must be given as :c:member:`PyModuleDef.m_name`.
If the array contains slots corresponding to :c:type:`PyModuleDef`
members, the values must match.
For example, if you use :c:macro:`Py_mod_name` in :c:member:`!m_slots`;
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
For example, if you use :c:macro:`Py_mod_name` in :c:member:`!m_slots`;
For example, if you use :c:macro:`Py_mod_name` in :c:member:`!m_slots`,

#undef DEF_SLOT_CASE
#undef COPY_DEF_SLOT
#undef COPY_NONDEF_SLOT
#undef _COPY_COMMON_SLOT
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
#undef _COPY_COMMON_SLOT
#undef COPY_NONNULL_SLOT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants