gh-86493: Modernize modules initialization code#106858
gh-86493: Modernize modules initialization code#106858serhiy-storchaka merged 3 commits intopython:mainfrom
Conversation
Use PyModule_Add() or PyModule_AddObjectRef() instead of soft deprecated PyModule_AddObject().
serhiy-storchaka
requested review from
abalkin,
corona10,
pganssle,
rhettinger and
tiran
as code owners
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM, but a few compiler warnings should be fixed first: see GHA job results, especially the Ubuntu job which logged many warnings.
| if (o == NULL) { | ||
| Tkinter_TclError = PyErr_NewException("_tkinter.TclError", NULL, NULL); | ||
| if (PyModule_AddObjectRef(m, "TclError", Tkinter_TclError)) { | ||
| Py_DECREF(m); |
There was a problem hiding this comment.
Note for later: i prefer "exec" functions which returns -1 on error, and the caller is responsible to manage the module refcount.
| if (PyModule_AddObject(m, name, o) == 0) { | ||
| return 0; | ||
| } | ||
| Py_DECREF(o); | ||
| return -1; |
There was a problem hiding this comment.
It is very confusing code with inverse logic. I thought there was a bug.
| return -1; | ||
| } | ||
| res = PyModule_AddObject( | ||
| res = PyModule_AddObjectRef( |
There was a problem hiding this comment.
I think there was a bug.
Modules/xxlimited_35.c
Outdated
| Py_INCREF(ErrorObject); | ||
| if (PyModule_AddObject(m, "error", ErrorObject) < 0) { | ||
| Py_DECREF(ErrorObject); | ||
| if (PyModule_AddObjectRef(m, "error", ErrorObject) < 0) { |
There was a problem hiding this comment.
It is interesting, that the compiler complains about PyModule_Add, but not about PyModule_AddObjectRef. There is a bug in PyModule_AddObjectRef declaration.
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
LGTM. I didn't check individual refcount, I rely on Refleaks buildbots for that and other reviewers 😁 Overall the change LGTM and makes the code shorter and more regular. The old code was really hard to understand with INCREF/DECREF dance.
|
That's a big change, thanks. |
Use PyModule_Add() or PyModule_AddObjectRef() instead of soft deprecated PyModule_AddObject().
4 participants
Use PyModule_Add() or PyModule_AddObjectRef() instead of soft deprecated PyModule_AddObject().
PyModule_AddObject() is only left in two modules:
_testcapiand_testbuffer. But that code ignores any failures for now. They need more significant rewriting.📚 Documentation preview 📚: https://cpython-previews--106858.org.readthedocs.build/