comparison chaining has wrong source positions in most contexts (python 3.11.0rc2) #95921

15r10nk · 2022-08-12T09:25:16Z

problem

comparison chaining has wrong source positions in most contexts

>>> cnd = True                                            
>>> a = 1 if 1 in 2 == 3 else 0                                                                
Traceback (most recent call last):                                                              
  File "example.py", line 43, in <module>                                                             
    a = 1 if 1 in 2 == 3 else 0                                              
        ^^^^^^^^^^^^^^^^^^^^^^^                                                                  
TypeError: argument of type 'int' is not iterable

This bug requires two things to happen:

The comparison chain has to contain more than on comparison the type does not matter (==,in,<=,is,...)
The context has to be something other than an expression

no problem

The following examples work:
only one comparison

Traceback (most recent call last):
  File "example.py", line 4, in <module>
    if 5<"5":
       ^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

inside expression

>>> a=False or 4<"5"<6
Traceback (most recent call last):
  File "example.py", line 15, in <module>
    a=False or 4<"5"<6
               ^^^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

reason

The problem seems to be the positions in the compiled bytecode:

script:

import dis

def foo():
    if 1<2<"no int":
        return 1
    return 0

for p in dis.get_instructions(foo):
    print(p.positions, p.opname,p.argval)

output (Python 3.11.0rc2+):

Positions(lineno=4, end_lineno=4, col_offset=0, end_col_offset=0) RESUME 0
Positions(lineno=5, end_lineno=5, col_offset=7, end_col_offset=8) LOAD_CONST 1
Positions(lineno=5, end_lineno=5, col_offset=9, end_col_offset=10) LOAD_CONST 2
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) SWAP 2
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) COPY 2
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) COMPARE_OP < # range of the whole if
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) POP_JUMP_FORWARD_IF_FALSE 30
Positions(lineno=5, end_lineno=5, col_offset=11, end_col_offset=19) LOAD_CONST no int
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) COMPARE_OP < # range of the whole if
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) POP_JUMP_FORWARD_IF_FALSE 38
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) JUMP_FORWARD 34
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) POP_TOP None
Positions(lineno=5, end_lineno=6, col_offset=4, end_col_offset=16) JUMP_FORWARD 38
Positions(lineno=6, end_lineno=6, col_offset=15, end_col_offset=16) LOAD_CONST 1
Positions(lineno=6, end_lineno=6, col_offset=15, end_col_offset=16) RETURN_VALUE None
Positions(lineno=7, end_lineno=7, col_offset=11, end_col_offset=12) LOAD_CONST 0
Positions(lineno=7, end_lineno=7, col_offset=11, end_col_offset=12) RETURN_VALUE None

