Skip to content

gh-135700: Fix instructions in __annotate__ have incorrect code positions #136543

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.

Sign up for GitHub

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 8 commits into
base: main
Choose a base branch
from
Open

gh-135700: Fix instructions in __annotate__ have incorrect code positions #136543

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4b2ec1e
fix instructions in __annotate__ have incorrect code positions
AndPuQing Jun 22, 2025
35246e7
📜🤖 Added by blurb_it.
blurb-it[bot] Jun 22, 2025
97bddb8
Update Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issu…
AndPuQing Jun 22, 2025
4de616a
add unit tests
AndPuQing Jun 22, 2025
a297926
Update Lib/test/test_pdb.py
AndPuQing Jul 13, 2025
5820ce8
fix codegen in annotations
AndPuQing Jul 13, 2025
850601a
add empty check
AndPuQing Jul 13, 2025
0e14a6d
assert deferred_anno is not empty
AndPuQing Jul 13, 2025
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
25 changes: 25 additions & 0 deletions Lib/test/test_pdb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3816,6 +3816,31 @@ def bar():
self.assertIn('-> pass', lines)
self.assertIn('(Pdb) 42', lines)

def test_issue135700(self):
# See gh-135700
module_code = textwrap.dedent("""\
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 = textwrap.dedent("""
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])
Copy link
Member

Choose a reason for hiding this comment

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

This test passes for me on main (without this PR).

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is the indentation. @AndPuQing indented my code in the strings which (for some unknown reason for me) caused the test to pass. a textwrap.dedent should help.

Copy link
Contributor Author

@AndPuQing AndPuQing Jul 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, I fixed the indent problem, but it seems that the current fix also cannot pass the test.

======================================================================
FAIL: test_issue135700 (test.test_pdb.PdbTestCase.test_issue135700)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/mnt/data/xxx/dev/cpython/Lib/test/test_pdb.py", line 3842, in test_issue135700
    self.assertNotIn("(1)__annotate__()", result[0])
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: '(1)__annotate__()' unexpectedly found in '> ...
../cpython/python -m dis testmod.py                                                                            (base) 
  --           MAKE_CELL                0 (__conditional_annotations__)

   0           RESUME                   0

   1           LOAD_CONST               2 (<code object __annotate__ at 0x798bd679cd50, file "testmod.py", line 1>)
               MAKE_FUNCTION
               STORE_NAME               2 (__annotate__)
               BUILD_SET                0
               STORE_NAME               0 (__conditional_annotations__)

   3           LOAD_BUILD_CLASS
               PUSH_NULL
               LOAD_CONST               0 (<code object ClassVar at 0x798bd6bb1940, file "testmod.py", line 3>)
               MAKE_FUNCTION
               LOAD_CONST               1 ('ClassVar')
               CALL                     2
               STORE_NAME               1 (ClassVar)

   5           LOAD_NAME                0 (__conditional_annotations__)
               LOAD_SMALL_INT           0
               SET_ADD                  1
               POP_TOP
               LOAD_CONST               3 (None)
               RETURN_VALUE

Disassembly of <code object ClassVar at 0x798bd6bb1940, file "testmod.py", line 3>:
  3           RESUME                   0
              LOAD_NAME                0 (__name__)
              STORE_NAME               1 (__module__)
              LOAD_CONST               0 ('ClassVar')
              STORE_NAME               2 (__qualname__)
              LOAD_SMALL_INT           3
              STORE_NAME               3 (__firstlineno__)

  4           LOAD_CONST               1 (())
              STORE_NAME               4 (__static_attributes__)
              LOAD_CONST               2 (None)
              RETURN_VALUE

Disassembly of <code object __annotate__ at 0x798bd679cd50, file "testmod.py", line 1>:
  1           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

  5           LOAD_SMALL_INT           0
              LOAD_GLOBAL              0 (__conditional_annotations__)
              CONTAINS_OP              0 (in)
              POP_JUMP_IF_FALSE       10 (to L2)
              NOT_TAKEN
              LOAD_GLOBAL              2 (ClassVar)
              COPY                     2
              LOAD_CONST               1 ('__dataclass_fields__')

  1           STORE_SUBSCR
      L2:     RETURN_VALUE

Copy link
Contributor

@15r10nk 15r10nk Jul 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

hmm, @AndPuQing your fix solved the problem which I have reported in the issue (the positions in the bytecode and column information) but not the problem with pdb because the line number of the synthetic code in __annotate__ is still 1. I don't know if there is a way to express an non existing code location (-1 maybe), but this is what would be needed here. Another solution might be to use the location of the first ast-node which is handled by the code (line 5 in this case). This would cause no line change when the code is executed. I thing this might be the most practical solution here.

Copy link
Member

Choose a reason for hiding this comment

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

Cc @gaogaotiantian

Copy link
Member

Choose a reason for hiding this comment

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

I feel this is more of a question for experts on tracing (you :D ). From the annotations perspective, the code here isn't directly connected to any code the user wrote; it seems reasonable to me to pretend it's on the first line of the module, or on the first annotation in the module, whatever works better for tools that use the data. But maybe Larry has a stronger opinion.

Copy link
Member

Choose a reason for hiding this comment

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

