GeezerVison

This is the same as #23240, but without large changes to modules _ssl, _testbuffer, _testcapi, posix, select, socket, _msi, msvcrt. They will be extracted to separate PRs.

Could you split this PR in two parts? The first uncontroversial PR which only adds the function, and then a second PR which modify PyInit functions to use it.

These days, there are dozens of PRs modifying PyInit functions which would be in conflict with the second part. See bpo-40077 and bpo-1635741.

I still prefer https://github.com/python/cpython/pull/23286/files over PyModule_Add. The three new APIs from my PR will simplify a lot of common cases:

• initialize and add new type to module dict
• initialize and add new exception to module dict
• add int/str/float/bool constants from a static array

How about we land PR #23286 first, port more code to the new API, and then see if we still need PyModule_Add?

This PR is stale because it has been open for 30 days with no activity.

I dislike APIs stealing references. IMO we should stop adding such API. I would prefer regular (boring and verbose) reference counting to make the C API easier to understand. Currently, developers have to check the documentation of each function to check if it returns a borrowed or strong reference, if it adds or removes a reference (and leave the ref count unchanged), etc.

It is a convenient API. How else would you fix Modules/mathmodule.c, for example?

The "N" code in Py_BuildValue() serves the same purpose. It allows to write in one line the code which would require 7 lines otherwise:

return Py_BuildValue("Nl", _PyLong_FromTime_t(sec), nsec);

instead of:

PyObject *tmp = _PyLong_FromTime_t(sec);
if (tmp == NULL) {
    return NULL;
}
PyObject *res = Py_BuildValue("Ol", tmp, nsec);
Py_DECREF(tmp);
return res;

It is a convenient API. How else would you fix Modules/mathmodule.c, for example?

Usually, a macro is written to create a temporary object and add it to the module. If you use PyModule_AddObjectRef(), the macro would also clear the reference.

I dislike the fact that macro is "required" to write correct code.

In 2020, @tiran proposed designing a new API to make this work easier with a declarative syntax: https://discuss.python.org/t/define-module-constants-in-pymoduledef/5749

Macro is convenient because it can include "goto error" or "return NULL". But having to write such macro in every module is tiring. PyModule_AddNew() does not need to be a macro, it can be a regular function. It is the original PyModule_AddObject() written right.

We can make it a private function at first if you so dislike adding it to the public API. Then we can add new @tiran's API and decide whether it is worth to add public PyModule_AddNew(). But I think that it is better to have PyModule_AddNew() than not have neither PyModule_AddNew() nor new module initialization API.

I dislike APIs stealing references. IMO we should stop adding such API.

IMO, that's a good guideline, but PyModule_AddNew is a great example of when we should break this guideline. (Doesn't beat Py_DecRef, but it's close!)

But it should be very clear from the name. I don't think New is clear enough, and would prefer to one of Steal, Consume, Take, Move (see this draft) -- but we should settle on one of those and use it consistently for new API of this kind.

To be clear, I dislike APIs stealing references, but I don't plan to block this PR. I will just not endorse it :-)

If using PyModule_AddObjectRef() is not convenient, maybe we need to provide "something" to make it easier to use. Sadly, the C language is limited in terms of macros/templates :-( The approach based on a description is promising, but I also know that there are many technical challenges which may not be possible to be solved.

By the way, I'm really unhappy on the bad state of our PyInit functions: many of them simply have no error checking, or just one final PyErr_Occurred() check. That's bad. The trend is more to disallow calling functions with an exception set (it might be enforced later). If this new function makes the situation better: go ahead ;-)

What does "New" stand for? This function does not create a new reference, but steals a reference. Is is "New" because its a new function replacing an old one? What if tomorrow, there is a newer function? Should we call it PyModule_AddNewer or PyModule_AddNewEx?

I suggest to just call it PyModule_Add().

It adds a newly created object.
I think that the format unit "N" in Py_BuildValue() means "new", because it is purposed to add newly created object, for which we do not need to keep other reference. I was guided by the same reasons for choosing name PyModule_AddNew(). Initially I named it simply PyModule_Add(). I can rename it back.

It may be easier to review this big PR if you could move code fixing PyInit functions into a separated PR.

Already did this. #106767 and #106768.

Ok, but this PR changes Modules/_curses_panel.c for example, and the change doesn't even use PyModule_Add(). Can you maybe revert these changes? If not, tell me, I will try to get more time to through the whole change.

Thanks Victor.

Even if I dislike stealing references, the API seems to help simplifying PyInit code a lot.

I added the PyModule_Add() function to the pythoncapi-compat project: python/pythoncapi-compat@e3f0eba

I wrote PR #106859 to document PyModule_AddObject() soft deprecation in What's New in Python 3.13.