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 locations of boolean sub-expressions are too wide #98390

Closed
iritkatriel opened this issue Oct 18, 2022 · 2 comments · Fixed by #98396
Closed

Source locations of boolean sub-expressions are too wide #98390

iritkatriel opened this issue Oct 18, 2022 · 2 comments · Fixed by #98396
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

This is a particular case of #93691.

This code:

def f():
 a, b, e, f = 1,2,3,4
 if (a or
    (b and e) or
    f):
   12


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)])

Outputs:

[((1, 1, 0, 0), 'RESUME', 0),
 ((2, 2, 14, 21), 'LOAD_CONST', (1, 2, 3, 4)),
 ((2, 2, 1, 11), 'UNPACK_SEQUENCE', 4),
 ((2, 2, 1, 2), 'STORE_FAST', 'a'),
 ((2, 2, 4, 5), 'STORE_FAST', 'b'),
 ((2, 2, 7, 8), 'STORE_FAST', 'e'),
 ((2, 2, 10, 11), 'STORE_FAST', 'f'),
 ((3, 3, 5, 6), 'LOAD_FAST', 'a'),
 ((3, 6, 1, 5), 'POP_JUMP_IF_TRUE', 32),        <--- 
 ((4, 4, 5, 6), 'LOAD_FAST', 'b'),
 ((3, 6, 1, 5), 'POP_JUMP_IF_FALSE', 28),      <--- 
 ((4, 4, 11, 12), 'LOAD_FAST', 'e'),
 ((3, 6, 1, 5), 'POP_JUMP_IF_TRUE', 32),        <--- 
 ((5, 5, 4, 5), 'LOAD_FAST', 'f'),
 ((3, 6, 1, 5), 'POP_JUMP_IF_FALSE', 36),      <--- 
 ((6, 6, 3, 5), 'LOAD_CONST', None),
 ((6, 6, 3, 5), 'RETURN_VALUE', None),
 ((3, 6, 1, 5), 'LOAD_CONST', None),
 ((3, 6, 1, 5), 'RETURN_VALUE', None)]

The marked jumps have very wide locations - spanning the whole conditional expression. They should instead point to the particular sub-expression that they come from.

@iritkatriel iritkatriel added the type-bug An unexpected behavior, bug, or error label Oct 18, 2022
@iritkatriel iritkatriel self-assigned this Oct 18, 2022
@iritkatriel iritkatriel added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.12 bugs and security fixes labels Oct 18, 2022
@iritkatriel iritkatriel changed the title Source locations of boolean expressions are too wide Source locations of boolean sub-expressions are too wide Oct 18, 2022
iritkatriel added a commit to iritkatriel/cpython that referenced this issue Oct 18, 2022
iritkatriel added a commit to iritkatriel/cpython that referenced this issue Oct 18, 2022
@iritkatriel
Copy link
Member Author

With #98396 , we get:

[((1, 1, 0, 0), 'RESUME', 0),
 ((2, 2, 14, 21), 'LOAD_CONST', (1, 2, 3, 4)),
 ((2, 2, 1, 11), 'UNPACK_SEQUENCE', 4),
 ((2, 2, 1, 2), 'STORE_FAST', 'a'),
 ((2, 2, 4, 5), 'STORE_FAST', 'b'),
 ((2, 2, 7, 8), 'STORE_FAST', 'e'),
 ((2, 2, 10, 11), 'STORE_FAST', 'f'),
 ((3, 3, 5, 6), 'LOAD_FAST', 'a'),
 ((3, 3, 5, 6), 'POP_JUMP_IF_TRUE', 32),
 ((4, 4, 5, 6), 'LOAD_FAST', 'b'),
 ((4, 4, 5, 6), 'POP_JUMP_IF_FALSE', 28),
 ((4, 4, 11, 12), 'LOAD_FAST', 'e'),
 ((4, 4, 11, 12), 'POP_JUMP_IF_TRUE', 32),
 ((5, 5, 4, 5), 'LOAD_FAST', 'f'),
 ((5, 5, 4, 5), 'POP_JUMP_IF_FALSE', 36),
 ((6, 6, 3, 5), 'LOAD_CONST', None),
 ((6, 6, 3, 5), 'RETURN_VALUE', None),
 ((5, 5, 4, 5), 'LOAD_CONST', None),
 ((5, 5, 4, 5), 'RETURN_VALUE', None)]

@iritkatriel
Copy link
Member Author

@nedbat FYI.

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
Development

Successfully merging a pull request may close this issue.

1 participant