Skip to content

Set line number on the POP_TOP that follows a RETURN_GENERATOR #109923

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.

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

Closed
iritkatriel opened this issue Sep 26, 2023 · 2 comments · Fixed by #109924
Closed

Set line number on the POP_TOP that follows a RETURN_GENERATOR #109923

iritkatriel opened this issue Sep 26, 2023 · 2 comments · Fixed by #109924

Comments

@iritkatriel
Copy link
Member

iritkatriel commented Sep 26, 2023

Currently the POP_TOP of a generator doesn't get a line number.
If we give it the same line number as the RETURN_GENERATOR and RESUME, that will reduce the size of the line number table.

The missing line number also got in my way while working on #109094.

>>> def f(): yield 42
... 
>>> dis.dis(f)
   1           0 RETURN_GENERATOR

None           2 POP_TOP

   1           4 RESUME                   0
               6 LOAD_CONST               1 (42)
               8 YIELD_VALUE              1
              10 RESUME                   1
              12 POP_TOP
              14 RETURN_CONST             0 (None)

None     >>   16 CALL_INTRINSIC_1         3 (INTRINSIC_STOPITERATION_ERROR)
              18 RERAISE                  1
ExceptionTable:
  4 to 14 -> 16 [0] lasti

Linked PRs

@markshannon
Copy link
Member

The POP_TOP always comes before the RESUME, so the line number should never be visible.

I agree it is worth doing in order to reduce the size of the table, but I'm curious how it helped with #109094.

@iritkatriel
Copy link
Member Author

I'm curious how it helped with #109094.

There's a test that throws into a newly created generator. The instr_ptr points to the POP_TOP and the prev_instr points to the RETURN_GENERATOR so that line number in the traceback was different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants