Skip to content

Commit 363fa89

Browse files
Leprechaunzcopybara-github
authored andcommitted
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=#11425 from Leprechaunz:unset-by-index a47fba5 PiperOrigin-RevId: 508691545
1 parent 6aefc47 commit 363fa89

File tree

3 files changed

+31
-4
lines changed

3 files changed

+31
-4
lines changed

php/ext/google/protobuf/array.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,13 +406,13 @@ PHP_METHOD(RepeatedField, offsetUnset) {
406406
return;
407407
}
408408

409-
if (size == 0 || index != size - 1) {
409+
if (size == 0 || index < 0 || index >= size) {
410410
php_error_docref(NULL, E_USER_ERROR, "Cannot remove element at %ld.\n",
411411
index);
412412
return;
413413
}
414414

415-
upb_Array_Resize(intern->array, size - 1, Arena_Get(&intern->arena));
415+
upb_Array_Delete(intern->array, index, 1);
416416
}
417417

418418
/**

php/src/Google/Protobuf/Internal/RepeatedField.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,13 +219,13 @@ public function offsetSet($offset, $value)
219219
public function offsetUnset($offset)
220220
{
221221
$count = count($this->container);
222-
if (!is_numeric($offset) || $count === 0 || $offset !== $count - 1) {
222+
if (!is_numeric($offset) || $count === 0 || $offset < 0 || $offset >= $count) {
223223
trigger_error(
224224
"Cannot remove element at the given index",
225225
E_USER_ERROR);
226226
return;
227227
}
228-
array_pop($this->container);
228+
array_splice($this->container, $offset, 1);
229229
}
230230

231231
/**

php/tests/ArrayTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -655,4 +655,31 @@ public function testClone()
655655
$arr2 = clone $arr;
656656
$this->assertSame($arr[0], $arr2[0]);
657657
}
658+
659+
#########################################################
660+
# Test offsetUnset
661+
#########################################################
662+
663+
public function testOffsetUnset()
664+
{
665+
$arr = new RepeatedField(GPBType::INT32);
666+
$arr[] = 0;
667+
$arr[] = 1;
668+
$arr[] = 2;
669+
670+
$this->assertSame(3, count($arr));
671+
$this->assertSame(0, $arr[0]);
672+
$this->assertSame(1, $arr[1]);
673+
$this->assertSame(2, $arr[2]);
674+
675+
$arr->offsetUnset(1);
676+
$this->assertSame(0, $arr[0]);
677+
$this->assertSame(2, $arr[1]);
678+
679+
$arr->offsetUnset(0);
680+
$this->assertSame(2, $arr[0]);
681+
682+
$arr->offsetUnset(0);
683+
$this->assertCount(0, $arr);
684+
}
658685
}

0 commit comments

Comments
 (0)