chore: use new PyCode API on Python 3.12 #4916
Conversation
|
Looks good, GHA pass, but I see you marked it as draft. Is there more you want to do here? |
I am checking more possible replacements of PyCode like this. |
|
@rwgk Nothing to add, can review again? |
| PyObject *self_arg = PyTuple_GET_ITEM(co_varnames, 0); | ||
| Py_DECREF(co_varnames); | ||
| PyObject *self_caller = dict_getitem(locals, self_arg); | ||
| Py_DECREF(locals); |
There was a problem hiding this comment.
Surprised we missed this.
There was a problem hiding this comment.
Unfortunately this turns out to be incorrect.
I only discovered that when trying to deploy this PR at Google, but in retrospect it's totally clear:
https://docs.python.org/3/c-api/reflection.html#c.PyEval_GetLocals
Return value: Borrowed reference. Part of the Stable ABI.
I'll take care of removing this line again. (I'll report the full error message there.)
There was a problem hiding this comment.
Oh, I didn't notice the CPython API convention.
|
Thank you @cyyever! |
henryiii
changed the title
This PR replaces PyObject_GetAttrString with PyCode_GetVarnames which is found by a warning of casting ''_object'' to "PyCodeObject".
It also fixes an object leak which was found by code review.Suggested changelog entry: