Address feedback from @picnixz

vberlier · vberlier · commit 5f67be092216 · 2025-05-16T01:57:10.000+02:00
diff --git a/Doc/library/re.rst b/Doc/library/re.rst
@@ -1494,7 +1494,7 @@ You can also destructure match objects with python's ``match`` statement::
 
    .. versionadded:: 3.6
 
-   .. versionchanged:: 3.14
+   .. versionchanged:: next
 
       Negative indexing is now supported. This allows accessing match groups
       from the end, starting from the last group defined in the pattern::
@@ -1523,7 +1523,7 @@ You can also destructure match objects with python's ``match`` statement::
       >>> len(m)
       3
 
-   .. versionadded:: 3.14
+   .. versionadded:: next
 
 
 .. method:: Match.groups(default=None)
@@ -1596,13 +1596,13 @@ You can also destructure match objects with python's ``match`` statement::
 
    Raises :exc:`ValueError` if the value is not present.
 
-   .. versionadded:: 3.14
+   .. versionadded:: next
 
 .. method:: Match.count(value, /)
 
    Return the number of occurrences of the value among the matched groups.
 
-   .. versionadded:: 3.14
+   .. versionadded:: next
 
 .. attribute:: Match.pos
 
diff --git a/Lib/test/test_re.py b/Lib/test/test_re.py
@@ -8,6 +8,7 @@
 import sys
 import unittest
 import warnings
+from collections.abc import Sequence
 from re import Scanner
 from weakref import proxy
 
@@ -598,40 +599,47 @@ def test_match_getitem(self):
         self.assertEqual(m[1], 'a')
         self.assertEqual(m[2], None)
         self.assertEqual(m[3], 'c')
+        self.assertEqual(m[-1], 'c')
+        self.assertEqual(m[-2], None)
+        self.assertEqual(m[-3], 'a')
+        self.assertEqual(m[-4], 'ac')
 
         # Cannot assign.
         with self.assertRaises(TypeError):
             m[0] = 1
 
-    def test_match_sequence(self):
-        from collections.abc import Sequence
-
+        # Slices.
         m = re.match(r"(a)(b)(c)", "abc")
-        self.assertIsInstance(m, Sequence)
-        self.assertEqual(len(m), 4)
-
-        self.assertEqual(m[0], "abc")
-        self.assertEqual(m[1], "a")
-        self.assertEqual(m[2], "b")
-        self.assertEqual(m[3], "c")
-        with self.assertRaises(IndexError):
-            _ = m[4]
-
-        self.assertEqual(m[-1], "c")
-        self.assertEqual(m[-2], "b")
-        self.assertEqual(m[-3], "a")
-        self.assertEqual(m[-4], "abc")
-        with self.assertRaises(IndexError):
-            _ = m[-5]
-
+        self.assertEqual(m[:0], ())
+        self.assertEqual(m[:1], ("abc",))
+        self.assertEqual(m[:2], ("abc", "a"))
+        self.assertEqual(m[:3], ("abc", "a", "b"))
+        self.assertEqual(m[:4], ("abc", "a", "b", "c"))
+        self.assertEqual(m[0:], ("abc", "a", "b", "c"))
+        self.assertEqual(m[1:], ("a", "b", "c"))
+        self.assertEqual(m[2:], ("b", "c"))
+        self.assertEqual(m[3:], ("c",))
+        self.assertEqual(m[4:], ())
+        self.assertEqual(m[:-4], ())
+        self.assertEqual(m[:-3], ("abc",))
+        self.assertEqual(m[:-2], ("abc", "a"))
+        self.assertEqual(m[:-1], ("abc", "a", "b"))
+        self.assertEqual(m[-4:], ("abc", "a", "b", "c"))
+        self.assertEqual(m[-3:], ("a", "b", "c"))
+        self.assertEqual(m[-2:], ("b", "c"))
+        self.assertEqual(m[-1:], ("c",))
         self.assertEqual(m[1:-1], ("a", "b"))
         self.assertEqual(m[::-1], ("c", "b", "a", "abc"))
