Skip to content

Commit a0a2d2f

Browse files
harjothkharaclaude
andcommitted
gh-151321: Fix opcode metadata flags for LOAD_DEREF and LOAD_CLOSURE
LOAD_DEREF and LOAD_CLOSURE were missing HAS_FREE_FLAG in the generated opcode metadata, and both also carried HAS_LOCAL_FLAG, so dis.hasfree no longer reported them while dis.haslocal wrongly included them. This regressed the documented dis.hasfree / dis.haslocal contract: LOAD_CLOSURE broke in 3.13 (when it became a pseudo-op inheriting LOAD_FAST's flags) and LOAD_DEREF broke in 3.14 (when it started using _PyCell_GetStackRef, which the cases generator's has_free heuristic did not recognize). Teach the cases generator to treat _PyCell_GetStackRef as a free-variable access, and give the LOAD_CLOSURE pseudo an explicit HAS_FREE flag. A pseudo that explicitly accesses a free variable now suppresses the inherited HAS_LOCAL, mirroring the existing "uses_locals and not has_free" rule for real instructions, so HAS_FREE and HAS_LOCAL are never both set. The change is to introspection metadata only; the generated interpreter and optimizer code are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 80f9467 commit a0a2d2f

6 files changed

Lines changed: 32 additions & 5 deletions

File tree

Include/internal/pycore_opcode_metadata.h

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_uop_metadata.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Lib/test/test__opcode.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,21 @@ def check_function(self, func, expected):
6161
check_function(self, _opcode.has_local, dis.haslocal)
6262
check_function(self, _opcode.has_exc, dis.hasexc)
6363

64+
def test_hasfree_membership(self):
65+
# gh-151321: opcodes that access a free (closure) variable must be
66+
# reported by dis.hasfree and not by dis.haslocal.
67+
free_ops = {
68+
'DELETE_DEREF',
69+
'LOAD_CLOSURE',
70+
'LOAD_DEREF',
71+
'LOAD_FROM_DICT_OR_DEREF',
72+
'MAKE_CELL',
73+
'STORE_DEREF',
74+
}
75+
self.assertEqual(free_ops, {dis.opname[op] for op in dis.hasfree})
76+
self.assertNotIn(dis.opmap['LOAD_DEREF'], dis.haslocal)
77+
self.assertNotIn(dis.opmap['LOAD_CLOSURE'], dis.haslocal)
78+
6479

6580
class StackEffectTests(unittest.TestCase):
6681
def test_stack_effect(self):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix the opcode metadata flags for :opcode:`LOAD_DEREF` and :opcode:`LOAD_CLOSURE`
2+
so that they are reported by :data:`dis.hasfree` again, and are no longer
3+
incorrectly reported by :data:`dis.haslocal`.

Python/bytecodes.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ dummy_func(
264264
_CHECK_PERIODIC_IF_NOT_YIELD_FROM +
265265
_MONITOR_RESUME;
266266

267-
pseudo(LOAD_CLOSURE, (-- unused)) = {
267+
pseudo(LOAD_CLOSURE, (-- unused), (HAS_FREE)) = {
268268
LOAD_FAST,
269269
};
270270

Tools/cases_generator/analyzer.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,15 @@ def dump(self, indent: str) -> None:
313313

314314
@property
315315
def properties(self) -> Properties:
316-
return Properties.from_list([i.properties for i in self.targets])
316+
properties = Properties.from_list([i.properties for i in self.targets])
317+
if "HAS_FREE" in self.flags:
318+
# A pseudo that explicitly accesses a free (closure) variable is
319+
# not a local access, even though it may target a local-accessing
320+
# instruction such as LOAD_FAST. This mirrors the
321+
# ``uses_locals and not has_free`` rule for real instructions so
322+
# that HAS_FREE and HAS_LOCAL are not both set (gh-151321).
323+
properties.uses_locals = False
324+
return properties
317325

318326

319327
@dataclass
@@ -970,6 +978,7 @@ def compute_properties(op: parser.CodeDef) -> Properties:
970978
or variable_used(op, "PyCell_GetRef")
971979
or variable_used(op, "PyCell_SetTakeRef")
972980
or variable_used(op, "PyCell_SwapTakeRef")
981+
or variable_used(op, "_PyCell_GetStackRef")
973982
)
974983
deopts_if = variable_used(op, "DEOPT_IF")
975984
exits_if = variable_used(op, "EXIT_IF")

0 commit comments

Comments
 (0)