]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
GH-123044: Give the `POP_TOP` after a case test a location in the body, not the patte...
authorMark Shannon <mark@hotpy.org>
Mon, 10 Mar 2025 17:31:16 +0000 (17:31 +0000)
committerGitHub <noreply@github.com>
Mon, 10 Mar 2025 17:31:16 +0000 (17:31 +0000)
Include/internal/pycore_symtable.h
Lib/test/test_monitoring.py
Misc/NEWS.d/next/Core_and_Builtins/2025-02-27-10-47-09.gh-issue-123044.8182Un.rst [new file with mode: 0644]
Python/assemble.c
Python/codegen.c
Python/flowgraph.c

index 3b87a7f869c4d743f8816b39c80e7923d3764159..fd6b9bcbe5abd878be8ff0acf35f0783d4fa3134 100644 (file)
@@ -58,6 +58,7 @@ typedef struct {
                .end_col_offset = (n)->end_col_offset }
 
 static const _Py_SourceLocation NO_LOCATION = {-1, -1, -1, -1};
+static const _Py_SourceLocation NEXT_LOCATION = {-2, -2, -2, -2};
 
 /* __future__ information */
 typedef struct {
index fc98b9eee383e4d66e7bea3277cd9e11d6be58da..23b0d56c767b4f0433cbb1e5d676093e0a8677b8 100644 (file)
@@ -1681,6 +1681,35 @@ class TestBranchAndJumpEvents(CheckEvents):
             ('branch left', 'func', 12, 12)])
 
 
+    def test_match(self):
+
+        def func(v=1):
+            x = 0
+            for v in range(4):
+                match v:
+                    case 1:
+                        x += 1
+                    case 2:
+                        x += 2
+                    case _:
+                        x += 3
+            return x
+
+        self.check_events(func, recorders = BRANCHES_RECORDERS, expected = [
+            ('branch left', 'func', 2, 2),
+            ('branch right', 'func', 4, 6),
+            ('branch right', 'func', 6, 8),
+            ('branch left', 'func', 2, 2),
+            ('branch left', 'func', 4, 5),
+            ('branch left', 'func', 2, 2),
+            ('branch right', 'func', 4, 6),
+            ('branch left', 'func', 6, 7),
+            ('branch left', 'func', 2, 2),
+            ('branch right', 'func', 4, 6),
+            ('branch right', 'func', 6, 8),
+            ('branch right', 'func', 2, 10)])
+
+
 class TestBranchConsistency(MonitoringTestBase, unittest.TestCase):
 
     def check_branches(self, run_func, test_func=None, tool=TEST_TOOL, recorders=BRANCH_OFFSET_RECORDERS):
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-02-27-10-47-09.gh-issue-123044.8182Un.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-02-27-10-47-09.gh-issue-123044.8182Un.rst
new file mode 100644 (file)
index 0000000..75ad311
--- /dev/null
@@ -0,0 +1,2 @@
+Make sure that the location of branch targets in ``match`` cases is in the
+body, not the pattern.
index e33918edf8e4b906729ac03e74b92bf9ee8b3cf9..6fdc0c86c0fe39487ddcbec490a31469d8eb7395 100644 (file)
@@ -291,6 +291,7 @@ write_location_info_entry(struct assembler* a, location loc, int isize)
         RETURN_IF_ERROR(_PyBytes_Resize(&a->a_linetable, len*2));
     }
     if (loc.lineno < 0) {
+        assert(loc.lineno == NO_LOCATION.lineno);
         write_location_info_none(a, isize);
         return SUCCESS;
     }
@@ -341,6 +342,18 @@ assemble_location_info(struct assembler *a, instr_sequence *instrs,
 {
     a->a_lineno = firstlineno;
     location loc = NO_LOCATION;
+    for (int i = instrs->s_used-1; i >= 0; i--) {
+        instruction *instr = &instrs->s_instrs[i];
+        if (same_location(instr->i_loc, NEXT_LOCATION)) {
+            if (IS_TERMINATOR_OPCODE(instr->i_opcode)) {
+                instr->i_loc = NO_LOCATION;
+            }
+            else {
+                assert(i < instrs->s_used-1);
+                instr->i_loc = instr[1].i_loc;
+            }
+        }
+    }
     int size = 0;
     for (int i = 0; i < instrs->s_used; i++) {
         instruction *instr = &instrs->s_instrs[i];
index 7a3f787aec0d2d3a42822f3d409a3df3f8b3a3b7..908711250d14bcb164f17fc77f1a3c0f7bd7f374 100644 (file)
@@ -6124,7 +6124,8 @@ codegen_match_inner(compiler *c, stmt_ty s, pattern_context *pc)
         }
         // Success! Pop the subject off, we're done with it:
         if (i != cases - has_default - 1) {
-            ADDOP(c, LOC(m->pattern), POP_TOP);
+            /* Use the next location to give better locations for branch events */
+            ADDOP(c, NEXT_LOCATION, POP_TOP);
         }
         VISIT_SEQ(c, stmt, m->body);
         ADDOP_JUMP(c, NO_LOCATION, JUMP, end);
index 6ba60d4312e56c1067a122fefcfa02003373ccc2..286f8999902e322faaa933a77072e34ddeedc2e7 100644 (file)
@@ -1078,8 +1078,8 @@ basicblock_remove_redundant_nops(basicblock *bb) {
                     location next_loc = NO_LOCATION;
                     for (int next_i=0; next_i < next->b_iused; next_i++) {
                         cfg_instr *instr = &next->b_instr[next_i];
-                        if (instr->i_opcode == NOP && instr->i_loc.lineno == NO_LOCATION.lineno) {
-                            /* Skip over NOPs without location, they will be removed */
+                        if (instr->i_opcode == NOP && instr->i_loc.lineno < 0) {
+                            /* Skip over NOPs without location, they will be removed */
                             continue;
                         }
                         next_loc = instr->i_loc;
@@ -2976,7 +2976,7 @@ propagate_line_numbers(basicblock *entryblock) {
 
         location prev_location = NO_LOCATION;
         for (int i = 0; i < b->b_iused; i++) {
-            if (b->b_instr[i].i_loc.lineno < 0) {
+            if (b->b_instr[i].i_loc.lineno == NO_LOCATION.lineno) {
                 b->b_instr[i].i_loc = prev_location;
             }
             else {
@@ -2985,7 +2985,7 @@ propagate_line_numbers(basicblock *entryblock) {
         }
         if (BB_HAS_FALLTHROUGH(b) && b->b_next->b_predecessors == 1) {
             if (b->b_next->b_iused > 0) {
-                if (b->b_next->b_instr[0].i_loc.lineno < 0) {
+                if (b->b_next->b_instr[0].i_loc.lineno == NO_LOCATION.lineno) {
                     b->b_next->b_instr[0].i_loc = prev_location;
                 }
             }
@@ -2993,7 +2993,7 @@ propagate_line_numbers(basicblock *entryblock) {
         if (is_jump(last)) {
             basicblock *target = last->i_target;
             if (target->b_predecessors == 1) {
-                if (target->b_instr[0].i_loc.lineno < 0) {
+                if (target->b_instr[0].i_loc.lineno == NO_LOCATION.lineno) {
                     target->b_instr[0].i_loc = prev_location;
                 }
             }