Skip to content

[WIP] bpo-1635741: Port _collections module to multiphase initialization. #19071

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Port _collections module to multiphase initialization (:pep:`489`).
81 changes: 39 additions & 42 deletions Modules/_collectionsmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2546,24 +2546,56 @@ static PyTypeObject tuplegetter_type = {

/* module level code ********************************************************/

PyDoc_STRVAR(module_doc,
PyDoc_STRVAR(collections_doc,
"High performance data structures.\n\
- deque: ordered collection accessible from endpoints only\n\
- defaultdict: dict subclass with a default value factory\n\
");

static struct PyMethodDef module_functions[] = {
static struct PyMethodDef collections_methods[] = {
_COLLECTIONS__COUNT_ELEMENTS_METHODDEF
{NULL, NULL} /* sentinel */
};

static int
collections_exec(PyObject *m) {
PyTypeObject *typelist[] = {
&deque_type,
&defdict_type,
&PyODict_Type,
&dequeiter_type,
&dequereviter_type,
&tuplegetter_type,
Copy link
Member

Choose a reason for hiding this comment

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

Either you use Py_ARRAY_COUNT() to get the list length, or you need to add a NULL terminator. Currently, you rely on an undefined behavior.

};

for (int i = 0; typelist[i] != NULL; i++) {
Copy link
Member Author

@corona10 corona10 Mar 19, 2020

Choose a reason for hiding this comment

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

@vstinner I'd like to create a new helper function for this.

This logic can be reused.
https://github.com/python/cpython/blob/514c469719f149e1722a91a9d0c63bf89dfefb2a/Modules/itertoolsmodule.c#L4733L4747

What do you think?

Copy link
Member

@vstinner vstinner Mar 19, 2020

Choose a reason for hiding this comment

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

Do you mean combine PyType_Ready + _PyType_Name + PyModule_AddObject? Yeah, maybe it's worth it, but I would prefer to add it once this PR lands.

PyTypeObject *type = typelist[i];
if (PyType_Ready(type) < 0) {
return -1;
}
const char *name = _PyType_Name(type);
Py_INCREF(type);
if (PyModule_AddObject(m, name, (PyObject *)type) < 0) {
Py_DECREF(type);
return -1;
}
}

return 0;
}

static struct PyModuleDef_Slot collections_slots[] = {
{Py_mod_exec, collections_exec},
{0, NULL}
};

static struct PyModuleDef _collectionsmodule = {
PyModuleDef_HEAD_INIT,
"_collections",
module_doc,
-1,
module_functions,
NULL,
collections_doc,
0,
collections_methods,
collections_slots,
NULL,
NULL,
NULL
Expand All @@ -2572,40 +2604,5 @@ static struct PyModuleDef _collectionsmodule = {
PyMODINIT_FUNC
PyInit__collections(void)
{
PyObject *m;

m = PyModule_Create(&_collectionsmodule);
if (m == NULL)
return NULL;

if (PyType_Ready(&deque_type) < 0)
return NULL;
Py_INCREF(&deque_type);
PyModule_AddObject(m, "deque", (PyObject *)&deque_type);

defdict_type.tp_base = &PyDict_Type;
if (PyType_Ready(&defdict_type) < 0)
return NULL;
Py_INCREF(&defdict_type);
PyModule_AddObject(m, "defaultdict", (PyObject *)&defdict_type);

Py_INCREF(&PyODict_Type);
PyModule_AddObject(m, "OrderedDict", (PyObject *)&PyODict_Type);

if (PyType_Ready(&dequeiter_type) < 0)
return NULL;
Py_INCREF(&dequeiter_type);
PyModule_AddObject(m, "_deque_iterator", (PyObject *)&dequeiter_type);

if (PyType_Ready(&dequereviter_type) < 0)
return NULL;
Py_INCREF(&dequereviter_type);
PyModule_AddObject(m, "_deque_reverse_iterator", (PyObject *)&dequereviter_type);

if (PyType_Ready(&tuplegetter_type) < 0)
return NULL;
Py_INCREF(&tuplegetter_type);
PyModule_AddObject(m, "_tuplegetter", (PyObject *)&tuplegetter_type);

return m;
return PyModuleDef_Init(&_collectionsmodule);
}