Skip to content

Commit 48ab83f

Browse files
committed
gh-140550: allow slots that repeat information from PyModuleDef
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.
1 parent 5f57f69 commit 48ab83f

File tree

5 files changed

+134
-48
lines changed

5 files changed

+134
-48
lines changed

Doc/c-api/module.rst

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -725,10 +725,11 @@ remove it.
725725
726726
An array of additional slots, terminated by a ``{0, NULL}`` entry.
727727
728-
This array may not contain slots corresponding to :c:type:`PyModuleDef`
729-
members.
730-
For example, you cannot use :c:macro:`Py_mod_name` in :c:member:`!m_slots`;
731-
the module name must be given as :c:member:`PyModuleDef.m_name`.
728+
If the array contains slots corresponding to :c:type:`PyModuleDef`
729+
members, the values must match.
730+
For example, if you use :c:macro:`Py_mod_name` in :c:member:`!m_slots`;
731+
:c:member:`PyModuleDef.m_name` must be set to the same pointer
732+
(not just an equal string).
732733
733734
.. versionchanged:: 3.5
734735

Lib/test/test_capi/test_module.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,7 @@ def test_create(self):
122122
_testcapi.pymodule_get_token(mod)
123123

124124
def test_def_slot(self):
125-
"""Slots that replace PyModuleDef fields can't be used with PyModuleDef
126-
"""
125+
"""Slots cannot contradict PyModuleDef fields"""
127126
for name in DEF_SLOTS:
128127
with self.subTest(name):
129128
spec = FakeSpec()
@@ -133,6 +132,11 @@ def test_def_slot(self):
133132
self.assertIn(name, str(cm.exception))
134133
self.assertIn("PyModuleDef", str(cm.exception))
135134

135+
def test_def_slot_parrot(self):
136+
"""Slots with same value as PyModuleDef fields are allowed"""
137+
spec = FakeSpec()
138+
_testcapi.module_from_def_slot_parrot(spec)
139+
136140
def test_repeated_def_slot(self):
137141
"""Slots that replace PyModuleDef fields can't be repeated"""
138142
for name in (*DEF_SLOTS, 'Py_mod_exec'):
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
In :c:member:`PyModuleDef.m_slots`, allow slots that repeat information
2+
present in :c:type:`PyModuleDef`.

Modules/_testcapi/module.c

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "parts.h"
22
#include "util.h"
3+
#include <stdbool.h>
34

45
// Test PyModule_* API
56

@@ -270,6 +271,47 @@ module_from_def_slot(PyObject *self, PyObject *spec)
270271
return result;
271272
}
272273

