René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Argument clinic: improve generated code for self converter type checks","articleBody":"# Feature or enhancement\r\n\r\nCurrently, typically for `__new__` and `__init__` methods, Argument Clinic will spell out the `self` type check as such:\r\n\r\n```python\r\nif kind == METHOD_NEW:\r\n    type_check = ('({0} == {1} ||\\n        '\r\n                  ' {0}-\u003etp_init == {2}tp_init)'\r\n                 ).format(self.name, type_object, prefix)\r\nelse:\r\n    type_check = ('(Py_IS_TYPE({0}, {1}) ||\\n        '\r\n                  ' Py_TYPE({0})-\u003etp_new == {2}tp_new)'\r\n                 ).format(self.name, type_object, prefix)\r\n```\r\n\r\n`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:\r\n\r\n\u003e 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](https://docs.python.org/3.10/c-api/type.html#c.PyTypeObject) for this class.\r\n\r\nFor heap types with module state, you'd typically do something like this:\r\n\r\n```c\r\ntypedef struct {\r\n   ...\r\n} myclass_object;\r\n\r\ntypedef struct {\r\n    PyTypeObject myclass_type;\r\n} module_state;\r\n\r\nstatic inline module_state *\r\nfind_state_by_type(PyTypeObject *tp)\r\n{\r\n    PyObject *mod = PyType_GetModuleByDef(\u0026moduledef); // Potentially slow!\r\n    void *state = PyModule_GetState(mod);\r\n    return (module_state*)state;\r\n}\r\n\r\n#define clinic_state() (find_state_by_type(type))\r\n#include \"clinic/mymod.c.h\"\r\n#undef clinic_state\r\n\r\n/*[clinic input]\r\nmodule mymod\r\nclass mymod.myclass \"myclass_object *\" \"clinic_state()-\u003emyclass_type\"\r\n[clinic start generated code]*/\r\n```\r\n\r\nCurrently, this generates clinic code like this:\r\n\r\n```c\r\nmymod_myclass_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)\r\n{\r\n    ...\r\n\r\n    if ((type == clinic_state()-\u003eRowType ||\r\n         type-\u003etp_init == clinic_state()-\u003eRowType-\u003etp_init) \u0026\u0026\r\n```\r\n\r\n... _potentially_ calling `PyType_GetModuleByDef` twice in the self type check.\r\n\r\n# Pitch\r\n\r\nSuggesting 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\r\n\r\n```diff\r\ndiff --git a/Modules/_sqlite/clinic/cursor.c.h b/Modules/_sqlite/clinic/cursor.c.h\r\nindex 36b8d0051a..633ad2e73d 100644\r\n--- a/Modules/_sqlite/clinic/cursor.c.h\r\n+++ b/Modules/_sqlite/clinic/cursor.c.h\r\n@@ -16,10 +16,11 @@ static int\r\n pysqlite_cursor_init(PyObject *self, PyObject *args, PyObject *kwargs)\r\n {\r\n     int return_value = -1;\r\n+    PyTypeObject *self_tp = clinic_state()-\u003eCursorType;\r\n     pysqlite_Connection *connection;\r\n \r\n-    if ((Py_IS_TYPE(self, clinic_state()-\u003eCursorType) ||\r\n-         Py_TYPE(self)-\u003etp_new == clinic_state()-\u003eCursorType-\u003etp_new) \u0026\u0026\r\n+    if ((Py_IS_TYPE(self, self_tp) ||\r\n+         Py_TYPE(self)-\u003etp_new == self_tp-\u003etp_new) \u0026\u0026\r\n         !_PyArg_NoKeywords(\"Cursor\", kwargs)) {\r\n         goto exit;\r\n     }\r\n@@ -318,4 +319,4 @@ pysqlite_cursor_close(pysqlite_Cursor *self, PyObject *Py_UNUSED(ignored))\r\n {\r\n     return pysqlite_cursor_close_impl(self);\r\n }\r\n-/*[clinic end generated code: output=e53e75a32a9d92bd input=a9049054013a1b77]*/\r\n+/*[clinic end generated code: output=e5eac0cbe29c88ad input=a9049054013a1b77]*/\r\n```\r\n\r\n# Previous discussion\r\n\r\nhttps://github.com/python/cpython/pull/101302#discussion_r1089924859\r\n\r\n\u003c!-- gh-linked-prs --\u003e\r\n### Linked PRs\r\n* gh-101411\r\n\u003c!-- /gh-linked-prs --\u003e\r\n","author":{"url":"https://github.com/erlend-aasland","@type":"Person","name":"erlend-aasland"},"datePublished":"2023-01-29T22:42:07.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":1},"url":"https://github.com/101409/cpython/issues/101409"}