the generated ast looks fine (snippet):

            If(
               test=Compare(
                  left=Constant(
                     value=1,
                     lineno=5,
                     col_offset=7, # correct
                     end_lineno=5,
                     end_col_offset=8),
                  ops=[
                     Lt(),
                     Lt()],
                  comparators=[
                     Constant(
                        value=2,
                        lineno=5,
                        col_offset=9, # correct
                        end_lineno=5,
                        end_col_offset=10),
                     Constant(
                        value='no int',
                        lineno=5,
                        col_offset=11, # correct
                        end_lineno=5,
                        end_col_offset=19)],
                  lineno=5,
                  col_offset=7,
                  end_lineno=5,
                  end_col_offset=19),

other examples

here is a list of all the problems I found so far:

>>> if 4<"5"<6:
...     pass
...
Traceback (most recent call last):
  File "example.py", line 20, in <module>
    if 4<"5"<6:
TypeError: '<' not supported between instances of 'int' and 'str'
>>> while 4<"5"<6:
...     pass
...
Traceback (most recent call last):
  File "example.py", line 24, in <module>
    while 4<"5"<6:
TypeError: '<' not supported between instances of 'int' and 'str'

>>> assert 4<"5"<6
Traceback (most recent call last):
  File "example.py", line 27, in <module>
    assert 4<"5"<6
TypeError: '<' not supported between instances of 'int' and 'str'

>>> a=2+(5 if 4<"5"<6 else 3)
Traceback (most recent call last):
  File "example.py", line 29, in <module>
    a=2+(5 if 4<"5"<6 else 3)
         ^^^^^^^^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

>>> a=[a for a in [1] if 4<"5"<a ]
Traceback (most recent call last):
  File "example.py", line 34, in <listcomp>
    a=[a for a in [1] if 4<"5"<a ]
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

>>> a={a for a in [1] if 4<"5"<a }
Traceback (most recent call last):
  File "example.py", line 35, in <setcomp>
    a={a for a in [1] if 4<"5"<a }
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

>>> a={a:a for a in [1] if 4<"5"<a }
Traceback (most recent call last):
  File "example.py", line 36, in <dictcomp>
    a={a:a for a in [1] if 4<"5"<a }
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

>>> a=list((a for a in [1] if 4<"5"<a ))
Traceback (most recent call last):
  File "example.py", line 37, in <genexpr>
    a=list((a for a in [1] if 4<"5"<a ))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

>>> cnd = True
>>> a = 1 if 1 in 2 == 3 else 0
Traceback (most recent call last):
  File "example.py", line 43, in <module>
    a = 1 if 1 in 2 == 3 else 0
        ^^^^^^^^^^^^^^^^^^^^^^^
TypeError: argument of type 'int' is not iterable

>>> if True and 4<"5"<6:
...     pass
...
Traceback (most recent call last):
  File "example.py", line 45, in <module>
    if True and 4<"5"<6:
TypeError: '<' not supported between instances of 'int' and 'str'

expected result

The positions should always match the ast-node range:

>>> a=list((a for a in [1] if 4<"5"<a ))
Traceback (most recent call last):
  File "example.py", line 37, in <genexpr>
    a=list((a for a in [1] if 4<"5"<a ))
                              ^^^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

or, even better only the failing comparison:

>>> a=list((a for a in [1] if 4<"5"<a ))
Traceback (most recent call last):
  File "example.py", line 37, in <genexpr>
    a=list((a for a in [1] if 4<"5"<a ))
                              ^^^^^
TypeError: '<' not supported between instances of 'int' and 'str'

The text was updated successfully, but these errors were encountered:

…sert rewriting This affects also chained comparisons inside normal assertions because of python/cpython#95921. This is also the reason for the changes in sample results. once that is fixed we could revert the changes in the sample_results

15r10nk · 2022-09-18T14:03:55Z

@brandtbucher I think this is related to #93691.
Do you know if there someone already working on this?

Does it makes sense for me to look into it? I have (almost) no cpython core developer experience and it would take me probably far longer than someone else.

brandtbucher · 2022-09-18T14:30:39Z

Thanks for the detailed report. This issue seems to stem from the fact that we have two implementations of comparisons in the bytecode compiler: one generic version (compiler_compare) and one optimized for branching conditions (in compiler_jump_if). The latter doesn’t update the location to the current node, which is probably an oversight.

I’ll have a fix up either today or tomorrow. It will probably need to wait for 3.11.1, though.

…96968) (cherry picked from commit dfc73b5)

…thonGH-96968). (cherry picked from commit dfc73b5) Co-authored-by: Brandt Bucher <brandtbucher@microsoft.com>

GH-96973) (cherry picked from commit dfc73b5) Automerge-Triggered-By: GH:brandtbucher

GH-96974) (cherry picked from commit dfc73b5) Automerge-Triggered-By: GH:brandtbucher

brandtbucher · 2022-09-20T22:29:35Z

Thanks again for the great bug report!

15r10nk · 2022-09-21T15:47:35Z

Big thank you for the quick bugfix @brandtbucher. But the fix caused another bug.

The CALL which creates the AssertionError("msg") gets now incorrect positions.

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 (with your fix):

Positions(lineno=4, end_lineno=4, col_offset=17, end_col_offset=20) COMPARE_OP <
Positions(lineno=4, end_lineno=4, col_offset=17, end_col_offset=20) CALL 0

You see that the positions of the CALL are equal to the positions of the b<c.

output (the commit before your fix):

Positions(lineno=4, end_lineno=4, col_offset=17, end_col_offset=20) COMPARE_OP <
Positions(lineno=4, end_lineno=4, col_offset=4, end_col_offset=28) CALL 0

The positions of the CALL is here the whole assert statement (which is ok for me, because it is easy to handle).

