Skip to content
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

Source location of return instruction in a with block is incorrect #98442

Closed
iritkatriel opened this issue Oct 19, 2022 · 3 comments · Fixed by #120763 · May be fixed by #98443
Closed

Source location of return instruction in a with block is incorrect #98442

iritkatriel opened this issue Oct 19, 2022 · 3 comments · Fixed by #120763 · May be fixed by #98443
Assignees
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@iritkatriel
Copy link
Member

iritkatriel commented Oct 19, 2022

def f():
 with x:
   return 42

import dis
from pprint import pprint as pp
def pos(p):
    return (p.lineno, p.end_lineno, p.col_offset, p.end_col_offset)

pp([(pos(x.positions), x.opname, x.argval) for x in dis.get_instructions(f)])

Output:

[((1, 1, 0, 0), 'RESUME', 0),
 ((2, 2, 6, 7), 'LOAD_GLOBAL', 'x'),
 ((2, 3, 1, 12), 'BEFORE_WITH', None),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((3, 3, 10, 12), 'NOP', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'CALL', 2),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'LOAD_CONST', 42),                 <-- incorrect
 ((2, 3, 1, 12), 'RETURN_VALUE', None),             <-- incorrect
 ((2, 3, 1, 12), 'PUSH_EXC_INFO', None),
 ((2, 3, 1, 12), 'WITH_EXCEPT_START', None),
 ((2, 3, 1, 12), 'POP_JUMP_IF_TRUE', 50),
 ((2, 3, 1, 12), 'RERAISE', 2),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'POP_EXCEPT', None),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'RETURN_VALUE', None),
 ((None, None, None, None), 'COPY', 3),
 ((None, None, None, None), 'POP_EXCEPT', None),
 ((None, None, None, None), 'RERAISE', 1)]

Linked PRs

@iritkatriel iritkatriel added the type-bug An unexpected behavior, bug, or error label Oct 19, 2022
@iritkatriel iritkatriel self-assigned this Oct 19, 2022
@iritkatriel iritkatriel added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.12 bugs and security fixes labels Oct 19, 2022
iritkatriel added a commit to iritkatriel/cpython that referenced this issue Oct 19, 2022
@iritkatriel iritkatriel changed the title location of return instruction in a with block is incorrect Source locations of return instruction in a with block is incorrect Oct 19, 2022
@iritkatriel iritkatriel changed the title Source locations of return instruction in a with block is incorrect Source location of return instruction in a with block is incorrect Oct 19, 2022
@iritkatriel
Copy link
Member Author

@nedbat FYI.

@iritkatriel
Copy link
Member Author

This is now less wrong, but still there are instructions (in the __exit__ call) that span the whole body of the with-block:

The return instruction takes its location from there, so it will be fixed once this is.

[((1, 1, 0, 0), 'RESUME', 0),
 ((2, 2, 6, 7), 'LOAD_GLOBAL', 'x'),
 ((2, 2, 6, 7), 'COPY', 1),
 ((2, 2, 6, 7), 'LOAD_SPECIAL', 1),
 ((2, 2, 6, 7), 'SWAP', 2),
 ((2, 2, 6, 7), 'SWAP', 3),
 ((2, 2, 6, 7), 'LOAD_SPECIAL', 0),
 ((2, 2, 6, 7), 'CALL', 0),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((3, 3, 10, 12), 'NOP', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),      <-- Too wide
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'CALL', 3),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'RETURN_CONST', 42),
 ((2, 2, 6, 7), 'PUSH_EXC_INFO', None),
 ((2, 2, 6, 7), 'WITH_EXCEPT_START', None),
 ((2, 2, 6, 7), 'TO_BOOL', None),
 ((2, 2, 6, 7), 'POP_JUMP_IF_TRUE', 70),
 ((2, 2, 6, 7), 'RERAISE', 2),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'POP_EXCEPT', None),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'RETURN_CONST', None),
 ((None, None, None, None), 'COPY', 3),
 ((None, None, None, None), 'POP_EXCEPT', None),
 ((None, None, None, None), 'RERAISE', 1)]

@iritkatriel
Copy link
Member Author

With the fix in the PR we get:

[((1, 1, 0, 0), 'RESUME', 0),
 ((2, 2, 6, 7), 'LOAD_GLOBAL', 'x'),
 ((2, 2, 6, 7), 'COPY', 1),
 ((2, 2, 6, 7), 'LOAD_SPECIAL', 1),
 ((2, 2, 6, 7), 'SWAP', 2),
 ((2, 2, 6, 7), 'SWAP', 3),
 ((2, 2, 6, 7), 'LOAD_SPECIAL', 0),
 ((2, 2, 6, 7), 'CALL', 0),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((3, 3, 10, 12), 'NOP', None),
 ((2, 2, 6, 7), 'LOAD_CONST', None),
 ((2, 2, 6, 7), 'LOAD_CONST', None),
 ((2, 2, 6, 7), 'LOAD_CONST', None),
 ((2, 2, 6, 7), 'CALL', 3),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'RETURN_CONST', 42),
 ((2, 2, 6, 7), 'PUSH_EXC_INFO', None),
 ((2, 2, 6, 7), 'WITH_EXCEPT_START', None),
 ((2, 2, 6, 7), 'TO_BOOL', None),
 ((2, 2, 6, 7), 'POP_JUMP_IF_TRUE', 70),
 ((2, 2, 6, 7), 'RERAISE', 2),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'POP_EXCEPT', None),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'POP_TOP', None),
 ((2, 2, 6, 7), 'RETURN_CONST', None),
 ((None, None, None, None), 'COPY', 3),
 ((None, None, None, None), 'POP_EXCEPT', None),
 ((None, None, None, None), 'RERAISE', 1)]

iritkatriel added a commit that referenced this issue Jun 20, 2024
…0763)

gh-98442: fix location of with statement's cleanup instructions
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 20, 2024
pythonGH-120763)

(cherry picked from commit 55596ae)

Co-authored-by: Irit Katriel <[email protected]>
pythongh-98442: fix location of with statement's cleanup instructions
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 20, 2024
pythonGH-120763)

(cherry picked from commit 55596ae)

Co-authored-by: Irit Katriel <[email protected]>
pythongh-98442: fix location of with statement's cleanup instructions
mrahtz pushed a commit to mrahtz/cpython that referenced this issue Jun 30, 2024
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
iritkatriel added a commit that referenced this issue Sep 2, 2024
…ns (GH-120763) (#120786)

gh-98442: fix locations of with statement's cleanup instructions (GH-120763)
(cherry picked from commit 55596ae)


gh-98442: fix location of with statement's cleanup instructions

Co-authored-by: Irit Katriel <[email protected]>
iritkatriel added a commit that referenced this issue Sep 15, 2024
…ns (GH-120763) (#120787)

gh-98442: fix locations of with statement's cleanup instructions (GH-120763)
(cherry picked from commit 55596ae)


gh-98442: fix location of with statement's cleanup instructions

Co-authored-by: Irit Katriel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
1 participant