More readable module constant addition statement

[rruuaanng]

Feature or enhancement

Proposal:

I would like to ask if we need to package the addition of macro and constant for some modules in Modules/*? I think they should be like socketmodule.c, maybe it will be better. If allowed, I'll submit the PR. Like this:

#define ADD_INT_MACRO(MOD, INT) do {                    \
    if (PyModule_AddIntConstant(MOD, #INT, INT) < 0) {  \
        goto error;                                     \
    }                                                   \
} while (0)

#ifdef AF_APPLETALK
    /* Appletalk DDP */
    ADD_INT_MACRO(m, AF_APPLETALK);
#endif

for example:
from posixmodule.c

#ifdef TMP_MAX
    if (PyModule_AddIntMacro(m, TMP_MAX)) return -1;
#endif
#ifdef WCONTINUED
    if (PyModule_AddIntMacro(m, WCONTINUED)) return -1;
#endif

from symtablemodule.c

if (PyModule_AddIntMacro(m, USE) < 0) return -1;
if (PyModule_AddIntMacro(m, DEF_GLOBAL) < 0) return -1;

I'm not sure if this is a cosmetic change, but I think it's necessary since there are only a few old modules that don't have this.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

[rruuaanng]

cc @vstinner

[picnixz]

We tend to avoid packaging macros like that, except for the C API tests which has some macros or hashlib-related code which also some macros. It would be better to have an ERRACTION suite that would be executed on error instead of just jumping to an 'error' label which may or may not exist. But honestly, considering the number of modules that would be affected and the additional indirection that it could cause while reading code, I doubt it would be accepted.

EDIT: Maybe I misunderstood, but do you mean to add modules-dedicated macros? if so, I think this would be a cosmetic-only change which also adds a level of indirection. The current code is fine IMO, and it could be refactored if we were to change the code in the future, but I don't think we'll just change the code for that.

EDIT 2: It might be ok for modules that have lots of constants just to improve readability. I would also ask @encukou, @erlend-aasland and @serhiy-storchaka since they are also part of the C API WG and may have a different opinion on that matter.

[serhiy-storchaka]

I do not see how this is anything but cosmetic changes. Cosmetic changes are allowed if the maintainer of that code decided that this will make the future maintenance easier. For every case it should be decided separately. We should not take a solution and look for problems, we should look for a suitable solution for each problem.

[rruuaanng]

I see. I'll package all unpackaged statements in all modules today. Stay tuned :)

[picnixz]

I see. I'll package all unpackaged statements in all modules today. Stay tuned :)

I'm not sure that we decided to do this. Please, ask the maintainer of each module you want to change whether this is needed or not before creating a corresponding PR.

[rruuaanng]

I agree that I'll list all the modules that need to be modified and complete them as required.

[serhiy-storchaka]

I a maintainer of some module need this, they can change the code of the corresponding module. Currently there is no need to massrewrite all modules.

See also an alternative idea in #86542.

[rruuaanng]

I'll read it carefully today and try to realize it in the near future.

Edit

In addition, thank you for your suggestions on this matter. I will pay close attention to the capi working group in the future.