gh-98390: Fix source locations of boolean sub-expressions#98396
gh-98390: Fix source locations of boolean sub-expressions#98396iritkatriel merged 2 commits intopython:mainfrom
Conversation
There was a problem hiding this comment.
Thanks, looks good.
One comment: I don't think this function needs ploc anymore. LOC(e) is correct for everything emitted here, right?
It's actually a bug that the Compare_kind case sets the location without restoring it after. I introduced it in #96968 but haven't fixed it yet (since I didn't want to interfere with your refactoring work).
I'm going over all the call sites to see who consumes the modified ploc (expect a PR about assert shortly). Once nobody consumes it, I will convert it to loc. |
|
hi @iritkatriel, script: def func():
assert a and b<c , "msg"
import dis
for i in dis.get_instructions(func):
if i.opname in ("COMPARE_OP","CALL"):
print(i.positions,i.opname,i.argval)output (Python 3.11.1): Positions(lineno=2, end_lineno=2, col_offset=17, end_col_offset=20) COMPARE_OP <
Positions(lineno=2, end_lineno=2, col_offset=17, end_col_offset=20) CALL 0The CALL is the creation of the assertion which has the same source range as output (Python 3.11.0): Positions(lineno=2, end_lineno=2, col_offset=17, end_col_offset=20) COMPARE_OP <
Positions(lineno=2, end_lineno=2, col_offset=4, end_col_offset=28) CALL 0The source range of CALL in 3.11.0 is the whole assertion, which looks reasonable for me. Some context: |
|
@15r10nk the behavior on @iritkatriel what do you think, is this a candidate for backport? |
|
It probably makes the most sense just to fix 3.11 separately (with a backport to 3.10), since location-related stuff has changed a lot on main. I assigned myself to the issue a while back, but other things came up. I'll just make a PR now (thanks for the reminder @15r10nk)! |
|
@carljm yes. I found this problem in 3.11.1. |
Fixes #98390.