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 all 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.

Why do we have a pdb test for it? From the original issue I don't see how pdb is involved. Will this problem affect pdb in some way? I don't think we should write a regression test in test_pdb if pdb is just a convenient tool to check a certain issue. If the issue breaks pdb, then that's fine.

To be honest I did not completely get the issue, but if the symptom is end_col_offset is wrong, how it impacts pdb?

Copy link
Contributor

Choose a reason for hiding this comment

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

@gaogaotiantian I think this is the context you are missing #135814 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Okay I think I got the background of this issue.

So this is introduced by PEP 649 right? We need an expert there. We need to define what's the correct line number for this. I don't believe the line number below is correct:

# <--- Please let me know if this position is weird or correct, 
   8           LOAD_CONST               2 (<code object __annotate__ at 0x7d090a29cd50, file "testmod.py", line 8>)
               MAKE_FUNCTION
               STORE_NAME               2 (__annotate__)
               BUILD_SET                0
               STORE_NAME               0 (__conditional_annotations__)

This is some global preparation for the future lazy evaluation for annotation, so the line number should be 1. On the other hand, the __annotation__ code object should probably have a single line number which is the line number of the actual annotation.

We should mark the target then shoot on it, not trying to fix something and see if the result is acceptable. (That's why I think we should involve the experts on PEP 647, @larryhastings maybe?).

As for pdb, this test itself is a bit confusing - not unacceptable, but we can probably make it easier to avoid using a separate module. I believe class annotation has the similar effect to reproduce this.

Also, even though this messes up with pdb, fundamentally it's a code object issue. We should also have tests in test_dis or something.

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 ?


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__`.
41 changes: 25 additions & 16 deletions Python/codegen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/

#include "Python.h"
#include "listobject.h"
#include "opcode.h"
#include "pycore_ast.h" // _PyAST_GetDocString()
#define NEED_OPCODE_TABLES
Expand Down Expand Up @@ -686,13 +687,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 +753,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 +789,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 +824,15 @@ 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
assert(PyList_GET_SIZE(deferred_anno) > 0);
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 +845,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 +876,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 +1570,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