sys.monitoring branches for match cases showing incorrectly

@markshannon

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

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?

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.

@iritkatriel

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?

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.

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.

GH-123044: Give the `POP_TOP` after a case test a location in the bod…

Navigation Menu

Repository navigation

sys.monitoring branches for `match` cases showing incorrectly #123044

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

sys.monitoring branches for match cases showing incorrectly #123044

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

gaogaotiantian commented on Aug 16, 2024

markshannon commented on Aug 16, 2024

markshannon commented on Aug 16, 2024

iritkatriel commented on Aug 16, 2024

markshannon commented on Aug 20, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

sys.monitoring branches for `match` cases showing incorrectly #123044