[Python-checkins] bpo-44600: Fix line numbers for pattern matching cleanup code (GH-27346)

[brandtbucher]

https://github.com/python/cpython/commit/4214f470f0cb9b6fef9a90758756fbc00ba95b5a
commit: 4214f470f0cb9b6fef9a90758756fbc00ba95b5a
branch: main
author: Charles Burkland <charles.aburkland at gmail.com>
committer: brandtbucher <brandtbucher at gmail.com>
date: 2021-07-25T16:42:07-07:00
summary:

bpo-44600: Fix line numbers for pattern matching cleanup code (GH-27346)

files:
A Misc/NEWS.d/next/Security/2021-07-25-20-04-54.bpo-44600.0WMldg.rst
M Lib/test/test_patma.py
M Python/compile.c

diff --git a/Lib/test/test_patma.py b/Lib/test/test_patma.py
index 69a648a5a79fae..96c1726b0f3c9d 100644
--- a/Lib/test/test_patma.py
+++ b/Lib/test/test_patma.py
@@ -3,6 +3,7 @@
 import dataclasses
 import enum
 import inspect
+import sys
 import unittest
 
 
@@ -3056,6 +3057,81 @@ class Keys:
         self.assertIs(z, None)
 
 
+class TestTracing(unittest.TestCase):
+
+    def _test_trace(self, func, expected_linenos, *f_args):
+        actual_linenos = set()
+        def trace(frame, event, arg):
+            if frame.f_code.co_name == func.__name__:
+                relative_lineno = frame.f_lineno - func.__code__.co_firstlineno
+                actual_linenos.add(relative_lineno)
+            return trace
+
+        sys.settrace(trace)
+        func(*f_args)
+        sys.settrace(None)
+        self.assertSetEqual(actual_linenos, expected_linenos)
+
+    def test_default_case_traces_correctly_a(self):
+        def default_no_assign(command):                        # 0
+            match command.split():                             # 1
+                case ["go", direction] if direction in "nesw": # 2
+                    return f"go {direction}"                   # 3
+                case ["go", _]:                                # 4
+                    return "no go"                             # 5
+                case _:                                        # 6
+                    return "default"                           # 7
+
+        self._test_trace(default_no_assign, {0, 1, 2, 3}, "go n")
+        self._test_trace(default_no_assign, {0, 1, 2, 4, 5}, "go x")
+        self._test_trace(default_no_assign, {0, 1, 2, 4, 6, 7}, "spam")
+
+    def test_default_case_traces_correctly_b(self):
+        def default_wildcard_assign(command):                  # 0
+            match command.split():                             # 1
+                case ["go", direction] if direction in "nesw": # 2
+                    return f"go {direction}"                   # 3
+                case ["go", _]:                                # 4
+                    return "no go"                             # 5
+                case x:                                        # 6
+                    return x                                   # 7
+
+        self._test_trace(default_wildcard_assign, {0, 1, 2, 3}, "go n")
+        self._test_trace(default_wildcard_assign, {0, 1, 2, 4, 5}, "go x")
+        self._test_trace(default_wildcard_assign, {0, 1, 2, 4, 6, 7}, "spam")
+
+    def test_default_case_traces_correctly_c(self):
+        def no_default(command):                               # 0
+            match command.split():                             # 1
+                case ["go", direction] if direction in "nesw": # 2
+                    return f"go {direction}"                   # 3
+                case ["go", _]:                                # 4
+                    return "no go"                             # 5
+
+        self._test_trace(no_default, {0, 1, 2, 3}, "go n")
+        self._test_trace(no_default, {0, 1, 2, 4, 5}, "go x")
+        self._test_trace(no_default, {0, 1, 2, 4}, "spam")
+
+    def test_default_case_traces_correctly_d(self):
+        def only_default_no_assign(command):  # 0
+            match command.split():            # 1
+                case _:                       # 2
+                    return "default"          # 3
+
+        self._test_trace(only_default_no_assign, {0, 1, 2, 3}, "go n")
+        self._test_trace(only_default_no_assign, {0, 1, 2, 3} , "go x")
+        self._test_trace(only_default_no_assign, {0, 1, 2, 3}, "spam")
+
+    def test_default_case_traces_correctly_e(self):
+        def only_default_wildcard_assign(command):  # 0
+            match command.split():                  # 1
+                case x:                             # 2
+                    return x                        # 3
+
+        self._test_trace(only_default_wildcard_assign, {0, 1, 2, 3}, "go n")
+        self._test_trace(only_default_wildcard_assign, {0, 1, 2, 3} , "go x")
+        self._test_trace(only_default_wildcard_assign, {0, 1, 2, 3}, "spam")
+
 if __name__ == "__main__":
     """
     # From inside environment using this Python, with pyperf installed:
diff --git a/Misc/NEWS.d/next/Security/2021-07-25-20-04-54.bpo-44600.0WMldg.rst b/Misc/NEWS.d/next/Security/2021-07-25-20-04-54.bpo-44600.0WMldg.rst
new file mode 100644
index 00000000000000..ea4e04f6da911c
--- /dev/null
+++ b/Misc/NEWS.d/next/Security/2021-07-25-20-04-54.bpo-44600.0WMldg.rst
@@ -0,0 +1 @@
+Fix incorrect line numbers while tracing some failed patterns in :ref:`match <match>` statements. Patch by Charles Burkland.
\ No newline at end of file
diff --git a/Python/compile.c b/Python/compile.c
index 3a20f6b57eb367..7fb8abf7749bbe 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -6576,17 +6576,25 @@ compiler_match_inner(struct compiler *c, stmt_ty s, pattern_context *pc)
         }
         VISIT_SEQ(c, stmt, m->body);
         ADDOP_JUMP(c, JUMP_FORWARD, end);
+        // If the pattern fails to match, we want the line number of the
+        // cleanup to be associated with the failed pattern, not the last line
+        // of the body
+        SET_LOC(c, m->pattern);
         RETURN_IF_FALSE(emit_and_reset_fail_pop(c, pc));
     }
     if (has_default) {
-        if (cases == 1) {
-            // No matches. Done with the subject:
-            ADDOP(c, POP_TOP);
-        }
         // A trailing "case _" is common, and lets us save a bit of redundant
         // pushing and popping in the loop above:
         m = asdl_seq_GET(s->v.Match.cases, cases - 1);
         SET_LOC(c, m->pattern);
+        if (cases == 1) {
+            // No matches. Done with the subject:
+            ADDOP(c, POP_TOP);
+        }
+        else {
+            // Show line coverage for default case (it doesn't create bytecode)
+            ADDOP(c, NOP);
+        }
         if (m->guard) {
             RETURN_IF_FALSE(compiler_jump_if(c, m->guard, end, 0));
         }