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 match sub-expressions are incorrect #98762

Closed
iritkatriel opened this issue Oct 27, 2022 · 2 comments
Closed

Source location of match sub-expressions are incorrect #98762

iritkatriel opened this issue Oct 27, 2022 · 2 comments
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 27, 2022

def f(x):
  match x:
    case a,b:
        return 1

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 is:

[((2, 2, 0, 0), 'RESUME', 0),
 ((3, 3, 8, 9), 'LOAD_FAST', 'x'),
 ((4, 4, 9, 12), 'MATCH_SEQUENCE', None),
 ((4, 4, 9, 12), 'POP_JUMP_IF_FALSE', 32),
 ((4, 4, 9, 12), 'GET_LEN', None),
 ((4, 4, 9, 12), 'LOAD_CONST', 2),
 ((4, 4, 9, 12), 'COMPARE_OP', '=='),
 ((4, 4, 9, 12), 'POP_JUMP_IF_FALSE', 32),
 ((4, 4, 9, 12), 'UNPACK_SEQUENCE', 2),
 ((4, 4, 11, 12), 'STORE_FAST', 'a'),           <-- incorrect
 ((4, 4, 11, 12), 'STORE_FAST', 'b'),
 ((5, 5, 15, 16), 'LOAD_CONST', 1),
 ((5, 5, 15, 16), 'RETURN_VALUE', None),
 ((4, 4, 9, 12), 'POP_TOP', None),
 ((4, 4, 9, 12), 'LOAD_CONST', None),
 ((4, 4, 9, 12), 'RETURN_VALUE', None)]
@iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.12 bugs and security fixes labels Oct 27, 2022
@iritkatriel iritkatriel self-assigned this Oct 27, 2022
@iritkatriel
Copy link
Member Author

FYI @nedbat .

@iritkatriel iritkatriel changed the title Source location of sequence match assignments are incorrect Source location of match sub-expressions are incorrect Oct 27, 2022
iritkatriel added a commit to iritkatriel/cpython that referenced this issue Oct 27, 2022
@brandtbucher
Copy link
Member

The issue with name stores is pretty tricky, and stems from the fact that names are only bound once the entire pattern has matched. Fixing the locations for name stores in "normal" patterns won't be too hard (just keep the original node nearby and use that location for the final store).

However, or-patterns pose a challenge. For example, what should the location be when storing a and b?

match ...:
    case [True, a, b] | [False, b, a]:
       ...

There's only one store instruction for each (at the very end), so I still think "the whole pattern" makes sense.

@brandtbucher brandtbucher self-assigned this Nov 18, 2022
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

No branches or pull requests

2 participants