Skip to content

bpo-43693: Turn localspluskinds into an object #26749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 21, 2021
Merged
5 changes: 1 addition & 4 deletions Include/cpython/code.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ typedef uint16_t _Py_CODEUNIT;
typedef struct _PyOpcache _PyOpcache;


typedef unsigned char _PyLocalsPlusKind;
typedef _PyLocalsPlusKind *_PyLocalsPlusKinds;

/* Bytecode object */
struct PyCodeObject {
PyObject_HEAD
Expand Down Expand Up @@ -75,7 +72,7 @@ struct PyCodeObject {
int co_firstlineno; /* first source line number */
PyObject *co_code; /* instruction opcodes */
PyObject *co_localsplusnames; /* tuple mapping offsets to names */
_PyLocalsPlusKinds co_localspluskinds; /* array mapping to local kinds */
PyObject *co_localspluskinds; /* Bytees mapping to local kinds (one byte per variable) */
PyObject *co_filename; /* unicode (where it was loaded from) */
PyObject *co_name; /* unicode (name, for reference) */
PyObject *co_linetable; /* string (encoding addr<->lineno mapping) See
Expand Down
39 changes: 17 additions & 22 deletions Include/internal/pycore_code.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,39 +180,34 @@ extern Py_ssize_t _Py_QuickenedCount;
* "free" kind is mutually exclusive with both.
*/

// For now _PyLocalsPlusKind and _PyLocalsPlusKinds are defined
// in Include/cpython/code.h.
/* Note that these all fit within _PyLocalsPlusKind, as do combinations. */
// Note that these all fit within a byte, as do combinations.
// Later, we will use the smaller numbers to differentiate the different
// kinds of locals (e.g. pos-only arg, varkwargs, local-only).
#define CO_FAST_LOCAL 0x20
#define CO_FAST_CELL 0x40
#define CO_FAST_FREE 0x80

static inline int
_PyCode_InitLocalsPlusKinds(int num, _PyLocalsPlusKinds *pkinds)
typedef unsigned char _PyLocals_Kind;

static inline _PyLocals_Kind
_PyLocals_GetKind(PyObject *kinds, int i)
{
if (num == 0) {
*pkinds = NULL;
return 0;
}
_PyLocalsPlusKinds kinds = PyMem_NEW(_PyLocalsPlusKind, num);
if (kinds == NULL) {
PyErr_NoMemory();
return -1;
}
*pkinds = kinds;
return 0;
assert(PyBytes_Check(kinds));
assert(0 <= i && i < PyBytes_GET_SIZE(kinds));
char *ptr = PyBytes_AS_STRING(kinds);
return (_PyLocals_Kind)(ptr[i]);
}

static inline void
_PyCode_ClearLocalsPlusKinds(_PyLocalsPlusKinds kinds)
_PyLocals_SetKind(PyObject *kinds, int i, _PyLocals_Kind kind)
{
if (kinds != NULL) {
PyMem_Free(kinds);
}
assert(PyBytes_Check(kinds));
assert(0 <= i && i < PyBytes_GET_SIZE(kinds));
char *ptr = PyBytes_AS_STRING(kinds);
ptr[i] = (char) kind;
}


struct _PyCodeConstructor {
/* metadata */
PyObject *filename;
Expand All @@ -229,8 +224,8 @@ struct _PyCodeConstructor {
PyObject *names;

/* mapping frame offsets to information */
PyObject *localsplusnames;
_PyLocalsPlusKinds localspluskinds;
PyObject *localsplusnames; // Tuple of strings
PyObject *localspluskinds; // Bytes object, one byte per variable

/* args (within varnames) */
int argcount;
Expand Down
3 changes: 2 additions & 1 deletion Lib/importlib/_bootstrap_external.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.11a1 3454 (compute cell offsets relative to locals bpo-43693)
# Python 3.11a1 3455 (add MAKE_CELL bpo-43693)
# Python 3.11a1 3456 (interleave cell args bpo-43693)
# Python 3.11a1 3457 (Change localsplus to a bytes object bpo-43693)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we're not using up too many magic numbers during pre-alpha...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are our options?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are our options?

Going back to the first magic number for 3.11a1. Everything is for the same bpo issue it seems.


#
# MAGIC must change whenever the bytecode emitted by the compiler may no
Expand All @@ -368,7 +369,7 @@ def _write_atomic(path, data, mode=0o666):
# Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
# in PC/launcher.c must also be updated.

MAGIC_NUMBER = (3456).to_bytes(2, 'little') + b'\r\n'
MAGIC_NUMBER = (3457).to_bytes(2, 'little') + b'\r\n'
_RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c

_PYCACHE = '__pycache__'
Expand Down
40 changes: 21 additions & 19 deletions Objects/codeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,16 +156,16 @@ validate_and_copy_tuple(PyObject *tup)

// This is also used in compile.c.
void
_Py_set_localsplus_info(int offset, PyObject *name, _PyLocalsPlusKind kind,
PyObject *names, _PyLocalsPlusKinds kinds)
_Py_set_localsplus_info(int offset, PyObject *name, _PyLocals_Kind kind,
PyObject *names, PyObject *kinds)
{
Py_INCREF(name);
PyTuple_SET_ITEM(names, offset, name);
kinds[offset] = kind;
_PyLocals_SetKind(kinds, offset, kind);
}

static void
get_localsplus_counts(PyObject *names, _PyLocalsPlusKinds kinds,
get_localsplus_counts(PyObject *names, PyObject *kinds,
int *pnlocals, int *pnplaincellvars, int *pncellvars,
int *pnfreevars)
{
Expand All @@ -175,17 +175,18 @@ get_localsplus_counts(PyObject *names, _PyLocalsPlusKinds kinds,
int nfreevars = 0;
Py_ssize_t nlocalsplus = PyTuple_GET_SIZE(names);
for (int i = 0; i < nlocalsplus; i++) {
if (kinds[i] & CO_FAST_LOCAL) {
_PyLocals_Kind kind = _PyLocals_GetKind(kinds, i);
if (kind & CO_FAST_LOCAL) {
nlocals += 1;
if (kinds[i] & CO_FAST_CELL) {
if (kind & CO_FAST_CELL) {
ncellvars += 1;
}
}
else if (kinds[i] & CO_FAST_CELL) {
else if (kind & CO_FAST_CELL) {
ncellvars += 1;
nplaincellvars += 1;
}
else if (kinds[i] & CO_FAST_FREE) {
else if (kind & CO_FAST_FREE) {
nfreevars += 1;
}
}
Expand All @@ -204,15 +205,16 @@ get_localsplus_counts(PyObject *names, _PyLocalsPlusKinds kinds,
}

static PyObject *
get_localsplus_names(PyCodeObject *co, _PyLocalsPlusKind kind, int num)
get_localsplus_names(PyCodeObject *co, char kind, int num)
{
PyObject *names = PyTuple_New(num);
if (names == NULL) {
return NULL;
}
int index = 0;
for (int offset = 0; offset < co->co_nlocalsplus; offset++) {
if ((co->co_localspluskinds[offset] & kind) == 0) {
_PyLocals_Kind k = _PyLocals_GetKind(co->co_localspluskinds, offset);
if ((k & kind) == 0) {
continue;
}
assert(index < num);
Expand All @@ -236,8 +238,7 @@ _PyCode_Validate(struct _PyCodeConstructor *con)
con->consts == NULL || !PyTuple_Check(con->consts) ||
con->names == NULL || !PyTuple_Check(con->names) ||
con->localsplusnames == NULL || !PyTuple_Check(con->localsplusnames) ||
(PyTuple_GET_SIZE(con->localsplusnames) && con->localspluskinds == NULL) ||
(!PyTuple_GET_SIZE(con->localsplusnames) && con->localspluskinds != NULL) ||
con->localspluskinds == NULL || !PyBytes_Check(con->localspluskinds) ||
con->name == NULL || !PyUnicode_Check(con->name) ||
con->filename == NULL || !PyUnicode_Check(con->filename) ||
con->linetable == NULL || !PyBytes_Check(con->linetable) ||
Expand Down Expand Up @@ -309,7 +310,7 @@ init_code(PyCodeObject *co, struct _PyCodeConstructor *con)

Py_INCREF(con->localsplusnames);
co->co_localsplusnames = con->localsplusnames;
// We take ownership of the kinds array.
Py_INCREF(con->localspluskinds);
co->co_localspluskinds = con->localspluskinds;

co->co_argcount = con->argcount;
Expand Down Expand Up @@ -394,7 +395,7 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,
{
PyCodeObject *co = NULL;
PyObject *localsplusnames = NULL;
_PyLocalsPlusKinds localspluskinds = NULL;
PyObject *localspluskinds = NULL;

if (varnames == NULL || !PyTuple_Check(varnames) ||
cellvars == NULL || !PyTuple_Check(cellvars) ||
Expand All @@ -413,7 +414,8 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,
if (localsplusnames == NULL) {
goto error;
}
if (_PyCode_InitLocalsPlusKinds(nlocalsplus, &localspluskinds) < 0) {
localspluskinds = PyBytes_FromStringAndSize(NULL, nlocalsplus);
if (localspluskinds == NULL) {
goto error;
}
int offset = 0;
Expand All @@ -438,7 +440,8 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,
// Merge the localsplus indices.
nlocalsplus -= 1;
offset -= 1;
localspluskinds[argoffset] |= CO_FAST_CELL;
_PyLocals_Kind kind = _PyLocals_GetKind(localspluskinds, argoffset);
_PyLocals_SetKind(localspluskinds, argoffset, kind | CO_FAST_CELL);
continue;
}
_Py_set_localsplus_info(offset, name, CO_FAST_CELL,
Expand Down Expand Up @@ -495,7 +498,6 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,
goto error;
}

localspluskinds = NULL; // This keeps it from getting freed below.
Py_INCREF(varnames);
co->co_varnames = varnames;
Py_INCREF(cellvars);
Expand All @@ -505,7 +507,7 @@ PyCode_NewWithPosOnlyArgs(int argcount, int posonlyargcount, int kwonlyargcount,

error:
Py_XDECREF(localsplusnames);
_PyCode_ClearLocalsPlusKinds(localspluskinds);
Py_XDECREF(localspluskinds);
return co;
}

Expand Down Expand Up @@ -1142,7 +1144,7 @@ code_dealloc(PyCodeObject *co)
Py_XDECREF(co->co_consts);
Py_XDECREF(co->co_names);
Py_XDECREF(co->co_localsplusnames);
_PyCode_ClearLocalsPlusKinds(co->co_localspluskinds);
Py_XDECREF(co->co_localspluskinds);
Py_XDECREF(co->co_varnames);
Py_XDECREF(co->co_freevars);
Py_XDECREF(co->co_cellvars);
Expand Down
4 changes: 2 additions & 2 deletions Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ PyFrame_FastToLocalsWithError(PyFrameObject *f)
co = f->f_code;
fast = f->f_localsptr;
for (int i = 0; i < co->co_nlocalsplus; i++) {
_PyLocalsPlusKind kind = co->co_localspluskinds[i];
char kind = _PyLocals_GetKind(co->co_localspluskinds, i);

/* If the namespace is unoptimized, then one of the
following cases applies:
Expand Down Expand Up @@ -1046,7 +1046,7 @@ PyFrame_LocalsToFast(PyFrameObject *f, int clear)

PyErr_Fetch(&error_type, &error_value, &error_traceback);
for (int i = 0; i < co->co_nlocalsplus; i++) {
_PyLocalsPlusKind kind = co->co_localspluskinds[i];
unsigned char kind = _PyLocals_GetKind(co->co_localspluskinds, i);

/* Same test as in PyFrame_FastToLocals() above. */
if (kind & CO_FAST_FREE && !(co->co_flags & CO_OPTIMIZED)) {
Expand Down
4 changes: 2 additions & 2 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -8879,7 +8879,7 @@ super_init_without_args(PyFrameObject *f, PyCodeObject *co,

PyObject *firstarg = f->f_localsptr[0];
// The first argument might be a cell.
if (firstarg != NULL && (co->co_localspluskinds[0] & CO_FAST_CELL)) {
if (firstarg != NULL && (_PyLocals_GetKind(co->co_localspluskinds, 0) & CO_FAST_CELL)) {
// "firstarg" is a cell here unless (very unlikely) super()
// was called from the C-API before the first MAKE_CELL op.
if (f->f_lasti >= 0) {
Expand All @@ -8898,7 +8898,7 @@ super_init_without_args(PyFrameObject *f, PyCodeObject *co,
PyTypeObject *type = NULL;
int i = co->co_nlocals + co->co_nplaincellvars;
for (; i < co->co_nlocalsplus; i++) {
assert((co->co_localspluskinds[i] & CO_FAST_FREE) != 0);
assert((_PyLocals_GetKind(co->co_localspluskinds, i) & CO_FAST_FREE) != 0);
PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
assert(PyUnicode_Check(name));
if (_PyUnicode_EqualToASCIIId(name, &PyId___class__)) {
Expand Down
10 changes: 5 additions & 5 deletions Programs/test_frozenmain.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

29 changes: 15 additions & 14 deletions Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -7186,14 +7186,16 @@ merge_const_one(struct compiler *c, PyObject **obj)
}

// This is in codeobject.c.
extern void _Py_set_localsplus_info(int, PyObject *, _PyLocalsPlusKind,
PyObject *, _PyLocalsPlusKinds);
extern void _Py_set_localsplus_info(int, PyObject *, unsigned char,
PyObject *, PyObject *);

static void
compute_localsplus_info(struct compiler *c, int nlocalsplus,
PyObject *names, _PyLocalsPlusKinds kinds)
static PyObject *
compute_localsplus_info(struct compiler *c, PyObject *names)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems strange to pass one in and return the other. Perhaps create both in this function?

Suggested change
static PyObject *
compute_localsplus_info(struct compiler *c, PyObject *names)
static int
compute_localsplus_info(struct compiler *c, int nlocalsplus, PyObject **pnames, PyObject **pkinds)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and something like "init_localsplus_info" is probably a better name with the shift in responsibility for this function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems strange to pass one in and return the other. Perhaps create both in this function?

I find the C idiom for returning multiple values pretty awkward, so I'll just allocate both before passing them into the function.

{
assert(PyTuple_GET_SIZE(names) == nlocalsplus);
Py_ssize_t nlocalsplus = PyTuple_GET_SIZE(names);
PyObject *kinds = PyBytes_FromStringAndSize(NULL, nlocalsplus);
if (kinds == NULL)
return NULL;

PyObject *k, *v;
Py_ssize_t pos = 0;
Expand All @@ -7202,7 +7204,7 @@ compute_localsplus_info(struct compiler *c, int nlocalsplus,
assert(offset >= 0);
assert(offset < nlocalsplus);
// For now we do not distinguish arg kinds.
_PyLocalsPlusKind kind = CO_FAST_LOCAL;
_PyLocals_Kind kind = CO_FAST_LOCAL;
if (PyDict_GetItem(c->u->u_cellvars, k) != NULL) {
kind |= CO_FAST_CELL;
}
Expand Down Expand Up @@ -7234,6 +7236,8 @@ compute_localsplus_info(struct compiler *c, int nlocalsplus,
assert(offset < nlocalsplus);
_Py_set_localsplus_info(offset, k, CO_FAST_FREE, names, kinds);
}

return kinds;
}

static PyCodeObject *
Expand All @@ -7244,7 +7248,7 @@ makecode(struct compiler *c, struct assembler *a, PyObject *constslist,
PyObject *names = NULL;
PyObject *consts = NULL;
PyObject *localsplusnames = NULL;
_PyLocalsPlusKinds localspluskinds = NULL;
PyObject *localspluskinds = NULL;
PyObject *name = NULL;

names = dict_keys_inorder(c->u->u_names, 0);
Expand Down Expand Up @@ -7280,10 +7284,10 @@ makecode(struct compiler *c, struct assembler *a, PyObject *constslist,
if (localsplusnames == NULL) {
goto error;
}
if (_PyCode_InitLocalsPlusKinds(nlocalsplus, &localspluskinds) < 0) {
localspluskinds = compute_localsplus_info(c, localsplusnames);
if (localspluskinds == NULL) {
goto error;
}
compute_localsplus_info(c, nlocalsplus, localsplusnames, localspluskinds);

struct _PyCodeConstructor con = {
.filename = c->c_filename,
Expand Down Expand Up @@ -7314,7 +7318,6 @@ makecode(struct compiler *c, struct assembler *a, PyObject *constslist,
}

if (!merge_const_one(c, &localsplusnames)) {
_PyCode_ClearLocalsPlusKinds(con.localspluskinds);
goto error;
}
con.localsplusnames = localsplusnames;
Expand All @@ -7324,13 +7327,11 @@ makecode(struct compiler *c, struct assembler *a, PyObject *constslist,
goto error;
}

localspluskinds = NULL; // This keeps it from getting freed below.

error:
Py_XDECREF(names);
Py_XDECREF(consts);
Py_XDECREF(localsplusnames);
_PyCode_ClearLocalsPlusKinds(localspluskinds);
Py_XDECREF(localspluskinds);
Py_XDECREF(name);
return co;
}
Expand Down
7 changes: 4 additions & 3 deletions Python/frozen_hello.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ const unsigned char _Py_M__hello[] = {
100,1,131,1,1,0,100,2,83,0,41,3,84,122,12,72,
101,108,108,111,32,119,111,114,108,100,33,78,41,2,90,11,
105,110,105,116,105,97,108,105,122,101,100,218,5,112,114,105,
110,116,169,0,122,14,60,102,114,111,122,101,110,32,104,101,
108,108,111,62,218,8,60,109,111,100,117,108,101,62,1,0,
0,0,115,4,0,0,0,4,0,12,1,243,0,0,0,0,
110,116,169,0,243,0,0,0,0,122,14,60,102,114,111,122,
101,110,32,104,101,108,108,111,62,218,8,60,109,111,100,117,
108,101,62,1,0,0,0,115,4,0,0,0,4,0,12,1,
114,2,0,0,0,
};
Loading