bpo-42990: Further refactoring of PyEval_ functions. (GH-24368)

* Further refactoring of PyEval_EvalCode and friends. Break into make-frame, and eval-frame parts.

* Simplify function vector call using new _PyEval_Vector.

* Remove unused internal functions: _PyEval_EvalCodeWithName and _PyEval_EvalCode.

* Don't use legacy function PyEval_EvalCodeEx.

Author: markshannon

Include/cpython/frameobject.h

@@ -71,8 +71,8 @@ PyAPI_FUNC(PyFrameObject *) PyFrame_New(PyThreadState *, PyCodeObject *,
                                         PyObject *, PyObject *);
 
 /* only internal use */
-PyFrameObject* _PyFrame_New_NoTrack(PyThreadState *, PyCodeObject *,
-                                    PyObject *, PyObject *, PyObject *);
+PyFrameObject*
+_PyFrame_New_NoTrack(PyThreadState *, PyFrameConstructor *, PyObject *);
 
 
 /* The rest of the interface is specific for frame objects */

Include/eval.h

@@ -18,16 +18,6 @@ PyAPI_FUNC(PyObject *) PyEval_EvalCodeEx(PyObject *co,
                                          PyObject *kwdefs, PyObject *closure);
 
 #ifndef Py_LIMITED_API
-PyAPI_FUNC(PyObject *) _PyEval_EvalCodeWithName(
-    PyObject *co,
-    PyObject *globals, PyObject *locals,
-    PyObject *const *args, Py_ssize_t argcount,
-    PyObject *const *kwnames, PyObject *const *kwargs,
-    Py_ssize_t kwcount, int kwstep,
-    PyObject *const *defs, Py_ssize_t defcount,
-    PyObject *kwdefs, PyObject *closure,
-    PyObject *name, PyObject *qualname);
-
 PyAPI_FUNC(PyObject *) _PyEval_CallTracing(PyObject *func, PyObject *args);
 #endif
 

Include/internal/pycore_ceval.h

@@ -40,12 +40,11 @@ _PyEval_EvalFrame(PyThreadState *tstate, PyFrameObject *f, int throwflag)
     return tstate->interp->eval_frame(tstate, f, throwflag);
 }
 
-extern PyObject *_PyEval_EvalCode(
-    PyThreadState *tstate,
-    PyFrameConstructor *desc, PyObject *locals,
-    PyObject *const *args, Py_ssize_t argcount,
-    PyObject *const *kwnames, PyObject *const *kwargs,
-    Py_ssize_t kwcount, int kwstep);
+extern PyObject *
+_PyEval_Vector(PyThreadState *tstate,
+            PyFrameConstructor *desc, PyObject *locals,
+            PyObject* const* args, size_t argcount,
+            PyObject *kwnames);
 
 #ifdef EXPERIMENTAL_ISOLATED_SUBINTERPRETERS
 extern int _PyEval_ThreadsInitialized(PyInterpreterState *interp);

Misc/NEWS.d/next/Core and Builtins/2021-01-30-11-31-44.bpo-42990.69h_zK.rst

@@ -0,0 +1,5 @@
+Refactor the ``PyEval_`` family of functions.
+
+* An new function ``_PyEval_Vector`` is added to simplify calls to Python from C.
+* ``_PyEval_EvalCodeWithName`` is removed
+* ``PyEval_EvalCodeEx`` is retained as part of the API, but is not used internally

Objects/call.c

@@ -328,87 +328,24 @@ PyCFunction_Call(PyObject *callable, PyObject *args, PyObject *kwargs)
 
 /* --- PyFunction call functions ---------------------------------- */
 
