Added tests for slice deletion and fixed bugs.

python/minimal_test.py

@@ -30,6 +30,7 @@
 import unittest
 from google.protobuf.pyext import _message
 from google.protobuf.internal import api_implementation
+from google.protobuf import unittest_pb2
 
 class TestMessageExtension(unittest.TestCase):
 
@@ -53,6 +54,21 @@ class TestMessageExtension(unittest.TestCase):
         self.assertTrue(_message._IS_UPB)
         self.assertEqual(api_implementation.Type(), "cpp")
 
+    def test_repeated_field_slice_delete(self):
+        def test_slice(start, end, step):
+            vals = list(range(20))
+            message = unittest_pb2.TestAllTypes(repeated_int32=vals)
+            del vals[start:end:step]
+            del message.repeated_int32[start:end:step]
+            self.assertEqual(vals, list(message.repeated_int32))
+        test_slice(3, 11, 1)
+        test_slice(3, 11, 2)
+        test_slice(3, 11, 3)
+        test_slice(11, 3, -1)
+        test_slice(11, 3, -2)
+        test_slice(11, 3, -3)
+        test_slice(10, 25, 4)
+
 #TestMessageExtension.test_descriptor_pool.__unittest_expecting_failure__ = True
 
 

python/repeated.c

@@ -79,8 +79,8 @@ typedef struct {
   //   - low bit clear: repeated field is reified (points to upb_array).
   uintptr_t field;
   union {
-    upb_array* arr;    // stub: the data for this array.
-    PyObject* parent;  // reified: owning pointer to parent message.
+    PyObject* parent;  // stub: owning pointer to parent message.
+    upb_array* arr;    // reified: the data for this array.
   } ptr;
 } PyUpb_RepeatedContainer;
 
@@ -371,19 +371,31 @@ static int PyUpb_RepeatedContainer_DeleteSubscript(upb_array* arr,
     start = end;
     step = -step;
   }
+
   size_t dst = start;
-  size_t src = step > 1 ? start + 1 : start + count;
+  size_t src;
   if (step > 1) {
-    // Move elements between steps.
-    for (Py_ssize_t i = 0; i < count; i++, dst += step, src += step + 1) {
+    // Move elements between steps:
+    //
+    //        src
+    //         |
+    // |------X---X---X---X------------------------------|
+    //        |
+    //       dst           <-------- tail -------------->
+    src = start + 1;
+    for (Py_ssize_t i = 1; i < count; i++, dst += step - 1, src += step) {
       upb_array_move(arr, dst, src, step);
     }
+  } else {
+    src = start + count;
   }
+
   // Move tail.
   size_t tail = upb_array_size(arr) - src;
-  assert(dst + tail == upb_array_size(arr) - count);
-  upb_array_move(arr, dst, src, upb_array_size(arr) - src);
-  upb_array_resize(arr, dst + tail, NULL);
+  size_t new_size = dst + tail;
+  assert(new_size == upb_array_size(arr) - count);
+  upb_array_move(arr, dst, src, tail);
+  upb_array_resize(arr, new_size, NULL);
   return 0;
 }