bpo-36328: Fix compiler warning in Py_NewInterpreter() by matrixise · Pull Request #12381 · python/cpython

The code in new_interpreter() means this change is technically unnecessary.

Sadly, compiler are weak static analyzers which commonly emit false alarms. Adding "= NULL" doesn't hurt performance but allow to focus on real bugs: real warnings, by ignoring such false alarm.

Regardless, I recommend adding *tstate = NULL; at the beginning of new_interpreter() instead of in the various failure cases. With that you shouldn't need to change Py_NewInterpreter(). It will also benefit any other call sites.

I see multiple cases where new_interpreter() returns NULL on purpose. I have no opinion on this API. At least, the code didn't change since Python 3.6: it's not a a regression my recent changes using _PyInitError.

FWIW, a related remaining issue I have with new_interpreter() is that it doesn't guarantee an error is set when tstate is "returned" as NULL (i.e. most of the return _Py_INIT_OK(); cases).

Honestly, I don't understand Py_NewInterpreter() API. I don't understand why some errors are fatal, but some others are not. Sadly, the current Py_NewInterpreter() API can only return NULL to indicate an error... and the exception is not passed to the caller: "handle_error:" label prints the exception and clears it...

That isn't a problem in the existing code. However, it would be easy to miss that when adding a non-fatal error case. That said, it isn't that big a deal and you don't need to address it in this PR (or at all). :) However, it may be worth considering fixing here or opening a separate issue. At very least a comment would help. Dealing with it in the handle_error goto label would benefit all call sites. That could be done with assert PyErr_Occurred() or by PyErr_SetString(PyExc_RuntimeError, "..."), etc.

Maybe things can be done, but IMHO this PR is just fine: it does what it says, fix a compiler warning :-)