-static PyObject* _Py_HOT_FUNCTION
-function_code_fastcall(PyThreadState *tstate, PyCodeObject *co,
-                       PyObject *const *args, Py_ssize_t nargs,
-                       PyFunctionObject *func)
-{
-    assert(tstate != NULL);
-    assert(func != NULL);
-
-    /* XXX Perhaps we should create a specialized
-       _PyFrame_New_NoTrack() that doesn't take locals, but does
-       take builtins without sanity checking them.
-       */
-    PyFrameObject *f = _PyFrame_New_NoTrack(tstate, co, func->func_globals, func->func_builtins, NULL);
-    if (f == NULL) {
-        return NULL;
-    }
-
-    PyObject **fastlocals = f->f_localsplus;
-
-    for (Py_ssize_t i = 0; i < nargs; i++) {
-        Py_INCREF(*args);
-        fastlocals[i] = *args++;
-    }
-    PyObject *result = _PyEval_EvalFrame(tstate, f, 0);
-
-    if (Py_REFCNT(f) > 1) {
-        Py_DECREF(f);
-        _PyObject_GC_TRACK(f);
-    }
-    else {
-        ++tstate->recursion_depth;
-        Py_DECREF(f);
-        --tstate->recursion_depth;
-    }
-    return result;
-}
-
-
 PyObject *
 _PyFunction_Vectorcall(PyObject *func, PyObject* const* stack,
                        size_t nargsf, PyObject *kwnames)
 {
     assert(PyFunction_Check(func));
-    assert(kwnames == NULL || PyTuple_CheckExact(kwnames));
-
+    PyFrameConstructor *f = PyFunction_AS_FRAME_CONSTRUCTOR(func);
     Py_ssize_t nargs = PyVectorcall_NARGS(nargsf);
     assert(nargs >= 0);
-    Py_ssize_t nkwargs = (kwnames == NULL) ? 0 : PyTuple_GET_SIZE(kwnames);
-    assert((nargs == 0 && nkwargs == 0) || stack != NULL);
-    /* kwnames must only contain strings and all keys must be unique */
-
     PyThreadState *tstate = _PyThreadState_GET();
-    PyCodeObject *co = (PyCodeObject *)PyFunction_GET_CODE(func);
-    PyObject *argdefs = PyFunction_GET_DEFAULTS(func);
-
-    if (co->co_kwonlyargcount == 0 && nkwargs == 0 &&
-        (co->co_flags & ~PyCF_MASK) == (CO_OPTIMIZED | CO_NEWLOCALS | CO_NOFREE))
-    {
-        if (argdefs == NULL && co->co_argcount == nargs) {
-            return function_code_fastcall(tstate, co, stack, nargs, (PyFunctionObject *)func);
-        }
-        else if (nargs == 0 && argdefs != NULL
-                 && co->co_argcount == PyTuple_GET_SIZE(argdefs)) {
-            /* function called with no arguments, but all parameters have
-               a default value: use default values as arguments .*/
-            stack = _PyTuple_ITEMS(argdefs);
-            return function_code_fastcall(tstate, co,
-                                          stack, PyTuple_GET_SIZE(argdefs),
-                                          (PyFunctionObject *)func);
-        }
+    assert(nargs == 0 || stack != NULL);
+    if (((PyCodeObject *)f->fc_code)->co_flags & CO_OPTIMIZED) {
+        return _PyEval_Vector(tstate, f, NULL, stack, nargs, kwnames);
+    }
+    else {
+        return _PyEval_Vector(tstate, f, f->fc_globals, stack, nargs, kwnames);
     }
-
-    return _PyEval_EvalCode(tstate,
-                PyFunction_AS_FRAME_CONSTRUCTOR(func), (PyObject *)NULL,
-                stack, nargs,
-                nkwargs ? _PyTuple_ITEMS(kwnames) : NULL,
-                stack + nargs,
-                nkwargs, 1);
 }
 
-
 /* --- More complex call functions -------------------------------- */
 
 /* External interface to call any callable object.

Objects/frameobject.c

@@ -816,11 +816,10 @@ frame_alloc(PyCodeObject *code)
 
 
 PyFrameObject* _Py_HOT_FUNCTION
-_PyFrame_New_NoTrack(PyThreadState *tstate, PyCodeObject *code,
-                     PyObject *globals, PyObject *builtins, PyObject *locals)
+_PyFrame_New_NoTrack(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals)
 {
 #ifdef Py_DEBUG
-    if (code == NULL || globals == NULL || builtins == NULL ||
+    if (con == NULL || con->fc_code == NULL ||
         (locals != NULL && !PyMapping_Check(locals))) {
         PyErr_BadInternalCall();
         return NULL;
@@ -829,38 +828,21 @@ _PyFrame_New_NoTrack(PyThreadState *tstate, PyCodeObject *code,
 
     PyFrameObject *back = tstate->frame;
 
-    PyFrameObject *f = frame_alloc(code);
+    PyFrameObject *f = frame_alloc((PyCodeObject *)con->fc_code);
     if (f == NULL) {
         return NULL;
     }
 
     f->f_stackdepth = 0;
-    Py_INCREF(builtins);
-    f->f_builtins = builtins;
+    Py_INCREF(con->fc_builtins);
+    f->f_builtins = con->fc_builtins;
     Py_XINCREF(back);
     f->f_back = back;
-    Py_INCREF(code);
-    Py_INCREF(globals);
-    f->f_globals = globals;
-    /* Most functions have CO_NEWLOCALS and CO_OPTIMIZED set. */
-    if ((code->co_flags & (CO_NEWLOCALS | CO_OPTIMIZED)) ==
-        (CO_NEWLOCALS | CO_OPTIMIZED))
-        ; /* f_locals = NULL; will be set by PyFrame_FastToLocals() */
-    else if (code->co_flags & CO_NEWLOCALS) {
-        locals = PyDict_New();
-        if (locals == NULL) {
-            Py_DECREF(f);
-            return NULL;
-        }
-        f->f_locals = locals;
-    }
-    else {
-        if (locals == NULL) {
-            locals = globals;
-        }
-        Py_INCREF(locals);
-        f->f_locals = locals;
-    }
+    Py_INCREF(con->fc_code);
+    Py_INCREF(con->fc_globals);
+    f->f_globals = con->fc_globals;
+    Py_XINCREF(locals);
+    f->f_locals = locals;
 
     f->f_lasti = -1;
     f->f_lineno = 0;
@@ -875,12 +857,23 @@ _PyFrame_New_NoTrack(PyThreadState *tstate, PyCodeObject *code,
     return f;
 }
 
+/* Legacy API */
 PyFrameObject*
 PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
             PyObject *globals, PyObject *locals)
 {
     PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals);
