gh-135700: Fix instructions in __annotate__ have incorrect code positions #135814
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1141,28 +1141,25 @@ def test_annotate_source_locations(self): | |
|
|
||
| for case_name, annotate_code in test_cases: | ||
| with self.subTest(case=case_name): | ||
| 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 | ||
| } | ||
|
There was a problem hiding this comment. Why does this need to be so complicated? Don't we just need to check that all the instructions in this code object have the same line number (excluding Nones)? There was a problem hiding this comment. Because the important thing is to make sure the instructions in no fix: Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) RESUME
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=2) LOAD_FAST_BORROW # incorrect
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=2) LOAD_SMALL_INT # incorrect
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=2) COMPARE_OP # incorrect
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=2) POP_JUMP_IF_FALSE # incorrect
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=2) NOT_TAKEN # incorrect
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=2) LOAD_COMMON_CONSTANT # incorrect
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=2) RAISE_VARARGS # incorrect
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=2) BUILD_MAP # incorrectfix: Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) RESUME
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) LOAD_FAST_BORROW
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) LOAD_SMALL_INT
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) COMPARE_OP
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) POP_JUMP_IF_FALSE
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) NOT_TAKEN
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) LOAD_COMMON_CONSTANT
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) RAISE_VARARGS
Positions(lineno=1, end_lineno=1, col_offset=0, end_col_offset=0) BUILD_MAPAnd not all annotate lines are the same, because some opcodes need to indicate the "code" where the annotation occurs. There was a problem hiding this comment. Ok, so we need to check they all have the same end_col_offset as well. There was a problem hiding this comment.
The test doesn't need to work for all inputs. Only the two inputs it runs on. There was a problem hiding this comment.
Just to be clear, instructions within code object can correspond to different line numbers. So I made an offset restriction here, limiting the check to between RESUME and BUILD_MAP (setup_annotations_scope) There was a problem hiding this comment. I don't have much of an intuition on how line numbers should work for synthetic code. There was a problem hiding this comment. @15r10nk are you able to suggest a meaningful unit test for this? Alternatively, can you confirm that the PR resolved the issue? There was a problem hiding this comment. I can verify that this fixes the issue which I have reported 👍 . I also found out that the reported bug affected debugging with pdb. example.py import example_mod
print(example_mod.__annotations__)example_mod.py 22
class ClassVar():
pass
__dataclass_fields__: ClassVarThe breakpoint is hit a second time when the synthetic code is executed which uses the incorrect source positions. I remember that I found multiple different problems with similar synthetic codes like this in the past ( There was a problem hiding this comment. Great, thank you! The pdb example can probably be used for a unit test (in test_pdb). I'd remove the test that is currently in this PR. There was a problem hiding this comment. hi @iritkatriel @AndPuQing , I wrote a test with test_pdb.py maybe that can be used for this pr. class PdbTestCase(unittest.TestCase):
...
def test_issue135700(self):
""" https://github.com/python/cpython/issues/135700 """
module_code = """\
22
class ClassVar:
pass
__dataclass_fields__: ClassVar
"""
with open('testmod.py', 'w') as f:
f.write(module_code)
self.addCleanup(os_helper.unlink, "testmod.py")
script = """
import testmod
print(testmod.__annotations__)
"""
commands = """
b testmod.py:1
c
c
"""
result = self.run_pdb_script(script, commands)
self.assertNotIn("(1)__annotate__()", result[0]) |
||
| self.assertEqual( | ||
| len(setup_annotations_scope_positions), | ||
| 1, | ||
| f"{case_name}: Expected uniform positions, found {len(setup_annotations_scope_positions)}: {setup_annotations_scope_positions}", | ||
| ) | ||
|
|
||
| def test_kw_names(self): | ||
| # Test that value is displayed for keyword argument names: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert here that
annotate_codeis a code object and thatannotate_code.co_nameis'__annotate__'.