Skip to content

gh-126835: Remove unused docstring const #130016

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion Include/internal/pycore_flowgraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ void _PyCfgBuilder_Free(struct _PyCfgBuilder *g);
int _PyCfgBuilder_CheckSize(struct _PyCfgBuilder* g);

int _PyCfg_OptimizeCodeUnit(struct _PyCfgBuilder *g, PyObject *consts, PyObject *const_cache,
int nlocals, int nparams, int firstlineno);
int nlocals, int nparams, int firstlineno, unsigned has_docstring);

struct _PyCfgBuilder* _PyCfg_FromInstructionSequence(_PyInstructionSequence *seq);
int _PyCfg_ToInstructionSequence(struct _PyCfgBuilder *g, _PyInstructionSequence *seq);
Expand Down
8 changes: 4 additions & 4 deletions Lib/test/test_code.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
freevars: ('x',)
nlocals: 1
flags: 19
consts: ('None',)
consts: ()

>>> def h(x, y):
... a = x + y
Expand All @@ -50,7 +50,7 @@
freevars: ()
nlocals: 5
flags: 3
consts: ('None',)
consts: ()

>>> def attrs(obj):
... print(obj.attr1)
Expand Down Expand Up @@ -104,7 +104,7 @@
freevars: ()
nlocals: 3
flags: 3
consts: ('None',)
consts: ()

>>> def posonly_args(a,b,/,c):
... return a,b,c
Expand All @@ -121,7 +121,7 @@
freevars: ()
nlocals: 3
flags: 3
consts: ('None',)
consts: ()

>>> def has_docstring(x: str):
... 'This is a one-line doc string'
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ def f():
return "unused"

self.assertEqual(f.__code__.co_consts,
(True, "used"))
("used", ))

@support.cpython_only
def test_remove_unused_consts_extended_args(self):
Expand Down
9 changes: 3 additions & 6 deletions Lib/test/test_dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -1425,9 +1425,9 @@ def get_disassembly(self, func, lasti=-1, wrapper=True, **kwargs):


if dis.code_info.__doc__ is None:
code_info_consts = "0: None"
code_info_consts = ""
else:
code_info_consts = "0: 'Formatted details of methods, functions, or code.'"
code_info_consts = "Constants:\n 0: 'Formatted details of methods, functions, or code.'"

code_info_code_info = f"""\
Name: code_info
Expand All @@ -1438,8 +1438,7 @@ def get_disassembly(self, func, lasti=-1, wrapper=True, **kwargs):
Number of locals: 1
Stack size: \\d+
Flags: OPTIMIZED, NEWLOCALS, HAS_DOCSTRING
Constants:
{code_info_consts}
{code_info_consts}
Names:
0: _format_code_info
1: _get_code_object
Expand Down Expand Up @@ -1546,8 +1545,6 @@ def f(c=c):
Number of locals: 0
Stack size: \\d+
Flags: 0x0
Constants:
0: None
Names:
0: x"""

Expand Down
22 changes: 11 additions & 11 deletions Lib/test/test_peepholer.py
Original file line number Diff line number Diff line change
Expand Up @@ -1057,16 +1057,16 @@ def test_conditional_jump_forward_non_const_condition(self):
expected_insts = [
('LOAD_NAME', 1, 11),
('POP_JUMP_IF_TRUE', lbl := self.Label(), 12),
('LOAD_CONST', 1, 13),
('LOAD_CONST', 0, 13),
('RETURN_VALUE', None, 13),
lbl,
('LOAD_CONST', 2, 14),
('LOAD_CONST', 1, 14),
('RETURN_VALUE', None, 14),
]
self.cfg_optimization_test(insts,
expected_insts,
consts=[0, 1, 2, 3, 4],
expected_consts=[0, 2, 3])
expected_consts=[2, 3])

def test_list_exceeding_stack_use_guideline(self):
def f():
Expand Down Expand Up @@ -1107,10 +1107,10 @@ def test_multiple_foldings(self):
('RETURN_VALUE', None, 0)
]
after = [
('LOAD_CONST', 1, 0),
('LOAD_CONST', 0, 0),
('RETURN_VALUE', None, 0)
]
self.cfg_optimization_test(before, after, consts=[], expected_consts=[(2,), (1, 2)])
self.cfg_optimization_test(before, after, consts=[], expected_consts=[(1, 2)])

def test_build_empty_tuple(self):
before = [
Expand Down Expand Up @@ -1277,13 +1277,13 @@ def test_conditional_jump_forward_const_condition(self):
expected_insts = [
('NOP', None, 11),
('NOP', None, 12),
('LOAD_CONST', 1, 14),
('LOAD_CONST', 0, 14),
('RETURN_VALUE', None, 14),
]
self.cfg_optimization_test(insts,
expected_insts,
consts=[0, 1, 2, 3, 4],
expected_consts=[0, 3])
expected_consts=[3])