274+
static PyModuleDef parrot_def = {
275+
PyModuleDef_HEAD_INIT,
276+
.m_name = "test_capi/parrot",
277+
.m_doc = "created from redundant information",
278+
.m_size = 123,
279+
.m_methods = a_methoddef_array,
280+
.m_traverse = noop_traverse,
281+
.m_clear = noop_clear,
282+
.m_free = (void*)noop_free,
283+
.m_slots = NULL /* set below */,
284+
};
285+
static PyModuleDef_Slot parrot_slots[] = {
286+
{Py_mod_name, "test_capi/parrot"},
287+
{Py_mod_doc, "created from redundant information"},
288+
{Py_mod_state_size, (void*)123},
289+
{Py_mod_methods, a_methoddef_array},
290+
{Py_mod_state_traverse, noop_traverse},
291+
{Py_mod_state_clear, noop_clear},
292+
{Py_mod_state_free, (void*)noop_free},
293+
{Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
294+
{Py_mod_token, &parrot_def},
295+
{Py_mod_gil, Py_MOD_GIL_NOT_USED},
296+
{0},
297+
};
298+
299+
static PyObject *
300+
module_from_def_slot_parrot(PyObject *self, PyObject *spec)
301+
{
302+
parrot_def.m_slots = parrot_slots;
303+
PyObject *module = PyModule_FromDefAndSpec(&parrot_def, spec);
304+
if (!module || (PyModule_Exec(module) < 0)) {
305+
return NULL;
306+
}
307+
Py_ssize_t size;
308+
assert(PyModule_GetStateSize(module, &size) == 0);
309+
assert(size == 123);
310+
PyModuleDef *def = PyModule_GetDef(module);
311+
assert(def == &parrot_def);
312+
return module;
313+
}
314+
273315
static int
274316
another_exec(PyObject *module)
275317
{
@@ -368,6 +410,7 @@ static PyMethodDef test_methods[] = {
368410
{"module_from_slots_null_slot", module_from_slots_null_slot, METH_O},
369411
{"module_from_def_multiple_exec", module_from_def_multiple_exec, METH_O},
370412
{"module_from_def_slot", module_from_def_slot, METH_O},
413+
{"module_from_def_slot_parrot", module_from_def_slot_parrot, METH_O},
371414
{"pymodule_get_token", pymodule_get_token, METH_O},
372415
{"pymodule_get_def", pymodule_get_def, METH_O},
373416
{"pymodule_get_state_size", pymodule_get_state_size, METH_O},

Objects/moduleobject.c

Lines changed: 78 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -410,35 +410,78 @@ module_from_def_and_spec(
410410
goto error;
411411
}
412412

413+
bool seen_m_name_slot = false;
414+
bool seen_m_doc_slot = false;
415+
bool seen_m_size_slot = false;
416+
bool seen_m_methods_slot = false;
417+
bool seen_m_traverse_slot = false;
418+
bool seen_m_clear_slot = false;
419+
bool seen_m_free_slot = false;
413420
for (cur_slot = def_like->m_slots; cur_slot && cur_slot->slot; cur_slot++) {
414-
// Macro to copy a non-NULL, non-repeatable slot that's unusable with
415-
// PyModuleDef. The destination must be initially NULL.
416-
#define COPY_COMMON_SLOT(SLOT, TYPE, DEST) \
421+
422+
// Macro to copy a non-NULL, non-repeatable slot.
423+
#define COPY_NONNULL_SLOT(SLOTNAME, TYPE, DEST) \
417424
do { \
418425
if (!(TYPE)(cur_slot->value)) { \
419426
PyErr_Format( \
420427
PyExc_SystemError, \
421-
"module %s: " #SLOT " must not be NULL", \
422-
name); \
428+
"module %s: %s must not be NULL", \
429+
name, SLOTNAME); \
423430
goto error; \
424431
} \
425-
if (original_def) { \
432+
DEST = (TYPE)(cur_slot->value); \
433+
} while (0); \
434+
/////////////////////////////////////////////////////////////////
435+
436+
// Macro to copy a non-NULL, non-repeatable slot to def_like.
437+
#define COPY_DEF_SLOT(SLOTNAME, TYPE, MEMBER) \
438+
do { \
439+
if (seen_ ## MEMBER ## _slot) { \
426440
PyErr_Format( \
427441
PyExc_SystemError, \
428-
"module %s: " #SLOT " used with PyModuleDef", \
429-
name); \
442+
"module %s has more than one %s slot", \
443+
name, SLOTNAME); \
430444
goto error; \
431445
} \
446+
seen_ ## MEMBER ## _slot = true; \
447+
if (original_def) { \
448+
TYPE orig_value = (TYPE)original_def->MEMBER; \
449+
TYPE new_value = (TYPE)cur_slot->value; \
450+
if (orig_value != new_value) { \
451+
PyErr_Format( \
452+
PyExc_SystemError, \
453+
"module %s: %s conflicts with " \
454+
"PyModuleDef." #MEMBER, \
455+
name, SLOTNAME); \
456+
goto error; \
457+
} \
458+
} \
459+
COPY_NONNULL_SLOT(SLOTNAME, TYPE, (def_like->MEMBER)) \
460+
} while (0); \
461+
/////////////////////////////////////////////////////////////////
462+
463+
// Macro to copy a non-NULL, non-repeatable slot without a
464+
// corresponding PyModuleDef member.
465+
// DEST must be initially NULL (so we don't need a seen_* flag).
466+
#define COPY_NONDEF_SLOT(SLOTNAME, TYPE, DEST) \
467+
do { \
432468
if (DEST) { \
433469
PyErr_Format( \
434470
PyExc_SystemError, \
435-
"module %s has more than one " #SLOT " slot", \
436-
name); \
471+
"module %s has more than one %s slot", \
472+
name, SLOTNAME); \
437473
goto error; \
438474
} \
439-
DEST = (TYPE)(cur_slot->value); \
475+
COPY_NONNULL_SLOT(SLOTNAME, TYPE, DEST) \
440476
} while (0); \
441477
/////////////////////////////////////////////////////////////////
478+
479+
// Define the whole common case
480+
#define DEF_SLOT_CASE(SLOT, TYPE, MEMBER) \
481+
case SLOT: \
482+
COPY_DEF_SLOT(#SLOT, TYPE, MEMBER); \
483+
break; \
484+
/////////////////////////////////////////////////////////////////
442485
switch (cur_slot->slot) {
443486
case Py_mod_create:
444487
if (create) {
@@ -453,14 +496,15 @@ module_from_def_and_spec(
453496
case Py_mod_exec:
454497
has_execution_slots = 1;
455498
if (!original_def) {
456-
COPY_COMMON_SLOT(Py_mod_exec, _Py_modexecfunc, m_exec);
499+
COPY_NONDEF_SLOT("Py_mod_exec", _Py_modexecfunc, m_exec);
457500
}
458501
break;
459502
case Py_mod_multiple_interpreters:
460503
if (has_multiple_interpreters_slot) {
461504
PyErr_Format(
462505
PyExc_SystemError,
463-
"module %s has more than one 'multiple interpreters' slots",
506+
"module %s has more than one 'multiple interpreters' "
507+
"slots",
464508
name);
465509
goto error;
466510
}
@@ -483,34 +527,23 @@ module_from_def_and_spec(
483527
goto error;
484528
}
485529
break;
486-
case Py_mod_name:
487-
COPY_COMMON_SLOT(Py_mod_name, char*, def_like->m_name);
488-
break;
489-
case Py_mod_doc:
490-
COPY_COMMON_SLOT(Py_mod_doc, char*, def_like->m_doc);
491-
break;
492-
case Py_mod_state_size:
493-
COPY_COMMON_SLOT(Py_mod_state_size, Py_ssize_t,
494-
def_like->m_size);
495-
break;
496-
case Py_mod_methods:
497-
COPY_COMMON_SLOT(Py_mod_methods, PyMethodDef*,
498-
def_like->m_methods);
499-
break;
500-
case Py_mod_state_traverse:
501-
COPY_COMMON_SLOT(Py_mod_state_traverse, traverseproc,
502-
def_like->m_traverse);
503-
break;
504-
case Py_mod_state_clear:
505-
COPY_COMMON_SLOT(Py_mod_state_clear, inquiry,
506-
def_like->m_clear);
507-
break;
508-
case Py_mod_state_free:
509-
COPY_COMMON_SLOT(Py_mod_state_free, freefunc,
510-
def_like->m_free);
511-
break;
530+
DEF_SLOT_CASE(Py_mod_name, char*, m_name)
531+
DEF_SLOT_CASE(Py_mod_doc, char*, m_doc)
532+
DEF_SLOT_CASE(Py_mod_state_size, Py_ssize_t, m_size)
533+
DEF_SLOT_CASE(Py_mod_methods, PyMethodDef*, m_methods)
534+
DEF_SLOT_CASE(Py_mod_state_traverse, traverseproc, m_traverse)
535+
DEF_SLOT_CASE(Py_mod_state_clear, inquiry, m_clear)
536+
DEF_SLOT_CASE(Py_mod_state_free, freefunc, m_free)
512537
case Py_mod_token:
513-
COPY_COMMON_SLOT(Py_mod_token, void*, token);
538+
if (original_def && original_def != cur_slot->value) {
539+
PyErr_Format(
540+
PyExc_SystemError,
541+
"module %s: arbitrary Py_mod_token not "
542+
"allowed with PyModuleDef",
543+
name);
544+
goto error;
545+
}
546+
COPY_NONDEF_SLOT("Py_mod_token", void*, token);
514547
break;
515548
default:
516549
assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT);
@@ -520,7 +553,10 @@ module_from_def_and_spec(
520553
name, cur_slot->slot);
521554
goto error;
522555
}
523-
#undef COPY_COMMON_SLOT
556+
#undef DEF_SLOT_CASE
557+
#undef COPY_DEF_SLOT
558+
#undef COPY_NONDEF_SLOT
559+
#undef _COPY_COMMON_SLOT
524560
}
525561

526562
#ifdef Py_GIL_DISABLED
@@ -590,7 +626,7 @@ module_from_def_and_spec(
590626
mod->md_state = NULL;
591627
module_copy_members_from_deflike(mod, def_like);
592628
if (original_def) {
593-
assert (!token);
629+
assert (!token || token == original_def);
594630
mod->md_token = original_def;
595631
mod->md_token_is_def = 1;
596632
}

0 commit comments

Comments
 (0)