gh-101944: mistake in documentation for PyModule_AddObjectRef()

[OTheDev]

Closes #101944.

The issue author opened #101945, but closed it shortly after (not sure why).

I double-checked the source code of the function and can confirm that there is a mistake in the docs:

cpython/Python/modsupport.c

Lines 630 to 660 in 924a3bf

	int
	PyModule_AddObjectRef(PyObject *mod, const char *name, PyObject *value)
	{
	if (!PyModule_Check(mod)) {
	PyErr_SetString(PyExc_TypeError,
	"PyModule_AddObjectRef() first argument "
	"must be a module");
	return -1;
	}
	if (!value) {
	if (!PyErr_Occurred()) {
	PyErr_SetString(PyExc_SystemError,
	"PyModule_AddObjectRef() must be called "
	"with an exception raised if value is NULL");
	}
	return -1;
	}
	PyObject *dict = PyModule_GetDict(mod);
	if (dict == NULL) {
	/* Internal error -- modules must have a dict! */
	PyErr_Format(PyExc_SystemError, "module '%s' has no __dict__",
	PyModule_GetName(mod));
	return -1;
	}
	if (PyDict_SetItemString(dict, name, value)) {
	return -1;
	}
	return 0;
	}

• Issue: Docs: PyModule_AddObjectRef should return -1 if value is NULL. #101944

[AlexWaygood]

LGTM. Thanks @OTheDev!

[vstinner]

I don't think that this change makes the documentation easier to understand. For me, it sounds more misleading than before :-(

[vstinner]

Please keep this general statement. There are many cases where the function can fail. For example, if mod is not a module or has no dictionary. PyDict_SetItemString() can fail with MemoryError, etc.

[vstinner]

This documentation sounds misleading to me. The normal case when value is NULL is that an exception is already set, so the function does not "raise an exception". Also, I dislike documenting the exact exception type for implementation details: please don't document "SystemError".

[erlend-aasland]

please don't document "SystemError".

That was my fault; I encouraged it in #101957 (comment). Sorry 'bout that!

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[vstinner]

I created a different doc change to clarify the NULL case: PR gh-129433.