-    PyFrameObject *f = _PyFrame_New_NoTrack(tstate, code, globals, builtins, locals);
+    PyFrameConstructor desc = {
+        .fc_globals = globals,
+        .fc_builtins = builtins,
+        .fc_name = code->co_name,
+        .fc_qualname = code->co_name,
+        .fc_code = (PyObject *)code,
+        .fc_defaults = NULL,
+        .fc_kwdefaults = NULL,
+        .fc_closure = NULL
+    };
+    PyFrameObject *f = _PyFrame_New_NoTrack(tstate, &desc, locals);
     Py_DECREF(builtins);
     if (f)
         _PyObject_GC_TRACK(f);

Objects/funcobject.c

@@ -575,9 +575,9 @@ func_new_impl(PyTypeObject *type, PyCodeObject *code, PyObject *globals,
 
     newfunc = (PyFunctionObject *)PyFunction_New((PyObject *)code,
                                                  globals);
-    if (newfunc == NULL)
+    if (newfunc == NULL) {
         return NULL;
-
+    }
     if (name != Py_None) {
         Py_INCREF(name);
         Py_SETREF(newfunc->func_name, name);

Python/bltinmodule.c

@@ -8,6 +8,7 @@
 #include "pycore_pyerrors.h"      // _PyErr_NoMemory()
 #include "pycore_pystate.h"       // _PyThreadState_GET()
 #include "pycore_tuple.h"         // _PyTuple_FromArray()
+#include "pycore_ceval.h"         // _PyEval_Vector()
 
 _Py_IDENTIFIER(__builtins__);
 _Py_IDENTIFIER(__dict__);
@@ -219,9 +220,9 @@ builtin___build_class__(PyObject *self, PyObject *const *args, Py_ssize_t nargs,
                      Py_TYPE(ns)->tp_name);
         goto error;
     }
-    cell = PyEval_EvalCodeEx(PyFunction_GET_CODE(func), PyFunction_GET_GLOBALS(func), ns,
-                             NULL, 0, NULL, 0, NULL, 0, NULL,
-                             PyFunction_GET_CLOSURE(func));
+    PyFrameConstructor *f =  PyFunction_AS_FRAME_CONSTRUCTOR(func);
+    PyThreadState *tstate = PyThreadState_GET();
+    cell = _PyEval_Vector(tstate, f, ns, NULL, 0, NULL);
     if (cell != NULL) {
         if (bases != orig_bases) {
             if (PyMapping_SetItemString(ns, "__orig_bases__", orig_bases) < 0) {