Skip to content
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

GH-100719: Remove the co_nplaincellvars field from code objects. #100721

Merged
merged 4 commits into from
Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Include/cpython/code.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ typedef struct {
int co_nlocalsplus; /* number of local + cell + free variables */ \
int co_framesize; /* Size of frame in words */ \
int co_nlocals; /* number of local variables */ \
int co_nplaincellvars; /* number of non-arg cell variables */ \
int co_ncellvars; /* total number of cell variables */ \
int co_nfreevars; /* number of free variables */ \
uint32_t co_version; /* version number */ \
Expand Down Expand Up @@ -157,6 +156,11 @@ static inline Py_ssize_t PyCode_GetNumFree(PyCodeObject *op) {
return op->co_nfreevars;
}

static inline int PyCode_GetFirstFree(PyCodeObject *op) {
assert(PyCode_Check(op));
return op->co_nlocalsplus - op->co_nfreevars;
}

#define _PyCode_CODE(CO) _Py_RVALUE((_Py_CODEUNIT *)(CO)->co_code_adaptive)
#define _PyCode_NBYTES(CO) (Py_SIZE(CO) * (Py_ssize_t)sizeof(_Py_CODEUNIT))

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Removed the co_nplaincellvars field from the code object, as it is
redundant.
14 changes: 4 additions & 10 deletions Objects/codeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,10 @@ _Py_set_localsplus_info(int offset, PyObject *name, _PyLocals_Kind kind,

static void
get_localsplus_counts(PyObject *names, PyObject *kinds,
int *pnlocals, int *pnplaincellvars, int *pncellvars,
int *pnlocals, int *pncellvars,
int *pnfreevars)
{
int nlocals = 0;
int nplaincellvars = 0;
int ncellvars = 0;
int nfreevars = 0;
Py_ssize_t nlocalsplus = PyTuple_GET_SIZE(names);
Expand All @@ -265,7 +264,6 @@ get_localsplus_counts(PyObject *names, PyObject *kinds,
}
else if (kind & CO_FAST_CELL) {
ncellvars += 1;
nplaincellvars += 1;
}
else if (kind & CO_FAST_FREE) {
nfreevars += 1;
Expand All @@ -274,9 +272,6 @@ get_localsplus_counts(PyObject *names, PyObject *kinds,
if (pnlocals != NULL) {
*pnlocals = nlocals;
}
if (pnplaincellvars != NULL) {
*pnplaincellvars = nplaincellvars;
}
if (pncellvars != NULL) {
*pncellvars = ncellvars;
}
Expand Down Expand Up @@ -351,7 +346,7 @@ _PyCode_Validate(struct _PyCodeConstructor *con)
* here to avoid the possibility of overflow (however remote). */
int nlocals;
get_localsplus_counts(con->localsplusnames, con->localspluskinds,
&nlocals, NULL, NULL, NULL);
&nlocals, NULL, NULL);
int nplainlocals = nlocals -
con->argcount -
con->kwonlyargcount -
Expand All @@ -371,9 +366,9 @@ static void
init_code(PyCodeObject *co, struct _PyCodeConstructor *con)
{
int nlocalsplus = (int)PyTuple_GET_SIZE(con->localsplusnames);
int nlocals, nplaincellvars, ncellvars, nfreevars;
int nlocals, ncellvars, nfreevars;
get_localsplus_counts(con->localsplusnames, con->localspluskinds,
&nlocals, &nplaincellvars, &ncellvars, &nfreevars);
&nlocals, &ncellvars, &nfreevars);

co->co_filename = Py_NewRef(con->filename);
co->co_name = Py_NewRef(con->name);
Expand Down Expand Up @@ -401,7 +396,6 @@ init_code(PyCodeObject *co, struct _PyCodeConstructor *con)
co->co_nlocalsplus = nlocalsplus;
co->co_nlocals = nlocals;
co->co_framesize = nlocalsplus + con->stacksize + FRAME_SPECIALS_SIZE;
co->co_nplaincellvars = nplaincellvars;
co->co_ncellvars = ncellvars;
co->co_nfreevars = nfreevars;
co->co_version = _Py_next_func_version;
Expand Down
2 changes: 1 addition & 1 deletion Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,7 @@ frame_init_get_vars(_PyInterpreterFrame *frame)

/* Free vars have not been initialized -- Do that */
PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
int offset = co->co_nlocals + co->co_nplaincellvars;
int offset = PyCode_GetFirstFree(co);
Copy link
Member

Choose a reason for hiding this comment

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

Py_ssize_t?

for (int i = 0; i < co->co_nfreevars; ++i) {
PyObject *o = PyTuple_GET_ITEM(closure, i);
frame->localsplus[offset + i] = Py_NewRef(o);
Expand Down
2 changes: 1 addition & 1 deletion Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -9514,7 +9514,7 @@ super_init_without_args(_PyInterpreterFrame *cframe, PyCodeObject *co,

// Look for __class__ in the free vars.
PyTypeObject *type = NULL;
int i = co->co_nlocals + co->co_nplaincellvars;
int i = PyCode_GetFirstFree(co);
Copy link
Member

Choose a reason for hiding this comment

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

Py_ssize_t?

Copy link
Member Author

@markshannon markshannon Jan 4, 2023

Choose a reason for hiding this comment

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

This should be an int. I'll change PyCode_GetFirstFree.

Copy link

Choose a reason for hiding this comment

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

@markshannon why should it be int?

for (; i < co->co_nlocalsplus; i++) {
assert((_PyLocals_GetKind(co->co_localspluskinds, i) & CO_FAST_FREE) != 0);
PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
Expand Down
2 changes: 1 addition & 1 deletion Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -1357,8 +1357,8 @@ dummy_func(
PyCodeObject *co = frame->f_code;
assert(PyFunction_Check(frame->f_funcobj));
PyObject *closure = ((PyFunctionObject *)frame->f_funcobj)->func_closure;
int offset = co->co_nlocals + co->co_nplaincellvars;
assert(oparg == co->co_nfreevars);
int offset = co->co_nlocalsplus - oparg;
for (int i = 0; i < oparg; ++i) {
PyObject *o = PyTuple_GET_ITEM(closure, i);
frame->localsplus[offset + i] = Py_NewRef(o);
Expand Down
2 changes: 1 addition & 1 deletion Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -3417,7 +3417,7 @@ format_exc_unbound(PyThreadState *tstate, PyCodeObject *co, int oparg)
if (_PyErr_Occurred(tstate))
return;
name = PyTuple_GET_ITEM(co->co_localsplusnames, oparg);
if (oparg < co->co_nplaincellvars + co->co_nlocals) {
if (oparg < PyCode_GetFirstFree(co)) {
format_exc_check_arg(tstate, PyExc_UnboundLocalError,
UNBOUNDLOCAL_ERROR_MSG, name);
} else {
Expand Down
2 changes: 1 addition & 1 deletion Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -2260,7 +2260,7 @@ compiler_make_closure(struct compiler *c, location loc,
qualname = co->co_name;

if (co->co_nfreevars) {
int i = co->co_nlocals + co->co_nplaincellvars;
int i = PyCode_GetFirstFree(co);
Copy link
Member

Choose a reason for hiding this comment

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

Py_ssize_t?

for (; i < co->co_nlocalsplus; ++i) {
/* Bypass com_addop_varname because it will generate
LOAD_DEREF but LOAD_CLOSURE is needed.
Expand Down
2 changes: 1 addition & 1 deletion Python/generated_cases.c.h

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

8 changes: 2 additions & 6 deletions Tools/build/deepfreeze.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ def get_localsplus_counts(code: types.CodeType,
names: Tuple[str, ...],
kinds: bytes) -> Tuple[int, int, int, int]:
nlocals = 0
nplaincellvars = 0
ncellvars = 0
nfreevars = 0
assert len(names) == len(kinds)
Expand All @@ -72,15 +71,13 @@ def get_localsplus_counts(code: types.CodeType,
ncellvars += 1
elif kind & CO_FAST_CELL:
ncellvars += 1
nplaincellvars += 1
elif kind & CO_FAST_FREE:
nfreevars += 1
assert nlocals == len(code.co_varnames) == code.co_nlocals, \
(nlocals, len(code.co_varnames), code.co_nlocals)
assert ncellvars == len(code.co_cellvars)
assert nfreevars == len(code.co_freevars)
assert len(names) == nlocals + nplaincellvars + nfreevars
return nlocals, nplaincellvars, ncellvars, nfreevars
return nlocals, ncellvars, nfreevars


PyUnicode_1BYTE_KIND = 1
Expand Down Expand Up @@ -243,7 +240,7 @@ def generate_code(self, name: str, code: types.CodeType) -> str:
co_localsplusnames = self.generate(name + "_localsplusnames", localsplusnames)
co_localspluskinds = self.generate(name + "_localspluskinds", localspluskinds)
# Derived values
nlocals, nplaincellvars, ncellvars, nfreevars = \
nlocals, ncellvars, nfreevars = \
get_localsplus_counts(code, localsplusnames, localspluskinds)
co_code_adaptive = make_string_literal(code.co_code)
self.write("static")
Expand All @@ -267,7 +264,6 @@ def generate_code(self, name: str, code: types.CodeType) -> str:
self.field(code, "co_firstlineno")
self.write(f".co_nlocalsplus = {len(localsplusnames)},")
self.field(code, "co_nlocals")
self.write(f".co_nplaincellvars = {nplaincellvars},")
self.write(f".co_ncellvars = {ncellvars},")
self.write(f".co_nfreevars = {nfreevars},")
self.write(f".co_version = {next_code_version},")
Expand Down