classification
Title: Use designated initializers to define PyTypeObject types
Type: Stage:
Components: Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, methane, serhiy.storchaka, vstinner, yselivanov
Priority: normal Keywords:

Created on 2017-01-13 14:31 by vstinner, last changed 2017-02-01 16:52 by vstinner. This issue is now closed.

Messages (8)
msg285399 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-01-13 14:31
Currently, PyTypeObject fields are set in order in the C code. Typical example:

PyTypeObject PyMethod_Type = {
    PyVarObject_HEAD_INIT(&PyType_Type, 0)
    "method",
    sizeof(PyMethodObject),
    0,
    (destructor)method_dealloc,                 /* tp_dealloc */
    0,                                          /* tp_print */
    0,                                          /* tp_getattr */
    0,                                          /* tp_setattr */
    0,                                          /* tp_reserved */
    (reprfunc)method_repr,                      /* tp_repr */
    0,                                          /* tp_as_number */
    0,                                          /* tp_as_sequence */
    0,                                          /* tp_as_mapping */
    (hashfunc)method_hash,                      /* tp_hash */
    method_call,                                /* tp_call */
    0,                                          /* tp_str */
    method_getattro,                            /* tp_getattro */
    PyObject_GenericSetAttr,                    /* tp_setattro */
    0,                                          /* tp_as_buffer */
    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, /* tp_flags */
    method_doc,                                 /* tp_doc */
    (traverseproc)method_traverse,              /* tp_traverse */
    0,                                          /* tp_clear */
    method_richcompare,                         /* tp_richcompare */
    offsetof(PyMethodObject, im_weakreflist), /* tp_weaklistoffset */
    0,                                          /* tp_iter */
    0,                                          /* tp_iternext */
    method_methods,                             /* tp_methods */
    method_memberlist,                          /* tp_members */
    method_getset,                              /* tp_getset */
    0,                                          /* tp_base */
    0,                                          /* tp_dict */
    method_descr_get,                           /* tp_descr_get */
    0,                                          /* tp_descr_set */
    0,                                          /* tp_dictoffset */
    0,                                          /* tp_init */
    0,                                          /* tp_alloc */
    method_new,                                 /* tp_new */
};

The aligned comment is an old practice required to be able to see to which field correspond a line.

This syntax usually produces a lot of zeros.

The C standard (C99?) allows to use a more readable syntax:

PyTypeObject PyMethod_Type = {
    PyVarObject_HEAD_INIT(&PyType_Type, 0)
    .tp_name = "method",
    .tp_basicsize = sizeof(PyMethodObject),
    .tp_dealloc = (destructor)method_dealloc,
    .tp_repr = (reprfunc)method_repr,
    .tp_hash = (hashfunc)method_hash,
    .tp_call = method_call,
    .tp_getattro = method_getattro,
    ...
};

It seems like it's possible to start with positional fields and then switch to named fields, so PyVarObject_HEAD_INIT() still works. Maybe maybe PyVarObject_HEAD_INIT() should also evolve? Or maybe we need a new macro using the ".tp_xxx=...," syntax?

INADA Naoki proposed to use this syntax in my pull request which adds a new tp_fastcall field which requires to add many zeros:
https://github.com/python/cpython/pull/65#pullrequestreview-16566423

Example of change:

@@ -370,6 +370,17 @@ PyTypeObject PyMethod_Type = {
     0,                                          /* tp_init */
     0,                                          /* tp_alloc */
     method_new,                                 /* tp_new */
+    0,                                          /* tp_free */
+    0,                                          /* tp_is_gc */
+    0,                                          /* tp_bases */
+    0,                                          /* tp_mro */
+    0,                                          /* tp_cache */
+    0,                                          /* tp_subclasses */
+    0,                                          /* tp_weaklist */
+    0,                                          /* tp_del */
+    0,                                          /* tp_version_tag */
+    0,                                          /* tp_finalize */
+    (fastternaryfunc)method_fastcall,           /* tp_fastcall */
 };
msg285401 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-13 14:56
The widespread use of PyType_FromSpec() will supersede this issue. But there is a caveat (issue26979).
msg285408 - (view) Author: Inada Naoki (methane) * (Python committer) Date: 2017-01-13 15:36
https://www.python.org/dev/peps/pep-0007/#c-dialect says
"designated initializers (especially nice for type declarations)"

So I think it's allowed explicitly.
To minimize diff, I think we can start using it when adding
slot at bottom of type declaration, or creating completely
new type.
msg285409 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-01-13 15:39
> The widespread use of PyType_FromSpec() will supersede this issue.

Oh, I didn't know PyType_FromSpec(). It is only used for a very few types in the Python stdlib.
msg285412 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-13 15:58
Designated initializers already are used in _asynciomodule.c. But this is one of C99 features that are not compatible with C++. Using them makes harder possible using C++ compiler in future.
msg285413 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-01-13 16:06
> Designated initializers already are used in _asynciomodule.c.

Oh, I didn't notice. Since I don't recall any bug report on compilation failing on this module, I guess that it's fine to start to use them in more code.

> But this is one of C99 features that are not compatible with C++. Using them makes harder possible using C++ compiler in future.

Hum, what kind of issue? With which compiler? What is it in the PEP 7 in this case?
msg285417 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-01-13 16:37
There is nothing about this in PEP 7, but I think it would be better to use common subset of C and C++. This would allow to migrate to the subset of C++ in future. It is sad that PEP 7 allows C99 features not compatible with C++.
msg286652 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-02-01 16:52
The question was discussed on the python-dev mailing list:
[Python-Dev] Can we use "designated initializer" widely in core modules?
https://mail.python.org/pipermail/python-dev/2017-January/147154.html

I understood that it's ok to use them in new code, but not to convert existing code:

Guido van Rossum: "I'm against changing any existing code to use it -- such massive changes are high risk and low reward. Just do it for new fields or new types."
https://mail.python.org/pipermail/python-dev/2017-January/147175.html

So I close this issue.

Note: I rejected my changes to add new tp_fastnew, tp_fastinit and tp_fastcall slots, issues #29358 and #29259.
History
Date User Action Args
2017-02-01 16:52:57vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg286652
2017-01-13 16:37:21serhiy.storchakasetmessages: + msg285417
2017-01-13 16:06:51vstinnersetnosy: + benjamin.peterson, yselivanov
messages: + msg285413
2017-01-13 15:58:52serhiy.storchakasetmessages: + msg285412
2017-01-13 15:39:20vstinnersetmessages: + msg285409
2017-01-13 15:36:32methanesetmessages: + msg285408
2017-01-13 14:56:59serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg285401
2017-01-13 14:31:05vstinnercreate