converting to draft meanwhile I try to address test failures

Is this the only way to fix an error (whatever it is)? Currently the complexity of errors handling is hidden in make_const(), and the rest of code is kept clear. This PR spreads complexity across the whole file. I am not a fun of such change.

What exactly issues with the current code? I will try to solve them in other way.

Currently the complexity of errors handling is hidden in make_const()

The first issue is that that's very confusing because if you don't know that make_const it's cleaning the error indicator, it looks like a leak and that would be an anti-pattern everywhere else in the code. Furthermore, if someone copies that function it will quickly introduce actual leaks if there is an error path after because the argument will not be explicitly captured. For instance if you do

int res = make_const(node_, PyBool_FromLong(!state->optimize), ctx_); // Code copied from other call
if (!res) {
    return 0;
}
PyObject* my_list = Py_ListNew(0);
if (!my_list) {
    return 0; // Whops! Forgoto to clean the bool
}

A reviewer that doesn't know this weird behaviour about make_const() will be very confused (I was in #123081 (comment))

The second thing it's that is capturing everything and clearing it (except KeyBoardInterrupt) which isn't granular at all and it will hide other things like MemoryError and other exceptions that may happen there for any reason.

This PR spreads complexity across the whole file

It just surfaces existing complexity and makes the error paths more obvious. It's like if you say that doing

try:
   ...
except KeyError:
   ...
except ZeroDivisioError
   ...
else:
   print("BAD")
   raise

instead of

try:
   .... a lot of code here ...
except:
   pass

it's adding a lot of complexity. Sure there is more code, but you are being explicit about what's being ignored and why (like the ZeroDivisionError) and it's not hiding errors that may be legitimate .

@serhiy-storchaka OTOH I don't feel very strongly about this so If you still think you prefer the current code anyway I'm happy to close this but

This is not an antipattern. If there is some code repeated multiple times, you can factor out it into a function. And if this is the same check of argument before every call of the function, it can be moved into that function. This simplifies multiple caller sites and guarantees that the check will be not missed.

Such pattern is not common in the public C API, this is why it can be confusing, but it is used for convenience if this is appropriate (see for example Py_BuildValue("N") and PyModule_Add()). It is more often used in local functions.

Usually it is more correct to check for exceptions types that should be silenced (OverflowError, MemoryError, RecursionError, whatever) instead of exceptions types that should not be silenced (KeyboardInterrupt), but what other exceptions can be raised here? There is a limited set of exceptions that can be raised here, and only KeyboardInterrupt should not be silenced. If we a wrong and missed some type of very rare exception that should be silenced, we do not want to throw it in the face of a user. I do not know any other exception besides KeyboardInterrupt that should not be ignored. So checking for KeyboardInterrupt is not only practical (less code), it is more error-proof.

I prefer the current code.

This is not an antipattern. If there is some code repeated multiple times, you can factor out it into a function. And if this is the same check of argument before every call of the function, it can be moved into that function. This simplifies multiple caller sites and guarantees that the check will be not missed.

I don't see this as refactoring common code, the function is called make_const not maybe_make_const or something that suggest that it may receive null or clear errors. The fact that it checks the argument and handles error is not apparent from the name nor in any reviews.

What's more, we already have problems with functions that swallor errors and new functions had to be introduced to fix the confision for users (see PyDict_GetItemWithError).

but what other exceptions can be raised here?

At the very least any of the calls that create Python objects can raise MemoryError and technically if signal handlers are installed then the signal handler can raise whatever exception and we will clear it.

Such pattern is not common in the public C API, this is why it can be confusing, but it is used for convenience if this is appropriate (see for example Py_BuildValue("N") and PyModule_Add()). It is more often used in local functions.

I disagree, in the first case (Py_BuildValue) you can tell what would happen with lifetimes from the caller site because of the N and PyModule_Add doesn't clear any existing error that I know.

What also worries me more is that there may be new exceptions appearing after refactors that are going to be ignored and the optimisations may not be performed and we will not find out easily unless the tests are triggering those problems as well, which is unlikely.

I'm closing this, I don't feel that passionate about it and I trust @serhiy-storchaka criteria