def test_conditional_jump_backward_non_const_condition(self):
insts = [
Expand Down Expand Up @@ -1322,18 +1322,18 @@ def test_except_handler_label(self):
insts = [
('SETUP_FINALLY', handler := self.Label(), 10),
('POP_BLOCK', None, -1),
('LOAD_CONST', 1, 11),
('LOAD_CONST', 0, 11),
('RETURN_VALUE', None, 11),
handler,
('LOAD_CONST', 2, 12),
('LOAD_CONST', 1, 12),
('RETURN_VALUE', None, 12),
]
expected_insts = [
('SETUP_FINALLY', handler := self.Label(), 10),
('LOAD_CONST', 1, 11),
('LOAD_CONST', 0, 11),
('RETURN_VALUE', None, 11),
handler,
('LOAD_CONST', 2, 12),
('LOAD_CONST', 1, 12),
('RETURN_VALUE', None, 12),
]
self.cfg_optimization_test(insts, expected_insts, consts=list(range(5)))
Expand Down
2 changes: 1 addition & 1 deletion Python/compile.c
Original file line number Diff line number Diff line change
Expand Up @@ -1325,7 +1325,7 @@ optimize_and_assemble_code_unit(struct compiler_unit *u, PyObject *const_cache,
assert(u->u_metadata.u_firstlineno);

if (_PyCfg_OptimizeCodeUnit(g, consts, const_cache, nlocals,
nparams, u->u_metadata.u_firstlineno) < 0) {
nparams, u->u_metadata.u_firstlineno, u->u_ste->ste_has_docstring) < 0) {
goto error;
}

Expand Down
16 changes: 10 additions & 6 deletions Python/flowgraph.c
Original file line number Diff line number Diff line change
Expand Up @@ -2281,7 +2281,7 @@ fast_scan_many_locals(basicblock *entryblock, int nlocals)
}

static int
remove_unused_consts(basicblock *entryblock, PyObject *consts)
remove_unused_consts(basicblock *entryblock, PyObject *consts, unsigned has_docstring)
{
assert(PyList_CheckExact(consts));
Py_ssize_t nconsts = PyList_GET_SIZE(consts);
Expand All @@ -2297,11 +2297,15 @@ remove_unused_consts(basicblock *entryblock, PyObject *consts)
if (index_map == NULL) {
goto end;
}
for (Py_ssize_t i = 1; i < nconsts; i++) {
for (Py_ssize_t i = 0; i < nconsts; i++) {
index_map[i] = -1;
}
// The first constant may be docstring; keep it always.
index_map[0] = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe update the comment above? It's only kept when we have the docstring now

if (has_docstring) {
assert(PyList_Size(consts) >= 1);
assert(PyUnicode_CheckExact(PyList_GET_ITEM(consts, 0)));
index_map[0] = 0;
}

/* mark used consts */
for (basicblock *b = entryblock; b != NULL; b = b->b_next) {
Expand Down Expand Up @@ -2764,7 +2768,7 @@ resolve_line_numbers(cfg_builder *g, int firstlineno)

int
_PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache,
int nlocals, int nparams, int firstlineno)
int nlocals, int nparams, int firstlineno, unsigned has_docstring)
{
assert(cfg_builder_check(g));
/** Preprocessing **/
Expand All @@ -2775,7 +2779,7 @@ _PyCfg_OptimizeCodeUnit(cfg_builder *g, PyObject *consts, PyObject *const_cache,

/** Optimization **/
RETURN_IF_ERROR(optimize_cfg(g, consts, const_cache, firstlineno));
RETURN_IF_ERROR(remove_unused_consts(g->g_entryblock, consts));
RETURN_IF_ERROR(remove_unused_consts(g->g_entryblock, consts, has_docstring));
RETURN_IF_ERROR(
add_checks_for_loads_of_uninitialized_variables(
g->g_entryblock, nlocals, nparams));
Expand Down Expand Up @@ -3191,7 +3195,7 @@ _PyCompile_OptimizeCfg(PyObject *seq, PyObject *consts, int nlocals)
}
int nparams = 0, firstlineno = 1;
if (_PyCfg_OptimizeCodeUnit(g, consts, const_cache, nlocals,
nparams, firstlineno) < 0) {
nparams, firstlineno, 0) < 0) {
goto error;
}
res = cfg_to_instruction_sequence(g);
Expand Down
Loading