From 983ae79532e7b624b8e096bb56b1b3ef17983643 Mon Sep 17 00:00:00 2001 From: PuQing Date: Sun, 22 Jun 2025 18:48:29 +0800 Subject: [PATCH 1/9] fix instructions in __annotate__ have incorrect code positions --- Python/codegen.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Python/codegen.c b/Python/codegen.c index 0023d72cd5e91d..a8744a533e63c7 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -685,13 +685,14 @@ codegen_setup_annotations_scope(compiler *c, location loc, PyObject *value_with_fake_globals = PyLong_FromLong(_Py_ANNOTATE_FORMAT_VALUE_WITH_FAKE_GLOBALS); assert(!SYMTABLE_ENTRY(c)->ste_has_docstring); _Py_DECLARE_STR(format, ".format"); - ADDOP_I(c, loc, LOAD_FAST, 0); - ADDOP_LOAD_CONST(c, loc, value_with_fake_globals); - ADDOP_I(c, loc, COMPARE_OP, (Py_GT << 5) | compare_masks[Py_GT]); + + ADDOP_I(c, NO_LOCATION, LOAD_FAST, 0); + ADDOP_LOAD_CONST(c, NO_LOCATION, value_with_fake_globals); + ADDOP_I(c, NO_LOCATION, COMPARE_OP, (Py_GT << 5) | compare_masks[Py_GT]); NEW_JUMP_TARGET_LABEL(c, body); - ADDOP_JUMP(c, loc, POP_JUMP_IF_FALSE, body); - ADDOP_I(c, loc, LOAD_COMMON_CONSTANT, CONSTANT_NOTIMPLEMENTEDERROR); - ADDOP_I(c, loc, RAISE_VARARGS, 1); + ADDOP_JUMP(c, NO_LOCATION, POP_JUMP_IF_FALSE, body); + ADDOP_I(c, NO_LOCATION, LOAD_COMMON_CONSTANT, CONSTANT_NOTIMPLEMENTEDERROR); + ADDOP_I(c, NO_LOCATION, RAISE_VARARGS, 1); USE_LABEL(c, body); return SUCCESS; } @@ -750,7 +751,7 @@ codegen_deferred_annotations_body(compiler *c, location loc, assert(PyList_CheckExact(conditional_annotation_indices)); assert(annotations_len == PyList_Size(conditional_annotation_indices)); - ADDOP_I(c, loc, BUILD_MAP, 0); // stack now contains + ADDOP_I(c, NO_LOCATION, BUILD_MAP, 0); // stack now contains for (Py_ssize_t i = 0; i < annotations_len; i++) { PyObject *ptr = PyList_GET_ITEM(deferred_anno, i); From dd3bea2b0f05ba96237e14a10aba171eede84458 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sun, 22 Jun 2025 10:47:28 +0000 Subject: [PATCH 2/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst new file mode 100644 index 00000000000000..6b1b9ddea2ab79 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst @@ -0,0 +1 @@ +Fix instructions in __annotate__ have incorrect code positions From c2f0a4d3c5c94fb865aaf4d52d8dd491dedb9c6b Mon Sep 17 00:00:00 2001 From: PuQing Date: Sun, 22 Jun 2025 20:13:25 +0800 Subject: [PATCH 3/9] Update Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst Co-authored-by: sobolevn --- .../2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst index 6b1b9ddea2ab79..dec0f3291c5125 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst @@ -1 +1 @@ -Fix instructions in __annotate__ have incorrect code positions +Fix instructions positions in :attr:`~object.__annotate__`. From b79060ef63156aff1d072dbb5fc89776ad2a5691 Mon Sep 17 00:00:00 2001 From: PuQing Date: Sun, 22 Jun 2025 23:08:09 +0800 Subject: [PATCH 4/9] add unit tests --- Lib/test/test_dis.py | 52 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 355990ed58ee09..e03924ae4d564d 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -431,6 +431,40 @@ def foo(a: int, b: str) -> str: RETURN_VALUE """ +issue_135700 = """\ +22 +333 +__dataclass_fields__: ClassVar +""" + +class _B: + 2 + c: int + +dis_issue_135700_class = """\ +Disassembly of __annotate_func__: + -- COPY_FREE_VARS 1 + + %3d RESUME 0 + LOAD_FAST_BORROW 0 (format) + LOAD_SMALL_INT 2 + COMPARE_OP 132 (>) + POP_JUMP_IF_FALSE 3 (to L1) + NOT_TAKEN + LOAD_COMMON_CONSTANT 1 (NotImplementedError) + RAISE_VARARGS 1 + L1: BUILD_MAP 0 + + %3d LOAD_DEREF 1 (__classdict__) + LOAD_FROM_DICT_OR_GLOBALS 0 (int) + COPY 2 + LOAD_CONST 1 ('c') + + %3d STORE_SUBSCR + RETURN_VALUE + +""" % (_B.__firstlineno__, _B.__firstlineno__ + 2, _B.__firstlineno__) + compound_stmt_str = """\ x = 0 while 1: @@ -1124,6 +1158,24 @@ def test_bug_46724(self): # Test that negative operargs are handled properly self.do_disassembly_test(bug46724, dis_bug46724) + def test_annotate_no_spurious_first_node_positions(self): + # Test that __annotate__ code doesn't inherit first AST node positions + annotate_code = compile(issue_135700, "", "exec").co_consts[1] + instructions = list(dis.Bytecode(annotate_code)) + + spurious_positions = 0 + for instr in instructions: + if (instr.positions and + instr.positions.lineno == 1 and + instr.positions.col_offset == 0 and + instr.positions.end_col_offset == 1): + spurious_positions += 1 + + self.assertEqual(spurious_positions, 0, + "No instructions should have first statement's position") + got = self.get_disassembly(_B, depth=1) + self.do_disassembly_compare(got, dis_issue_135700_class) + def test_kw_names(self): # Test that value is displayed for keyword argument names: self.do_disassembly_test(wrap_func_w_kwargs, dis_kw_names) From 2201864f4dbb1c5136bd864ed947c202e83bdfc9 Mon Sep 17 00:00:00 2001 From: PuQing Date: Mon, 23 Jun 2025 10:48:31 +0800 Subject: [PATCH 5/9] fix test --- Lib/test/test_dis.py | 60 ++++++++++++++++---------------------------- 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index e03924ae4d564d..1302b865c7734e 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -437,33 +437,11 @@ def foo(a: int, b: str) -> str: __dataclass_fields__: ClassVar """ +issue_135700_class = """\ class _B: 2 c: int - -dis_issue_135700_class = """\ -Disassembly of __annotate_func__: - -- COPY_FREE_VARS 1 - - %3d RESUME 0 - LOAD_FAST_BORROW 0 (format) - LOAD_SMALL_INT 2 - COMPARE_OP 132 (>) - POP_JUMP_IF_FALSE 3 (to L1) - NOT_TAKEN - LOAD_COMMON_CONSTANT 1 (NotImplementedError) - RAISE_VARARGS 1 - L1: BUILD_MAP 0 - - %3d LOAD_DEREF 1 (__classdict__) - LOAD_FROM_DICT_OR_GLOBALS 0 (int) - COPY 2 - LOAD_CONST 1 ('c') - - %3d STORE_SUBSCR - RETURN_VALUE - -""" % (_B.__firstlineno__, _B.__firstlineno__ + 2, _B.__firstlineno__) +""" compound_stmt_str = """\ x = 0 @@ -1160,21 +1138,25 @@ def test_bug_46724(self): def test_annotate_no_spurious_first_node_positions(self): # Test that __annotate__ code doesn't inherit first AST node positions - annotate_code = compile(issue_135700, "", "exec").co_consts[1] - instructions = list(dis.Bytecode(annotate_code)) - - spurious_positions = 0 - for instr in instructions: - if (instr.positions and - instr.positions.lineno == 1 and - instr.positions.col_offset == 0 and - instr.positions.end_col_offset == 1): - spurious_positions += 1 - - self.assertEqual(spurious_positions, 0, - "No instructions should have first statement's position") - got = self.get_disassembly(_B, depth=1) - self.do_disassembly_compare(got, dis_issue_135700_class) + test_cases = [ + ("module", compile(issue_135700, "", "exec").co_consts[1]), + ("class", compile(ast.parse(issue_135700_class), "?", "exec").co_consts[0].co_consts[1]) + ] + + for case_name, annotate_code in test_cases: + with self.subTest(case=case_name): + instructions = list(dis.Bytecode(annotate_code)) + + spurious_positions = sum( + 1 for instr in instructions + if (instr.positions and + instr.positions.lineno == 1 and + instr.positions.col_offset == 0 and + instr.positions.end_col_offset == 1) + ) + + self.assertEqual(spurious_positions, 0, + f"No instructions in {case_name} __annotate__ should have first statement's position") def test_kw_names(self): # Test that value is displayed for keyword argument names: From aed06b7a3d2a8f2ee2746768207ed2eb28bbcf8f Mon Sep 17 00:00:00 2001 From: PuQing Date: Mon, 23 Jun 2025 19:01:14 +0800 Subject: [PATCH 6/9] fix test --- Lib/test/test_dis.py | 53 +++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 1302b865c7734e..435bb7f02fef68 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -431,18 +431,6 @@ def foo(a: int, b: str) -> str: RETURN_VALUE """ -issue_135700 = """\ -22 -333 -__dataclass_fields__: ClassVar -""" - -issue_135700_class = """\ -class _B: - 2 - c: int -""" - compound_stmt_str = """\ x = 0 while 1: @@ -1138,25 +1126,44 @@ def test_bug_46724(self): def test_annotate_no_spurious_first_node_positions(self): # Test that __annotate__ code doesn't inherit first AST node positions + issue_135700 = "1\nx: int" + issue_135700_class = "class A:\n 1\n x: int" + test_cases = [ ("module", compile(issue_135700, "", "exec").co_consts[1]), - ("class", compile(ast.parse(issue_135700_class), "?", "exec").co_consts[0].co_consts[1]) + ( + "class", + compile(ast.parse(issue_135700_class), "?", "exec") + .co_consts[0] + .co_consts[1], + ), ] for case_name, annotate_code in test_cases: with self.subTest(case=case_name): instructions = list(dis.Bytecode(annotate_code)) - - spurious_positions = sum( - 1 for instr in instructions - if (instr.positions and - instr.positions.lineno == 1 and - instr.positions.col_offset == 0 and - instr.positions.end_col_offset == 1) + print(instructions) + resume_pos = next( + ( + inst.positions + for inst in instructions + if inst.opname == "RESUME" + ), + None, ) - - self.assertEqual(spurious_positions, 0, - f"No instructions in {case_name} __annotate__ should have first statement's position") + for instruction in instructions: + if instruction.opname == "BUILD_MAP": + break + if ( + instruction.opname != "RESUME" + and instruction.positions + and instruction.positions.lineno + ): + self.assertEqual( + instruction.positions, + resume_pos, + f"{case_name}: Unexpected position {instruction.positions} in {instruction.opname}, expected {resume_pos}", + ) def test_kw_names(self): # Test that value is displayed for keyword argument names: From d85247e8e447211407ddb1b4c33095699dad5527 Mon Sep 17 00:00:00 2001 From: PuQing Date: Tue, 24 Jun 2025 14:34:37 +0800 Subject: [PATCH 7/9] Update Lib/test/test_dis.py Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Lib/test/test_dis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 435bb7f02fef68..d80f5fccc6b1da 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -1124,7 +1124,7 @@ def test_bug_46724(self): # Test that negative operargs are handled properly self.do_disassembly_test(bug46724, dis_bug46724) - def test_annotate_no_spurious_first_node_positions(self): + def test_annotate_source_locations(self): # Test that __annotate__ code doesn't inherit first AST node positions issue_135700 = "1\nx: int" issue_135700_class = "class A:\n 1\n x: int" From eca9248cf58b7073321a87567c39f673331f2502 Mon Sep 17 00:00:00 2001 From: PuQing Date: Tue, 24 Jun 2025 14:35:14 +0800 Subject: [PATCH 8/9] Apply suggestions from code review Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Lib/test/test_dis.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index d80f5fccc6b1da..ce12539a1ff351 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -1125,7 +1125,7 @@ def test_bug_46724(self): self.do_disassembly_test(bug46724, dis_bug46724) def test_annotate_source_locations(self): - # Test that __annotate__ code doesn't inherit first AST node positions + # See gh-135700 issue_135700 = "1\nx: int" issue_135700_class = "class A:\n 1\n x: int" @@ -1142,7 +1142,6 @@ def test_annotate_source_locations(self): for case_name, annotate_code in test_cases: with self.subTest(case=case_name): instructions = list(dis.Bytecode(annotate_code)) - print(instructions) resume_pos = next( ( inst.positions From e14cb292ba21fff3d675b9444ecc42f705cc1851 Mon Sep 17 00:00:00 2001 From: PuQing Date: Tue, 24 Jun 2025 16:33:14 +0800 Subject: [PATCH 9/9] fix test --- Lib/test/test_dis.py | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index ce12539a1ff351..02c4ca8b1ba5f6 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -1141,28 +1141,25 @@ def test_annotate_source_locations(self): for case_name, annotate_code in test_cases: with self.subTest(case=case_name): - instructions = list(dis.Bytecode(annotate_code)) - resume_pos = next( - ( - inst.positions - for inst in instructions - if inst.opname == "RESUME" - ), - None, + line_starts_iterator = dis.findlinestarts(annotate_code) + valid_line_starts = [ + item[0] + for item in line_starts_iterator + if item[1] is not None + ] # The first item is not RESUME in class case + setup_scope_begin = valid_line_starts[0] + setup_scope_end = valid_line_starts[1] + setup_annotations_scope_positions = { + instr.positions + for instr in dis.get_instructions(annotate_code) + if setup_scope_begin <= instr.offset < setup_scope_end + and instr.positions + } + self.assertEqual( + len(setup_annotations_scope_positions), + 1, + f"{case_name}: Expected uniform positions, found {len(setup_annotations_scope_positions)}: {setup_annotations_scope_positions}", ) - for instruction in instructions: - if instruction.opname == "BUILD_MAP": - break - if ( - instruction.opname != "RESUME" - and instruction.positions - and instruction.positions.lineno - ): - self.assertEqual( - instruction.positions, - resume_pos, - f"{case_name}: Unexpected position {instruction.positions} in {instruction.opname}, expected {resume_pos}", - ) def test_kw_names(self): # Test that value is displayed for keyword argument names: