[Python-checkins] bpo-42990: Refactor _PyFrame_New_NoTrack() (GH-24566)
vstinner
webhook-mailer at python.org
Thu Feb 18 13:20:44 EST 2021
https://github.com/python/cpython/commit/44085a3fc9a150478aec1872dd1079c60dcc42f6
commit: 44085a3fc9a150478aec1872dd1079c60dcc42f6
branch: master
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2021-02-18T19:20:16+01:00
summary:
bpo-42990: Refactor _PyFrame_New_NoTrack() (GH-24566)
* Refactor _PyFrame_New_NoTrack() and PyFunction_NewWithQualName()
code.
* PyFrame_New() checks for _PyEval_BuiltinsFromGlobals() failure.
* Fix a ref leak in _PyEval_BuiltinsFromGlobals() error path.
* Complete PyFunction_GetModule() documentation: it returns a
borrowed reference and it can return NULL.
* Move _PyEval_BuiltinsFromGlobals() definition to the internal C
API.
* PyFunction_NewWithQualName() uses _Py_IDENTIFIER() API for the
"__name__" string to make it compatible with subinterpreters.
files:
M Doc/c-api/function.rst
M Include/cpython/frameobject.h
M Include/internal/pycore_ceval.h
M Objects/frameobject.c
M Objects/funcobject.c
M Python/ceval.c
diff --git a/Doc/c-api/function.rst b/Doc/c-api/function.rst
index 20968828e0bb3..ad008425f3811 100644
--- a/Doc/c-api/function.rst
+++ b/Doc/c-api/function.rst
@@ -61,9 +61,11 @@ There are a few functions specific to Python functions.
.. c:function:: PyObject* PyFunction_GetModule(PyObject *op)
- Return the *__module__* attribute of the function object *op*. This is normally
- a string containing the module name, but can be set to any other object by
- Python code.
+ Return a :term:`borrowed reference` to the *__module__* attribute of the
+ function object *op*. It can be *NULL*.
+
+ This is normally a string containing the module name, but can be set to any
+ other object by Python code.
.. c:function:: PyObject* PyFunction_GetDefaults(PyObject *op)
diff --git a/Include/cpython/frameobject.h b/Include/cpython/frameobject.h
index 5a19c006d9153..5122ec41a3d22 100644
--- a/Include/cpython/frameobject.h
+++ b/Include/cpython/frameobject.h
@@ -92,5 +92,3 @@ PyAPI_FUNC(void) PyFrame_FastToLocals(PyFrameObject *);
PyAPI_FUNC(void) _PyFrame_DebugMallocStats(FILE *out);
PyAPI_FUNC(PyFrameObject *) PyFrame_GetBack(PyFrameObject *frame);
-
-PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals);
diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h
index 0491d48a789eb..bb22322114ecb 100644
--- a/Include/internal/pycore_ceval.h
+++ b/Include/internal/pycore_ceval.h
@@ -31,9 +31,12 @@ PyAPI_FUNC(void) _PyEval_SetCoroutineOriginTrackingDepth(
PyThreadState *tstate,
int new_depth);
-/* Private function */
void _PyEval_Fini(void);
+
+extern PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals);
+
+
static inline PyObject*
_PyEval_EvalFrame(PyThreadState *tstate, PyFrameObject *f, int throwflag)
{
diff --git a/Objects/frameobject.c b/Objects/frameobject.c
index 57105e1a9eb1e..5f7fa40ff6e0e 100644
--- a/Objects/frameobject.c
+++ b/Objects/frameobject.c
@@ -1,12 +1,11 @@
/* Frame object implementation */
#include "Python.h"
-#include "pycore_object.h"
-#include "pycore_gc.h" // _PyObject_GC_IS_TRACKED()
+#include "pycore_ceval.h" // _PyEval_BuiltinsFromGlobals()
+#include "pycore_object.h" // _PyObject_GC_UNTRACK()
-#include "code.h"
-#include "frameobject.h"
-#include "opcode.h"
+#include "frameobject.h" // PyFrameObject
+#include "opcode.h" // EXTENDED_ARG
#include "structmember.h" // PyMemberDef
#define OFF(x) offsetof(PyFrameObject, x)
@@ -762,9 +761,7 @@ _Py_IDENTIFIER(__builtins__);
static inline PyFrameObject*
frame_alloc(PyCodeObject *code)
{
- PyFrameObject *f;
-
- f = code->co_zombieframe;
+ PyFrameObject *f = code->co_zombieframe;
if (f != NULL) {
code->co_zombieframe = NULL;
_Py_NewReference((PyObject *)f);
@@ -803,14 +800,11 @@ frame_alloc(PyCodeObject *code)
_Py_NewReference((PyObject *)f);
}
- f->f_code = code;
extras = code->co_nlocals + ncells + nfrees;
f->f_valuestack = f->f_localsplus + extras;
- for (Py_ssize_t i=0; i<extras; i++) {
+ for (Py_ssize_t i=0; i < extras; i++) {
f->f_localsplus[i] = NULL;
}
- f->f_locals = NULL;
- f->f_trace = NULL;
return f;
}
@@ -818,42 +812,33 @@ frame_alloc(PyCodeObject *code)
PyFrameObject* _Py_HOT_FUNCTION
_PyFrame_New_NoTrack(PyThreadState *tstate, PyFrameConstructor *con, PyObject *locals)
{
-#ifdef Py_DEBUG
- if (con == NULL || con->fc_code == NULL ||
- (locals != NULL && !PyMapping_Check(locals))) {
- PyErr_BadInternalCall();
- return NULL;
- }
-#endif
-
- PyFrameObject *back = tstate->frame;
+ assert(con != NULL);
+ assert(con->fc_globals != NULL);
+ assert(con->fc_builtins != NULL);
+ assert(con->fc_code != NULL);
+ assert(locals == NULL || PyMapping_Check(locals));
PyFrameObject *f = frame_alloc((PyCodeObject *)con->fc_code);
if (f == NULL) {
return NULL;
}
+ f->f_back = (PyFrameObject*)Py_XNewRef(tstate->frame);
+ f->f_code = (PyCodeObject *)Py_NewRef(con->fc_code);
+ f->f_builtins = Py_NewRef(con->fc_builtins);
+ f->f_globals = Py_NewRef(con->fc_globals);
+ f->f_locals = Py_XNewRef(locals);
+ // f_valuestack initialized by frame_alloc()
+ f->f_trace = NULL;
f->f_stackdepth = 0;
- Py_INCREF(con->fc_builtins);
- f->f_builtins = con->fc_builtins;
- Py_XINCREF(back);
- f->f_back = back;
- 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_trace_lines = 1;
+ f->f_trace_opcodes = 0;
+ f->f_gen = NULL;
f->f_lasti = -1;
f->f_lineno = 0;
f->f_iblock = 0;
f->f_state = FRAME_CREATED;
- f->f_gen = NULL;
- f->f_trace_opcodes = 0;
- f->f_trace_lines = 1;
-
- assert(f->f_code != NULL);
-
+ // f_blockstack and f_localsplus initialized by frame_alloc()
return f;
}
@@ -863,6 +848,9 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
PyObject *globals, PyObject *locals)
{
PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals);
+ if (builtins == NULL) {
+ return NULL;
+ }
PyFrameConstructor desc = {
.fc_globals = globals,
.fc_builtins = builtins,
@@ -875,8 +863,9 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
};
PyFrameObject *f = _PyFrame_New_NoTrack(tstate, &desc, locals);
Py_DECREF(builtins);
- if (f)
+ if (f) {
_PyObject_GC_TRACK(f);
+ }
return f;
}
@@ -1173,27 +1162,30 @@ PyFrame_GetBack(PyFrameObject *frame)
return back;
}
-PyObject *_PyEval_BuiltinsFromGlobals(PyObject *globals) {
+PyObject*
+_PyEval_BuiltinsFromGlobals(PyObject *globals)
+{
PyObject *builtins = _PyDict_GetItemIdWithError(globals, &PyId___builtins__);
if (builtins) {
if (PyModule_Check(builtins)) {
builtins = PyModule_GetDict(builtins);
assert(builtins != NULL);
}
+ return Py_NewRef(builtins);
}
+
+ if (PyErr_Occurred()) {
+ return NULL;
+ }
+
+ /* No builtins! Make up a minimal one. Give them 'None', at least. */
+ builtins = PyDict_New();
if (builtins == NULL) {
- if (PyErr_Occurred()) {
- return NULL;
- }
- /* No builtins! Make up a minimal one
- Give them 'None', at least. */
- builtins = PyDict_New();
- if (builtins == NULL ||
- PyDict_SetItemString(
- builtins, "None", Py_None) < 0)
- return NULL;
+ return NULL;
+ }
+ if (PyDict_SetItemString(builtins, "None", Py_None) < 0) {
+ Py_DECREF(builtins);
+ return NULL;
}
- else
- Py_INCREF(builtins);
return builtins;
}
diff --git a/Objects/funcobject.c b/Objects/funcobject.c
index 523930da8dc62..4b92f6c0342d9 100644
--- a/Objects/funcobject.c
+++ b/Objects/funcobject.c
@@ -2,83 +2,90 @@
/* Function object implementation */
#include "Python.h"
-#include "pycore_object.h"
-#include "frameobject.h"
-#include "code.h"
+#include "pycore_ceval.h" // _PyEval_BuiltinsFromGlobals()
+#include "pycore_object.h" // _PyObject_GC_UNTRACK()
#include "structmember.h" // PyMemberDef
PyObject *
PyFunction_NewWithQualName(PyObject *code, PyObject *globals, PyObject *qualname)
{
- PyFunctionObject *op;
- PyObject *doc, *consts, *module;
- static PyObject *__name__ = NULL;
+ assert(globals != NULL);
+ assert(PyDict_Check(globals));
+ Py_INCREF(globals);
- if (__name__ == NULL) {
- __name__ = PyUnicode_InternFromString("__name__");
- if (__name__ == NULL)
- return NULL;
+ PyCodeObject *code_obj = (PyCodeObject *)code;
+ Py_INCREF(code_obj);
+
+ PyObject *name = code_obj->co_name;
+ assert(name != NULL);
+ Py_INCREF(name);
+ if (!qualname) {
+ qualname = name;
}
+ Py_INCREF(qualname);
- /* __module__: If module name is in globals, use it.
- Otherwise, use None. */
- module = PyDict_GetItemWithError(globals, __name__);
- if (module) {
- Py_INCREF(module);
+ PyObject *consts = code_obj->co_consts;
+ assert(PyTuple_Check(consts));
+ PyObject *doc;
+ if (PyTuple_Size(consts) >= 1) {
+ doc = PyTuple_GetItem(consts, 0);
+ if (!PyUnicode_Check(doc)) {
+ doc = Py_None;
+ }
}
- else if (PyErr_Occurred()) {
- return NULL;
+ else {
+ doc = Py_None;
+ }
+ Py_INCREF(doc);
+
+ // __module__: Use globals['__name__'] if it exists, or NULL.
+ _Py_IDENTIFIER(__name__);
+ PyObject *module = _PyDict_GetItemIdWithError(globals, &PyId___name__);
+ PyObject *builtins = NULL;
+ if (module == NULL && PyErr_Occurred()) {
+ goto error;
+ }
+ Py_XINCREF(module);
+
+ builtins = _PyEval_BuiltinsFromGlobals(globals);
+ if (builtins == NULL) {
+ goto error;
}
- op = PyObject_GC_New(PyFunctionObject, &PyFunction_Type);
+ PyFunctionObject *op = PyObject_GC_New(PyFunctionObject, &PyFunction_Type);
if (op == NULL) {
- Py_XDECREF(module);
- return NULL;
+ goto error;
}
/* Note: No failures from this point on, since func_dealloc() does not
expect a partially-created object. */
- op->func_weakreflist = NULL;
- Py_INCREF(code);
- op->func_code = code;
- assert(globals != NULL);
- Py_INCREF(globals);
op->func_globals = globals;
- PyObject *builtins = _PyEval_BuiltinsFromGlobals(globals);
- if (builtins == NULL) {
- return NULL;
- }
op->func_builtins = builtins;
- op->func_name = ((PyCodeObject *)code)->co_name;
- Py_INCREF(op->func_name);
- op->func_defaults = NULL; /* No default arguments */
- op->func_kwdefaults = NULL; /* No keyword only defaults */
+ op->func_name = name;
+ op->func_qualname = qualname;
+ op->func_code = (PyObject*)code_obj;
+ op->func_defaults = NULL; // No default positional arguments
+ op->func_kwdefaults = NULL; // No default keyword arguments
op->func_closure = NULL;
- op->vectorcall = _PyFunction_Vectorcall;
- op->func_module = module;
-
- consts = ((PyCodeObject *)code)->co_consts;
- if (PyTuple_Size(consts) >= 1) {
- doc = PyTuple_GetItem(consts, 0);
- if (!PyUnicode_Check(doc))
- doc = Py_None;
- }
- else
- doc = Py_None;
- Py_INCREF(doc);
op->func_doc = doc;
-
op->func_dict = NULL;
+ op->func_weakreflist = NULL;
+ op->func_module = module;
op->func_annotations = NULL;
-
- if (qualname)
- op->func_qualname = qualname;
- else
- op->func_qualname = op->func_name;
- Py_INCREF(op->func_qualname);
+ op->vectorcall = _PyFunction_Vectorcall;
_PyObject_GC_TRACK(op);
return (PyObject *)op;
+
+error:
+ Py_DECREF(globals);
+ Py_DECREF(code_obj);
+ Py_DECREF(name);
+ Py_DECREF(qualname);
+ Py_DECREF(doc);
+ Py_XDECREF(module);
+ Py_XDECREF(builtins);
+ return NULL;
}
PyObject *
diff --git a/Python/ceval.c b/Python/ceval.c
index 9e4c2666ac6f7..0b7400359e001 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -908,7 +908,7 @@ PyEval_EvalCode(PyObject *co, PyObject *globals, PyObject *locals)
.fc_closure = NULL
};
PyThreadState *tstate = PyThreadState_GET();
- PyObject *res =_PyEval_Vector(tstate, &desc, locals, NULL, 0, NULL);
+ PyObject *res = _PyEval_Vector(tstate, &desc, locals, NULL, 0, NULL);
Py_DECREF(builtins);
return res;
}
@@ -4800,8 +4800,8 @@ PyEval_EvalCodeEx(PyObject *_co, PyObject *globals, PyObject *locals,
};
PyThreadState *tstate = _PyThreadState_GET();
res = _PyEval_Vector(tstate, &constr, locals,
- allargs, argcount,
- kwnames);
+ allargs, argcount,
+ kwnames);
if (kwcount) {
Py_DECREF(kwnames);
PyMem_Free(newargs);
More information about the Python-checkins
mailing list