sys.monitoring branches for `match` cases showing incorrectly

[jaltmayerpizzorno]

Bug report

Bug description:

With the code from PR 122564, the destinations of branches into the blocks of certain cases are mapping to the pattern as well, making them look like they go out of scope:

v = 1
match v:
    case 1:
        x = 1
    case 2:
        x = 2
    case 3:
        x = 3
x += 2

This code yields (branches listing below extracted from co_branches()):

  0          0       RESUME                   0

  1          2       LOAD_CONST               0 (1)
             4       STORE_NAME               0 (v)

  2          6       LOAD_NAME                0 (v)

  3          8       COPY                     1
            10       LOAD_CONST               0 (1)
            12       COMPARE_OP              88 (bool(==))
            16       POP_JUMP_IF_FALSE        5 (to L1)
            20       NOT_TAKEN
            22       POP_TOP

  4         24       LOAD_CONST               0 (1)
            26       STORE_NAME               1 (x)
            28       JUMP_FORWARD            19 (to L3)

  5   L1:   30       COPY                     1
            32       LOAD_CONST               1 (2)
            34       COMPARE_OP              88 (bool(==))
            38       POP_JUMP_IF_FALSE        5 (to L2)
            42       NOT_TAKEN
            44       POP_TOP

  6         46       LOAD_CONST               1 (2)
            48       STORE_NAME               1 (x)
            50       JUMP_FORWARD             8 (to L3)

  7   L2:   52       LOAD_CONST               2 (3)
            54       COMPARE_OP              88 (bool(==))
            58       POP_JUMP_IF_FALSE        3 (to L3)
            62       NOT_TAKEN

  8         64       LOAD_CONST               2 (3)
            66       STORE_NAME               1 (x)

  9   L3:   68       LOAD_NAME                1 (x)
            70       LOAD_CONST               1 (2)
            72       BINARY_OP               13 (+=)
            76       STORE_NAME               1 (x)
            78       RETURN_CONST             3 (None)
'<module>' branches:
    ex8.py 3:9-3:10 "1" (<module>@16) -> 3:9-3:10 "1" (<module>@22) [case -> out]
    ex8.py 3:9-3:10 "1" (<module>@16) -> 5:9-5:10 "2" (<module>@30) [case -> next case]
    ex8.py 5:9-5:10 "2" (<module>@38) -> 5:9-5:10 "2" (<module>@44) [case -> out]
    ex8.py 5:9-5:10 "2" (<module>@38) -> 7:9-7:10 "3" (<module>@52) [case -> next case]
    ex8.py 7:9-7:10 "3" (<module>@58) -> 8:12-8:13 "3" (<module>@64) [case -> body]
    ex8.py 7:9-7:10 "3" (<module>@58) -> 9:0-9:1 "x" (<module>@68) [case -> next]

@markshannon @nedbat

CPython versions tested on:

CPython main branch

Operating systems tested on:

macOS

Linked PRs

• GH-123044: Give the POP_TOP after a case test a location in the body, not the pattern. #130627

[gaogaotiantian]

Do we craete issues for unmerged PR? I thought issues are supposed to be about code that's already merged. For unmerged PRs, shouldn't we just discuss it in the PR itself?

[markshannon]

I don't think there is any harm in creating a new issue. We often break down larger pieces of work into smaller issues.
We should make it clear that this is part of #122564, though.

[markshannon]

I think we should tackle these issues of locations within match statement related issues as follows:

• Generally, we want to be giving POP_TOPs and other cleanup code NO_LOCATION in the front-end.
• When assigning locations to instructions with no locations we should prefer locations within the same basic block to those locations outside it.
• We also want the compiler to prefer locations following the basic block, to those preceding it, which assigning locations to basic blocks with no locations.

@iritkatriel does that make sense to you?

[iritkatriel]

We also want the compiler to prefer locations following the basic block, to those preceding it, which assigning locations to basic blocks with no locations.

I think it's doing the opposite now. Will need to experiment with this.

[markshannon]

Given that we cannot reliably expected NOT_TAKEN to be followed by an instruction with a meaningful location (see #123050) we'll need a different approach to this.

We still need correct locations for actual expressions, but we should be fine with the compiler filling in locations as it does now for artificial instructions.