This did not cause any incorrect Tracebacks (at least I don't know how to produce it), but can cause problems for tools which analyse this positions.

15r10nk · 2022-09-21T15:58:39Z

It is maybe also important that this is not the only example.

    assert b<c , "msg"
    assert a<b<c , "msg"
    assert a+b<c , "msg"

show the same problems. I looks like everything with one or more < causes this problem.

brandtbucher · 2022-09-22T04:08:24Z

Yep, I see the issue. My patch sets the location when recursing into the expression, but doesn't restore it after.

…rsion 1.1.0 Alex Hall (4): GHA: test 3.11 and PRs Use `only` instead of `find_node` function Ignore EXTENDED_ARG/NOP when counting CALL_FUNCTION ops 3.11 in setup.cfg Frank Hoffmann (91): feat: support for python 3.11 fix: fixed decorator detection for 3.11.0a5 fix: detect decorators in 3.11.a6 correctly and ignore qualname checks for 3.11 feat: added support for more bytecodes and worked around some issues moved implementation for 3.11 to NewNodeFinder added missing sample_results file for 3.11 skip ast nodes in the which are related to annotations skip samples with incorrect syntax fixed some issues based on the current 3.11 branch wip: added _collections.py which leads to this bug python/cpython#95150 [skip ci] fix: fixed incompatibilities with master refactor: removed workaround for a fixed bug in python 3.11 fix: removed limitation in tests where the reason was fixed in cpython fix: uncommented test_module_files refactor: moved some code and renamed both NodeFinders perf: removed call of co_positions fix: removed code without meaning refactor: removed some ifs which where never true and improved documentation refactor: applied changes requested in review and simplified code refactor: added instruction() and opname() methods fix: STORE_ATTR with multiline fix: DELETE_ATTR with multiline fix: multiline method calls test: limited new tests to python 3.11 and added some more linebreaks refactor: do attribute lookups always without lineno col_offset attribute comparison test: call more things fix: do not return Assert nodes, because they are caused by pytest assert rewriting fix: this allowed the profiling with cProfile but let the python 2.7 tests fail if you run them twice feat: cache get_instructions result fix: always try to find a node with all positions (required for some edge cases) fix: workaround for python/cpython#95921 fix: EXTENDED_ARG is used for classes with lots of decorators fix: fixed some pattern matching related issues test: test with and listcomp fix: fix issues with master fix: handle assert-nodes as KnownIssue and limit the cases which can be fixed fix: handle exception cleanup for unmapped bytecodes fix: fix chain comparisons and report a KnownIssue if not possible fix: ignore pattern matching for now feat: implemented deadcode detection feat: implemented verifier test: improved tests refactor: extracted PositionNodeFinder into _position_node_finder.py refactor: used 3.11 syntax for _position_node_finder.py refactor: use lru_cache refactor: restructured special cases docu: explained __classcell__ issue refactor: splitted __init__ refactor: created is_except_cleanup staticmethod refactor: simplified inst_match refactor: created parents helper method refactor: improved and tested mangled_name refactor: unified verification for % string formatting refactor: unified verification for function call and class definition doc: added missing todo refactor: use Tester to test with statement refactor: moved and renamed start() and end() fix: not can be found in some cases fix: `is/is not None` can be found in some cases fix: removed duplicate code fix: fix `test_with` syntax for older python versions fix: specify python version fix: skip assert only for cpython 3.11 refactor: removed unnecessary try refactor: some cleanup in _position_node_finder.py fix: extended tests and also mangle ExceptHandler.name test: deleted redundant test fix: check correct deadcode annotation and fixed implementation fix: with statement fix: use is to compare with True/False fix: renamed _case to case_ fix: removed unused variables refactor: moved contains_break outside fix(deadcode): if condition is not dead based on the condition itself fix(verify): missing node for string concatenation fix: `while True:` can be broken by a `break` inside a `else:` block of a inner loop fix: __debug__ does not generate a LOAD instruction fix: some more `not ...` and `not x is x` do not generate instructions fix: classes where the mangeled names are in the bases do not count for name mangeling fix: not in `assert a if cnd else not b` can not be found, because it is part of the control flow fix: attribute gets not mangled if class name is `_` fix: tuples of literals are also literals fix: fixed bug in decorator detection fix: ExceptionHandler names can be mangled fix: also handle formatting cases like '%50s'%(a,) fix: `except TypeError as ValueError:` generated a STORE_GLOBAL fix: catch ValueError thrown by `ast.parse("\0")` fix: improved deadcode analysis build: moved test dependencies to setup.cfg test: extended test for multiline method calls fix: use previous non-CACHE instruction if f_lasti is a CACHE instruction

15r10nk added the type-bug An unexpected behavior, bug, or error label Aug 12, 2022

15r10nk mentioned this issue Aug 12, 2022

feat: support for python 3.11 alexmojaki/executing#31

Merged

15r10nk added a commit to 15r10nk/executing that referenced this issue Aug 28, 2022

fix: workaround for python/cpython#95921

364dafc

15r10nk changed the title ~~comparison chaining in assertions produces wrong source positions in bytecode~~ comparison chaining has wrong source positions in most contexts (python 3.11.0rc2) Sep 18, 2022

brandtbucher self-assigned this Sep 18, 2022

bedevere-bot mentioned this issue Sep 20, 2022

GH-95921: Fix positions for some chained comparisons #96968

Merged

brandtbucher added a commit that referenced this issue Sep 20, 2022

GH-95921: Fix positions for some chained comparisons (GH-96968)

dfc73b5

brandtbucher added a commit to brandtbucher/cpython that referenced this issue Sep 20, 2022

pythonGH-95921: Fix positions for some chained comparisons (pythonGH-…

da93813

…96968) (cherry picked from commit dfc73b5)

brandtbucher added a commit to brandtbucher/cpython that referenced this issue Sep 20, 2022

[3.11] pythonGH-95921: Fix positions for some chained comparisons (py…

51ea608

…thonGH-96968). (cherry picked from commit dfc73b5) Co-authored-by: Brandt Bucher <brandtbucher@microsoft.com>

brandtbucher added a commit to brandtbucher/cpython that referenced this issue Sep 20, 2022

[3.11] pythonGH-95921: Fix positions for some chained comparisons (py…

733ccf4

…thonGH-96968). (cherry picked from commit dfc73b5) Co-authored-by: Brandt Bucher <brandtbucher@microsoft.com>

bedevere-bot mentioned this issue Sep 20, 2022

[3.11] GH-95921: Fix positions for some chained comparisons (GH-96968) #96973

Merged

brandtbucher added a commit to brandtbucher/cpython that referenced this issue Sep 20, 2022

[3.10] pythonGH-95921: Fix positions for some chained comparisons (py…

292910f

…thonGH-96968). (cherry picked from commit dfc73b5) Co-authored-by: Brandt Bucher <brandtbucher@microsoft.com>

bedevere-bot mentioned this issue Sep 20, 2022

[3.10] GH-95921: Fix positions for some chained comparisons (GH-96968) #96974

Merged

miss-islington pushed a commit that referenced this issue Sep 20, 2022

[3.11] GH-95921: Fix positions for some chained comparisons (GH-96968) (

0e61d2b

GH-96973) (cherry picked from commit dfc73b5) Automerge-Triggered-By: GH:brandtbucher

miss-islington pushed a commit that referenced this issue Sep 20, 2022

[3.10] GH-95921: Fix positions for some chained comparisons (GH-96968) (

aced809

GH-96974) (cherry picked from commit dfc73b5) Automerge-Triggered-By: GH:brandtbucher

brandtbucher closed this as completed Sep 20, 2022

15r10nk mentioned this issue Sep 21, 2022

Investigate identifying chained comparisons alexmojaki/executing#47

Open

brandtbucher reopened this Oct 12, 2022

iritkatriel mentioned this issue Oct 18, 2022

gh-98390: Fix source locations of boolean sub-expressions #98396

Merged

Nov	DEC	Jan
	18
2021	2022	2023

comparison chaining has wrong source positions in most contexts (python 3.11.0rc2) #95921

comparison chaining has wrong source positions in most contexts (python 3.11.0rc2) #95921

15r10nk commented Aug 12, 2022 •

edited

15r10nk commented Sep 18, 2022

brandtbucher commented Sep 18, 2022

brandtbucher commented Sep 20, 2022

15r10nk commented Sep 21, 2022

15r10nk commented Sep 21, 2022

brandtbucher commented Sep 22, 2022

comparison chaining has wrong source positions in most contexts (python 3.11.0rc2) #95921

comparison chaining has wrong source positions in most contexts (python 3.11.0rc2) #95921

Comments

15r10nk commented Aug 12, 2022 • edited

problem

no problem

reason

other examples

expected result

15r10nk commented Sep 18, 2022

brandtbucher commented Sep 18, 2022

brandtbucher commented Sep 20, 2022

15r10nk commented Sep 21, 2022

15r10nk commented Sep 21, 2022

brandtbucher commented Sep 22, 2022

15r10nk commented Aug 12, 2022 •

edited