From 363fa89b1f02d4c51028f8c8bcd506f08eaa6b49 Mon Sep 17 00:00:00 2001 From: Andrei Tcaci Date: Fri, 10 Feb 2023 10:16:28 -0800 Subject: [PATCH] RepeatedField: unset by index (#11425) Instead of using `array_pop()` to remove last element use `unset()` to remove by index Closes #11425 COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/11425 from Leprechaunz:unset-by-index a47fba57e43228c92791aa75e1bc04169491bf41 PiperOrigin-RevId: 508691545 --- php/ext/google/protobuf/array.c | 4 +-- .../Protobuf/Internal/RepeatedField.php | 4 +-- php/tests/ArrayTest.php | 27 +++++++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/php/ext/google/protobuf/array.c b/php/ext/google/protobuf/array.c index 72c7809674c6..09daffa16129 100644 --- a/php/ext/google/protobuf/array.c +++ b/php/ext/google/protobuf/array.c @@ -406,13 +406,13 @@ PHP_METHOD(RepeatedField, offsetUnset) { return; } - if (size == 0 || index != size - 1) { + if (size == 0 || index < 0 || index >= size) { php_error_docref(NULL, E_USER_ERROR, "Cannot remove element at %ld.\n", index); return; } - upb_Array_Resize(intern->array, size - 1, Arena_Get(&intern->arena)); + upb_Array_Delete(intern->array, index, 1); } /** diff --git a/php/src/Google/Protobuf/Internal/RepeatedField.php b/php/src/Google/Protobuf/Internal/RepeatedField.php index ea7971f134fe..f6ecd1c84a95 100644 --- a/php/src/Google/Protobuf/Internal/RepeatedField.php +++ b/php/src/Google/Protobuf/Internal/RepeatedField.php @@ -219,13 +219,13 @@ public function offsetSet($offset, $value) public function offsetUnset($offset) { $count = count($this->container); - if (!is_numeric($offset) || $count === 0 || $offset !== $count - 1) { + if (!is_numeric($offset) || $count === 0 || $offset < 0 || $offset >= $count) { trigger_error( "Cannot remove element at the given index", E_USER_ERROR); return; } - array_pop($this->container); + array_splice($this->container, $offset, 1); } /** diff --git a/php/tests/ArrayTest.php b/php/tests/ArrayTest.php index 9e8fcb8beada..2cade79026ea 100644 --- a/php/tests/ArrayTest.php +++ b/php/tests/ArrayTest.php @@ -655,4 +655,31 @@ public function testClone() $arr2 = clone $arr; $this->assertSame($arr[0], $arr2[0]); } + + ######################################################### + # Test offsetUnset + ######################################################### + + public function testOffsetUnset() + { + $arr = new RepeatedField(GPBType::INT32); + $arr[] = 0; + $arr[] = 1; + $arr[] = 2; + + $this->assertSame(3, count($arr)); + $this->assertSame(0, $arr[0]); + $this->assertSame(1, $arr[1]); + $this->assertSame(2, $arr[2]); + + $arr->offsetUnset(1); + $this->assertSame(0, $arr[0]); + $this->assertSame(2, $arr[1]); + + $arr->offsetUnset(0); + $this->assertSame(2, $arr[0]); + + $arr->offsetUnset(0); + $this->assertCount(0, $arr); + } }