GH-106701: Move _PyUopExecute to Python/executor.c - #106924
brandtbucher merged 3 commits into
Conversation
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
No need for a re-review if you decide to take my suggestion.
Sorry, something went wrong.
| void _PyEval_FormatKwargsError(PyThreadState *tstate, PyObject *func, PyObject *kwargs); | ||
| PyObject *_PyEval_MatchClass(PyThreadState *tstate, PyObject *subject, PyObject *type, Py_ssize_t nargs, PyObject *kwargs); | ||
| PyObject *_PyEval_MatchKeys(PyThreadState *tstate, PyObject *map, PyObject *keys); | ||
| int _PyEval_UnpackIterable(PyThreadState *tstate, PyObject *v, int argcnt, int argcntafter, PyObject **sp); |
There was a problem hiding this comment.
Didn't you say previously that these would have to be PyAPI_FUNC?
Sorry, something went wrong.
There was a problem hiding this comment.
Not yet, no. That can be added once we actually need the symbols to be exported.
Sorry, something went wrong.
| _PyEval_FormatExcCheckArg(tstate, PyExc_UnboundLocalError, | ||
| UNBOUNDLOCAL_ERROR_MSG, | ||
| PyTuple_GetItem(_PyFrame_GetCode(frame)->co_localsplusnames, oparg) | ||
| ); |
There was a problem hiding this comment.
I think it would be fine to call _PyEval_FormatExcUnbound here and at the corresponding point in ceval.c, then you won't have to move UNBOUNDLOCAL_ERROR_MSG into the header file (which feels a little unprincipled).
Sorry, something went wrong.
There was a problem hiding this comment.
I can do that, sure.
FWIW though, NAME_ERROR_MSG is already in ceval_macros.h, so it's not super out of place there. In fact, it might be nice to put NAME_ERROR_MSG, UNBOUNDFREE_ERROR_MSG, and UNBOUNDLOCAL_ERROR_MSG together in the same place.
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, all together in the same place is good, so then I vote for moving all to the header and not changing the code at the label.
Sorry, something went wrong.
Mostly just a mechanical refactoring.