+        self.assertEqual(m[::4], ("abc",))
+        self.assertEqual(m[2:2], ())
+        self.assertEqual(m[3:1], ())
+        self.assertEqual(m[1:3], ("a", "b"))
+        self.assertEqual(m[-1::-2], ("c", "a"))
 
-        it = iter(m)
-        self.assertEqual(next(it), "abc")
-        self.assertEqual(next(it), "a")
-        self.assertEqual(next(it), "b")
-        self.assertEqual(next(it), "c")
+    def test_match_sequence(self):
+        m = re.match(r"(a)(b)(c)", "abc")
+        self.assertIsInstance(m, Sequence)
+        self.assertEqual(len(m), 4)
 
         self.assertEqual(tuple(m), ("abc", "a", "b", "c"))
         self.assertEqual(list(m), ["abc", "a", "b", "c"])
@@ -650,36 +658,97 @@ def test_match_sequence(self):
 
         self.assertEqual(list(reversed(m)), ["c", "b", "a", "abc"])
 
+        for s, k, v in re.finditer(r"(\w+):(\w+)", "abc:123"):
+            self.assertEqual(s, "abc:123")
+            self.assertEqual(k, "abc")
+            self.assertEqual(v, "123")
+
+    def test_match_iter(self):
+        m = re.match(r"(a)(b)(c)", "abc")
+        it = iter(m)
+        self.assertEqual(next(it), "abc")
+        self.assertEqual(next(it), "a")
+        self.assertEqual(next(it), "b")
+        self.assertEqual(next(it), "c")
+        with self.assertRaises(StopIteration):
+            next(it)
+
+    def test_match_index(self):
+        m = re.match(r"(a)(b)(c)", "abc")
         self.assertEqual(m.index("abc"), 0)
         self.assertEqual(m.index("a"), 1)
         self.assertEqual(m.index("b"), 2)
         self.assertEqual(m.index("c"), 3)
         self.assertRaises(ValueError, m.index, "123")
 
+        # With start index.
+        self.assertRaises(ValueError, m.index, "abc", 1)
+        self.assertEqual(m.index("a", 1), 1)
+        self.assertEqual(m.index("b", 1), 2)
+        self.assertEqual(m.index("c", 1), 3)
+        self.assertRaises(ValueError, m.index, "123", 1)
+
+        self.assertRaises(ValueError, m.index, "abc", 2)
+        self.assertRaises(ValueError, m.index, "a", 2)
+        self.assertEqual(m.index("b", 2), 2)
+        self.assertEqual(m.index("c", 2), 3)
+        self.assertRaises(ValueError, m.index, "123", 2)
+
+        self.assertRaises(ValueError, m.index, "abc", 3)
+        self.assertRaises(ValueError, m.index, "a", 3)
+        self.assertRaises(ValueError, m.index, "b", 3)
+        self.assertEqual(m.index("c", 3), 3)
+        self.assertRaises(ValueError, m.index, "123", 3)
+
+        self.assertRaises(ValueError, m.index, "abc", 4)
+        self.assertRaises(ValueError, m.index, "a", 4)
+        self.assertRaises(ValueError, m.index, "b", 4)
+        self.assertRaises(ValueError, m.index, "c", 4)
+        self.assertRaises(ValueError, m.index, "123", 4)
+
+        # With start index and stop index.
+        self.assertRaises(ValueError, m.index, "b", 0, 2)
+        self.assertEqual(m.index("b", 1, 3), 2)
+        self.assertEqual(m.index("b", 2, 4), 2)
+        self.assertRaises(ValueError, m.index, "b", 3, 4)
+        self.assertRaises(ValueError, m.index, "b", -1, 0)
+
+        # Non-string objects.
+        self.assertRaises(ValueError, m.index, 123)
+        self.assertRaises(ValueError, m.index, [1, 2, 3])
+        self.assertRaises(ValueError, m.index, object())
+
+    def test_match_count(self):
+        m = re.match(r"(a)(b)(c)", "abc")
         self.assertEqual(m.count("abc"), 1)
         self.assertEqual(m.count("a"), 1)
         self.assertEqual(m.count("b"), 1)
         self.assertEqual(m.count("c"), 1)
         self.assertEqual(m.count("123"), 0)
 
