◐ Shell
clean mode source ↗

Argument clinic: improve generated code for self converter type checks

erlend-aasland

Feature or enhancement

Currently, typically for __new__ and __init__ methods, Argument Clinic will spell out the self type check as such:

if kind == METHOD_NEW:
    type_check = ('({0} == {1} ||\n        '
                  ' {0}->tp_init == {2}tp_init)'
                 ).format(self.name, type_object, prefix)
else:
    type_check = ('(Py_IS_TYPE({0}, {1}) ||\n        '
                  ' Py_TYPE({0})->tp_new == {2}tp_new)'
                 ).format(self.name, type_object, prefix)

prefix is a slightly modified variant of type_object, depending on if the latter is a pointer. This works swell in most cases, but with module state and heap types, the generated code is not optimal. First, let's just quote the AC docs on how to declare a class:

When you declare a class, you must also specify two aspects of its type in C: the type declaration you’d use for a pointer to an instance of this class, and a pointer to the PyTypeObject for this class.

For heap types with module state, you'd typically do something like this:

typedef struct {
   ...
} myclass_object;

typedef struct {
    PyTypeObject myclass_type;
} module_state;

static inline module_state *
find_state_by_type(PyTypeObject *tp)
{
    PyObject *mod = PyType_GetModuleByDef(&moduledef); // Potentially slow!
    void *state = PyModule_GetState(mod);
    return (module_state*)state;
}

#define clinic_state() (find_state_by_type(type))
#include "clinic/mymod.c.h"
#undef clinic_state

/*[clinic input]
module mymod
class mymod.myclass "myclass_object *" "clinic_state()->myclass_type"
[clinic start generated code]*/

Currently, this generates clinic code like this:

mymod_myclass_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
{
    ...

    if ((type == clinic_state()->RowType ||
         type->tp_init == clinic_state()->RowType->tp_init) &&

... potentially calling PyType_GetModuleByDef twice in the self type check.

Pitch

Suggesting to modify clinic to store the self type pointer in a local variable, and use that variable in the self type check. Proof-of-concept diff from the _sqlite extension module clinic code

diff --git a/Modules/_sqlite/clinic/cursor.c.h b/Modules/_sqlite/clinic/cursor.c.h
index 36b8d0051a..633ad2e73d 100644
--- a/Modules/_sqlite/clinic/cursor.c.h
+++ b/Modules/_sqlite/clinic/cursor.c.h
@@ -16,10 +16,11 @@ static int
 pysqlite_cursor_init(PyObject *self, PyObject *args, PyObject *kwargs)
 {
     int return_value = -1;
+    PyTypeObject *self_tp = clinic_state()->CursorType;
     pysqlite_Connection *connection;
 
-    if ((Py_IS_TYPE(self, clinic_state()->CursorType) ||
-         Py_TYPE(self)->tp_new == clinic_state()->CursorType->tp_new) &&
+    if ((Py_IS_TYPE(self, self_tp) ||
+         Py_TYPE(self)->tp_new == self_tp->tp_new) &&
         !_PyArg_NoKeywords("Cursor", kwargs)) {
         goto exit;
     }
@@ -318,4 +319,4 @@ pysqlite_cursor_close(pysqlite_Cursor *self, PyObject *Py_UNUSED(ignored))
 {
     return pysqlite_cursor_close_impl(self);
 }
-/*[clinic end generated code: output=e53e75a32a9d92bd input=a9049054013a1b77]*/
+/*[clinic end generated code: output=e5eac0cbe29c88ad input=a9049054013a1b77]*/

Previous discussion

#101302 (comment)

Linked PRs