Skip to content

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

@erlend-aasland

Description

@erlend-aasland

Feature or enhancement

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

ifkind==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:

typedefstruct{... } myclass_object; typedefstruct{PyTypeObjectmyclass_type} module_state; staticinlinemodule_state*find_state_by_type(PyTypeObject*tp){PyObject*mod=PyType_GetModuleByDef(&moduledef); // Potentially slow!void*state=PyModule_GetState(mod); return (module_state*)state} #defineclinic_state() (find_state_by_type(type)) #include"clinic/mymod.c.h"#undef clinic_state /*[clinic input]module mymodclass 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