+        # Non-string objects.
+        self.assertEqual(m.count(123), 0)
+        self.assertEqual(m.count([1, 2, 3]), 0)
+        self.assertEqual(m.count(object()), 0)
+
+    def test_match_match_case(self):
+        m = re.match(r"(a)(b)(c)", "abc")
+
         match m:
-            case [_, "a", "b", "c"]:
-                pass
+            case [abc, "a", "b", "c"]:
+                self.assertEqual(abc, "abc")
             case _:
                 self.fail()
 
         match re.match(r"(\d+)-(\d+)-(\d+)", "2025-05-07"):
-            case [_, year, month, day]:
+            case [date, year, month, day]:
+                self.assertEqual(date, "2025-05-07")
                 self.assertEqual(year, "2025")
                 self.assertEqual(month, "05")
                 self.assertEqual(day, "07")
             case _:
                 self.fail()
 
-        for s, k, v in re.finditer(r"(\w+):(\w+)", "abc:123"):
-            self.assertEqual(s, "abc:123")
-            self.assertEqual(k, "abc")
-            self.assertEqual(v, "123")
 
     def test_re_fullmatch(self):
         # Issue 16203: Proposal: add re.fullmatch() method.
diff --git a/Modules/_sre/sre.c b/Modules/_sre/sre.c
@@ -2453,7 +2453,7 @@ match_item(PyObject *op, Py_ssize_t index)
 }
 
 static PyObject*
-match_subscript(PyObject *op, PyObject* item)
+match_getitem(PyObject *op, PyObject* item)
 {
     MatchObject *self = _MatchObject_CAST(op);
 
@@ -2491,7 +2491,10 @@ match_subscript(PyObject *op, PyObject* item)
         if (self->pattern->groupindex) {
             PyObject* index = PyDict_GetItemWithError(self->pattern->groupindex, item);
             if (index && PyLong_Check(index)) {
-                return match_item(op, PyLong_AsSsize_t(index));
+                Py_ssize_t i = PyLong_AsSsize_t(index);
+                if (i != -1) {
+                    return match_item(op, i);
+                }
             }
         }
         if (!PyErr_Occurred()) {
@@ -2695,33 +2698,21 @@ _sre_SRE_Match_index_impl(MatchObject *self, PyObject *value,
                           Py_ssize_t start, Py_ssize_t stop)
 /*[clinic end generated code: output=846597f6f96f829c input=7f41b5a99e0ad88e]*/
 {
-    Py_ssize_t i;
+    PySlice_AdjustIndices(self->groups, &start, &stop, 1);
 
-    if (start < 0) {
-        start += self->groups;
-        if (start < 0) {
-            start = 0;
-        }
-    }
-    if (stop < 0) {
-        stop += self->groups;
-    }
-    else if (stop > self->groups) {
-        stop = self->groups;
-    }
-    for (i = start; i < stop; i++) {
+    for (Py_ssize_t i = start; i < stop; i++) {
         PyObject* group = match_getslice_by_index(self, i, Py_None);
         if (group == NULL) {
             return NULL;
         }
         int cmp = PyObject_RichCompareBool(group, value, Py_EQ);
         Py_DECREF(group);
+        if (cmp < 0) {
+            return NULL;
+        }
         if (cmp > 0) {
             return PyLong_FromSsize_t(i);
         }
-        else if (cmp < 0) {
-            return NULL;
-        }
     }
     PyErr_SetString(PyExc_ValueError, "match.index(x): x not in match");
     return NULL;
@@ -2750,12 +2741,12 @@ _sre_SRE_Match_count_impl(MatchObject *self, PyObject *value)
         }
         int cmp = PyObject_RichCompareBool(group, value, Py_EQ);
         Py_DECREF(group);
+        if (cmp < 0) {
+            return NULL;
+        }
         if (cmp > 0) {
             count++;
         }
-        else if (cmp < 0) {
-            return NULL;
-        }
     }
     return PyLong_FromSsize_t(count);
 }
@@ -3421,7 +3412,7 @@ static PyType_Slot match_slots[] = {
     {Py_sq_item, match_item},
 
     // Support group names provided as subscripts
-    {Py_mp_subscript, match_subscript},
+    {Py_mp_subscript, match_getitem},
 
     {0, NULL},
 };
