Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Argument clinic: improve generated code for self converter type checks #101409

Copy link
Copy link
Closed
@erlend-aasland

Description

@erlend-aasland
Issue body actions

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

Metadata

Metadata

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

    Morty Proxy This is a proxified and sanitized view of the page, visit original site.