Skip to content

Commit 03245b3

Browse files
committed
SplFixedArray can be used in nested 'foreach' loops
Because SplFixedArray is an Iterator, and stores its own iteration state, it cannot be used in nested 'foreach' loops. If you try, the inner loop will overwrite the iteration position of the outer loop, so that the outer loop 'thinks' it is finished after the inner loop finishes. To illustrate: $spl = SplFixedArray::fromArray([0, 1]); foreach ($spl as $a) { foreach ($spl as $b) { echo "$a $b"; } } Only prints two lines: 0 0 0 1 Interestingly, though SplFixedArray has methods like ::current(), ::next(), etc., these are not used by the 'foreach' loop. Rather, an instance of '_spl_fixedarray_it' is created internally and used to track the iteration state. Therefore, if the current iteration position is stored in '_spl_fixedarray_it', the nested 'foreach' loops will not interfere with each other in this way. However, the problem is that ::current(), ::next(), etc. are still part of the SplFixedArray interface and users may expect them to 'work' in a 'foreach' loop. For ::next() and ::rewind(), there is nothing we can do, since the SplFixedArray instance does not keep any reference to the _spl_fixedarray_it structures being used by the 'foreach' loops. However, we can still keep ::valid(), ::current(), and ::key() working by keeping a current iteration position in each SplFixedArray instance and updating it at the same time as we update the _spl_fixedarray_it. Although, they still break in cases like this: foreach ($spl as $a) { foreach ($spl as $b) { // do something } $spl->key(); // will NOT return iteration position of outer loop } Overall, this fix stinks and it may be better to bite the bullet and convert SplFixedArray to IteratorAggregate.
1 parent 5951ff7 commit 03245b3

File tree

2 files changed

+43
-11
lines changed

2 files changed

+43
-11
lines changed

ext/spl/spl_fixedarray.c

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ typedef struct _spl_fixedarray_object { /* {{{ */
5252
zend_function *fptr_offset_has;
5353
zend_function *fptr_offset_del;
5454
zend_function *fptr_count;
55-
int current;
55+
int current; /* used by SplFixedArray::current(), ::next(), etc. */
5656
int flags;
5757
zend_class_entry *ce_get_iterator;
5858
zend_object std;
@@ -61,6 +61,9 @@ typedef struct _spl_fixedarray_object { /* {{{ */
6161

6262
typedef struct _spl_fixedarray_it { /* {{{ */
6363
zend_user_iterator intern;
64+
/* _spl_fixedarray_it.current is used by iterator created via spl_fixedarray_get_iterator()
65+
* This makes nested 'foreach' loops work correctly */
66+
int current;
6467
} spl_fixedarray_it;
6568
/* }}} */
6669

@@ -806,25 +809,29 @@ static void spl_fixedarray_it_dtor(zend_object_iterator *iter) /* {{{ */
806809

807810
static void spl_fixedarray_it_rewind(zend_object_iterator *iter) /* {{{ */
808811
{
809-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
812+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
813+
spl_fixedarray_it *iterator = (spl_fixedarray_it *)iter;
810814

811815
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_REWIND) {
812816
zend_user_it_rewind(iter);
813817
} else {
814-
object->current = 0;
818+
/* Update object->current so that if $array->key() is called in a 'foreach'
819+
* block, it will return the expected value */
820+
object->current = iterator->current = 0;
815821
}
816822
}
817823
/* }}} */
818824

819825
static int spl_fixedarray_it_valid(zend_object_iterator *iter) /* {{{ */
820826
{
821-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
827+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
828+
spl_fixedarray_it *iterator = (spl_fixedarray_it *)iter;
822829

823830
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_VALID) {
824831
return zend_user_it_valid(iter);
825832
}
826833

827-
if (object->current >= 0 && object->current < object->array.size) {
834+
if (iterator->current >= 0 && iterator->current < object->array.size) {
828835
return SUCCESS;
829836
}
830837

@@ -835,14 +842,15 @@ static int spl_fixedarray_it_valid(zend_object_iterator *iter) /* {{{ */
835842
static zval *spl_fixedarray_it_get_current_data(zend_object_iterator *iter) /* {{{ */
836843
{
837844
zval zindex;
838-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
845+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
846+
spl_fixedarray_it *iterator = (spl_fixedarray_it *)iter;
839847

840848
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_CURRENT) {
841849
return zend_user_it_get_current_data(iter);
842850
} else {
843851
zval *data;
844852

845-
ZVAL_LONG(&zindex, object->current);
853+
ZVAL_LONG(&zindex, iterator->current);
846854

847855
data = spl_fixedarray_object_read_dimension_helper(object, &zindex);
848856

@@ -856,25 +864,29 @@ static zval *spl_fixedarray_it_get_current_data(zend_object_iterator *iter) /* {
856864

857865
static void spl_fixedarray_it_get_current_key(zend_object_iterator *iter, zval *key) /* {{{ */
858866
{
859-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
867+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
868+
spl_fixedarray_it *iterator = (spl_fixedarray_it *)iter;
860869

861870
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_KEY) {
862871
zend_user_it_get_current_key(iter, key);
863872
} else {
864-
ZVAL_LONG(key, object->current);
873+
ZVAL_LONG(key, iterator->current);
865874
}
866875
}
867876
/* }}} */
868877

869878
static void spl_fixedarray_it_move_forward(zend_object_iterator *iter) /* {{{ */
870879
{
871-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
880+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
881+
spl_fixedarray_it *iterator = (spl_fixedarray_it *)iter;
872882

873883
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_NEXT) {
874884
zend_user_it_move_forward(iter);
875885
} else {
876886
zend_user_it_invalidate_current(iter);
877-
object->current++;
887+
/* Update object->current so that if $array->key() is called in a 'foreach'
888+
* block, it will return the expected value */
889+
object->current = ++iterator->current;
878890
}
879891
}
880892
/* }}} */
@@ -986,6 +998,7 @@ zend_object_iterator *spl_fixedarray_get_iterator(zend_class_entry *ce, zval *ob
986998
ZVAL_OBJ(&iterator->intern.it.data, Z_OBJ_P(object));
987999
iterator->intern.it.funcs = &spl_fixedarray_it_funcs;
9881000
iterator->intern.ce = ce;
1001+
iterator->current = 0;
9891002
ZVAL_UNDEF(&iterator->intern.value);
9901003

9911004
return &iterator->intern.it;
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
Nested iteration of SplFixedArray using foreach loops
3+
--FILE--
4+
<?php
5+
6+
$array = SplFixedArray::fromArray([0, 1]);
7+
8+
foreach ($array as $value1) {
9+
foreach ($array as $value2) {
10+
echo "$value1 $value2\n";
11+
}
12+
}
13+
14+
?>
15+
--EXPECT--
16+
0 0
17+
0 1
18+
1 0
19+
1 1

0 commit comments

Comments
 (0)