diff --git a/python/minimal_test.py b/python/minimal_test.py index 32b83338f6..0c73a4b1a8 100644 --- a/python/minimal_test.py +++ b/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 diff --git a/python/repeated.c b/python/repeated.c index 6e96ccf399..7d797f2a31 100644 --- a/python/repeated.c +++ b/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; }