GH-106701: Move _PyUopExecute to Python/executor.c#106924
GH-106701: Move _PyUopExecute to Python/executor.c#106924brandtbucher merged 3 commits intopython:mainfrom
_PyUopExecute to Python/executor.c#106924Conversation
brandtbucher
added
skip news
interpreter-core
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
No need for a re-review if you decide to take my suggestion.
| 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?
There was a problem hiding this comment.
Not yet, no. That can be added once we actually need the symbols to be exported.
| _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).
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.
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.
3 participants
Mostly just a mechanical refactoring.
_PyUopExecute) to a new file #106701