Personally I think the best solution here is to set the line number to invalid so it won't trigger a line event at all - there's no corresponding source code for it. Any unexpected side effects for this solution @iritkatriel ?

Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by invalid? -1? That doesn't mean invalid exactly, it would probably get the line number of the previous instruction.

Copy link
Member

Choose a reason for hiding this comment

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

Do we have any indicator for "there's no line number for this byte code"? Anything before RESUME seems to have a -- with it. I'm curious whether that's a possible solution - that's the most "correct" solution. If not, we can seek for alternatives.

Copy link
Member

Choose a reason for hiding this comment

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

We have NO_LOCATION which usually gets replaced by a "previous instruction location", but not always (sometimes it doesn't matter). We don't currently have a way to indicate that an instruction must not have a location. It's not hard to add that, but we need to determine if that's the right thing to do.


def test_step_into_botframe(self):
# gh-125422
# pdb should not be able to step into the botframe (bdb.py)
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core_and_Builtins/2025-06-22-10-47-27.gh-issue-135700.0qdtCl.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix instructions positions in :attr:`~object.__annotate__`.
42 changes: 26 additions & 16 deletions Python/codegen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,13 +686,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;
}
Expand Down Expand Up @@ -751,7 +752,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 <annos>
ADDOP_I(c, NO_LOCATION, BUILD_MAP, 0); // stack now contains <annos>

for (Py_ssize_t i = 0; i < annotations_len; i++) {
PyObject *ptr = PyList_GET_ITEM(deferred_anno, i);
Expand Down Expand Up @@ -787,7 +788,7 @@ codegen_deferred_annotations_body(compiler *c, location loc,
ADDOP_I(c, LOC(st), COPY, 2);
ADDOP_LOAD_CONST_NEW(c, LOC(st), mangled);
// stack now contains <annos> <name> <annos> <value>
ADDOP(c, loc, STORE_SUBSCR);
ADDOP(c, LOC(st), STORE_SUBSCR);
// stack now contains <annos>

USE_LABEL(c, not_set);
Expand Down Expand Up @@ -822,7 +823,17 @@ codegen_process_deferred_annotations(compiler *c, location loc)
PySTEntryObject *ste = SYMTABLE_ENTRY(c);
assert(ste->ste_annotation_block != NULL);
void *key = (void *)((uintptr_t)ste->ste_id + 1);
if (codegen_setup_annotations_scope(c, loc, key,

// Get the first annotation location
if (PyList_GET_SIZE(deferred_anno) == 0) {
goto error;
}
PyObject* ptr = PyList_GET_ITEM(deferred_anno, 0);
stmt_ty st = (stmt_ty)PyLong_AsVoidPtr(ptr);
if (st == NULL) {
goto error;
}
if (codegen_setup_annotations_scope(c, LOC(st), key,
ste->ste_annotation_block->ste_name) < 0) {
goto error;
}
Expand All @@ -835,12 +846,11 @@ codegen_process_deferred_annotations(compiler *c, location loc)
Py_DECREF(deferred_anno);
Py_DECREF(conditional_annotation_indices);

RETURN_IF_ERROR(codegen_leave_annotations_scope(c, loc));
RETURN_IF_ERROR(codegen_leave_annotations_scope(c, LOC(st)));
RETURN_IF_ERROR(codegen_nameop(
c, loc,
c, LOC(st),
ste->ste_type == ClassBlock ? &_Py_ID(__annotate_func__) : &_Py_ID(__annotate__),
Store));

if (need_separate_block) {
RETURN_IF_ERROR(_PyCompile_EndAnnotationSetup(c));
}
Expand All @@ -867,8 +877,8 @@ int
_PyCodegen_Module(compiler *c, location loc, asdl_stmt_seq *stmts, bool is_interactive)
{
if (SYMTABLE_ENTRY(c)->ste_has_conditional_annotations) {
ADDOP_I(c, loc, BUILD_SET, 0);
ADDOP_N(c, loc, STORE_NAME, &_Py_ID(__conditional_annotations__), names);
ADDOP_I(c, NO_LOCATION, BUILD_SET, 0);
ADDOP_N(c, NO_LOCATION, STORE_NAME, &_Py_ID(__conditional_annotations__), names);
}
return codegen_body(c, loc, stmts, is_interactive);
}
Expand Down Expand Up @@ -1561,8 +1571,8 @@ codegen_class_body(compiler *c, stmt_ty s, int firstlineno)
ADDOP_N_IN_SCOPE(c, loc, STORE_DEREF, &_Py_ID(__classdict__), cellvars);
}
if (SYMTABLE_ENTRY(c)->ste_has_conditional_annotations) {
ADDOP_I(c, loc, BUILD_SET, 0);
ADDOP_N_IN_SCOPE(c, loc, STORE_DEREF, &_Py_ID(__conditional_annotations__), cellvars);
ADDOP_I(c, NO_LOCATION, BUILD_SET, 0);
ADDOP_N_IN_SCOPE(c, NO_LOCATION, STORE_DEREF, &_Py_ID(__conditional_annotations__), cellvars);
}
/* compile the body proper */
RETURN_IF_ERROR_IN_SCOPE(c, codegen_body(c, loc, s->v.ClassDef.body, false));
Expand Down
Loading