Skip to content

Commit a510c7d

Browse files
alexdowadalexdowad
committed
SplFixedArray is Aggregate, not Iterable
One strange feature of SplFixedArray was that it could not be used in nested foreach loops. If one did so, the inner loop would overwrite the iteration state of the outer loop. To illustrate: $spl = SplFixedArray::fromArray([0, 1]); foreach ($spl as $a) { foreach ($spl as $b) { echo "$a $b"; } } Would only print two lines: 0 0 0 1 Use the new InternalIterator feature which was introduced in ff19ec2 to convert SplFixedArray to an Aggregate rather than Iterable. As a bonus, we get to trim down some ugly code! Yay!
1 parent 8fef83d commit a510c7d

9 files changed

+154
-364
lines changed

‎ext/spl/spl_fixedarray.c

Lines changed: 39 additions & 172 deletions
Original file line numberDiff line numberDiff line change
@@ -46,29 +46,22 @@ typedef struct _spl_fixedarray { /* {{{ */
4646
/* }}} */
4747

4848
typedef struct _spl_fixedarray_object { /* {{{ */
49-
spl_fixedarray array;
50-
zend_function *fptr_offset_get;
51-
zend_function *fptr_offset_set;
52-
zend_function *fptr_offset_has;
53-
zend_function *fptr_offset_del;
54-
zend_function *fptr_count;
55-
int current;
56-
int flags;
57-
zend_object std;
49+
spl_fixedarray array;
50+
zend_function *fptr_offset_get;
51+
zend_function *fptr_offset_set;
52+
zend_function *fptr_offset_has;
53+
zend_function *fptr_offset_del;
54+
zend_function *fptr_count;
55+
zend_object std;
5856
} spl_fixedarray_object;
5957
/* }}} */
6058

6159
typedef struct _spl_fixedarray_it { /* {{{ */
62-
zend_user_iterator intern;
60+
zend_object_iterator intern;
61+
int current;
6362
} spl_fixedarray_it;
6463
/* }}} */
6564

66-
#define SPL_FIXEDARRAY_OVERLOADED_REWIND 0x0001
67-
#define SPL_FIXEDARRAY_OVERLOADED_VALID 0x0002
68-
#define SPL_FIXEDARRAY_OVERLOADED_KEY 0x0004
69-
#define SPL_FIXEDARRAY_OVERLOADED_CURRENT 0x0008
70-
#define SPL_FIXEDARRAY_OVERLOADED_NEXT 0x0010
71-
7265
static inline spl_fixedarray_object *spl_fixed_array_from_obj(zend_object *obj) /* {{{ */ {
7366
return (spl_fixedarray_object*)((char*)(obj) - XtOffsetOf(spl_fixedarray_object, std));
7467
}
@@ -201,23 +194,17 @@ static void spl_fixedarray_object_free_storage(zend_object *object) /* {{{ */
201194
}
202195
/* }}} */
203196

204-
zend_object_iterator *spl_fixedarray_get_iterator(zend_class_entry *ce, zval *object, int by_ref);
205-
206197
static zend_object *spl_fixedarray_object_new_ex(zend_class_entry *class_type, zend_object *orig, int clone_orig) /* {{{ */
207198
{
208199
spl_fixedarray_object *intern;
209200
zend_class_entry *parent = class_type;
210201
int inherited = 0;
211-
zend_class_iterator_funcs *funcs_ptr;
212202

213203
intern = zend_object_alloc(sizeof(spl_fixedarray_object), parent);
214204

215205
zend_object_std_init(&intern->std, class_type);
216206
object_properties_init(&intern->std, class_type);
217207

218-
intern->current = 0;
219-
intern->flags = 0;
220-
221208
if (orig && clone_orig) {
222209
spl_fixedarray_object *other = spl_fixed_array_from_obj(orig);
223210
spl_fixedarray_init(&intern->array, other->array.size);
@@ -236,31 +223,7 @@ static zend_object *spl_fixedarray_object_new_ex(zend_class_entry *class_type, z
236223

237224
ZEND_ASSERT(parent);
238225

239-
funcs_ptr = class_type->iterator_funcs_ptr;
240-
if (!funcs_ptr->zf_current) {
241-
funcs_ptr->zf_rewind = zend_hash_str_find_ptr(&class_type->function_table, "rewind", sizeof("rewind") - 1);
242-
funcs_ptr->zf_valid = zend_hash_str_find_ptr(&class_type->function_table, "valid", sizeof("valid") - 1);
243-
funcs_ptr->zf_key = zend_hash_str_find_ptr(&class_type->function_table, "key", sizeof("key") - 1);
244-
funcs_ptr->zf_current = zend_hash_str_find_ptr(&class_type->function_table, "current", sizeof("current") - 1);
245-
funcs_ptr->zf_next = zend_hash_str_find_ptr(&class_type->function_table, "next", sizeof("next") - 1);
246-
}
247226
if (inherited) {
248-
if (funcs_ptr->zf_rewind->common.scope != parent) {
249-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_REWIND;
250-
}
251-
if (funcs_ptr->zf_valid->common.scope != parent) {
252-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_VALID;
253-
}
254-
if (funcs_ptr->zf_key->common.scope != parent) {
255-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_KEY;
256-
}
257-
if (funcs_ptr->zf_current->common.scope != parent) {
258-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_CURRENT;
259-
}
260-
if (funcs_ptr->zf_next->common.scope != parent) {
261-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_NEXT;
262-
}
263-
264227
intern->fptr_offset_get = zend_hash_str_find_ptr(&class_type->function_table, "offsetget", sizeof("offsetget") - 1);
265228
if (intern->fptr_offset_get->common.scope == parent) {
266229
intern->fptr_offset_get = NULL;
@@ -780,36 +743,35 @@ PHP_METHOD(SplFixedArray, offsetUnset)
780743

781744
} /* }}} */
782745

783-
static void spl_fixedarray_it_dtor(zend_object_iterator *iter) /* {{{ */
746+
/* {{{ Create a new iterator from a SplFixedArray instance. */
747+
PHP_METHOD(SplFixedArray, getIterator)
784748
{
785-
spl_fixedarray_it *iterator = (spl_fixedarray_it *)iter;
749+
if (zend_parse_parameters_none() == FAILURE) {
750+
return;
751+
}
786752

787-
zend_user_it_invalidate_current(iter);
788-
zval_ptr_dtor(&iterator->intern.it.data);
753+
zend_create_internal_iterator_zval(return_value, ZEND_THIS);
789754
}
790755
/* }}} */
791756

792-
static void spl_fixedarray_it_rewind(zend_object_iterator *iter) /* {{{ */
757+
static void spl_fixedarray_it_dtor(zend_object_iterator *iter) /* {{{ */
793758
{
794-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
759+
zval_ptr_dtor(&iter->data);
760+
}
761+
/* }}} */
795762

796-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_REWIND) {
797-
zend_user_it_rewind(iter);
798-
} else {
799-
object->current = 0;
800-
}
763+
static void spl_fixedarray_it_rewind(zend_object_iterator *iter) /* {{{ */
764+
{
765+
((spl_fixedarray_it*)iter)->current = 0;
801766
}
802767
/* }}} */
803768

804769
static int spl_fixedarray_it_valid(zend_object_iterator *iter) /* {{{ */
805770
{
806-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
771+
spl_fixedarray_it *iterator = (spl_fixedarray_it*)iter;
772+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
807773

808-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_VALID) {
809-
return zend_user_it_valid(iter);
810-
}
811-
812-
if (object->current >= 0 && object->current < object->array.size) {
774+
if (iterator->current >= 0 && iterator->current < object->array.size) {
813775
return SUCCESS;
814776
}
815777

@@ -819,122 +781,29 @@ static int spl_fixedarray_it_valid(zend_object_iterator *iter) /* {{{ */
819781

820782
static zval *spl_fixedarray_it_get_current_data(zend_object_iterator *iter) /* {{{ */
821783
{
822-
zval zindex;
823-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
824-
825-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_CURRENT) {
826-
return zend_user_it_get_current_data(iter);
827-
} else {
828-
zval *data;
784+
zval zindex, *data;
785+
spl_fixedarray_it *iterator = (spl_fixedarray_it*)iter;
786+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
829787

830-
ZVAL_LONG(&zindex, object->current);
788+
ZVAL_LONG(&zindex, iterator->current);
789+
data = spl_fixedarray_object_read_dimension_helper(object, &zindex);
831790

832-
data = spl_fixedarray_object_read_dimension_helper(object, &zindex);
833-
834-
if (data == NULL) {
835-
data = &EG(uninitialized_zval);
836-
}
837-
return data;
791+
if (data == NULL) {
792+
data = &EG(uninitialized_zval);
838793
}
794+
return data;
839795
}
840796
/* }}} */
841797

842798
static void spl_fixedarray_it_get_current_key(zend_object_iterator *iter, zval *key) /* {{{ */
843799
{
844-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
845-
846-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_KEY) {
847-
zend_user_it_get_current_key(iter, key);
848-
} else {
849-
ZVAL_LONG(key, object->current);
850-
}
800+
ZVAL_LONG(key, ((spl_fixedarray_it*)iter)->current);
851801
}
852802
/* }}} */
853803

854804
static void spl_fixedarray_it_move_forward(zend_object_iterator *iter) /* {{{ */
855805
{
856-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
857-
858-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_NEXT) {
859-
zend_user_it_move_forward(iter);
860-
} else {
861-
zend_user_it_invalidate_current(iter);
862-
object->current++;
863-
}
864-
}
865-
/* }}} */
866-
867-
/* {{{ Return current array key */
868-
PHP_METHOD(SplFixedArray, key)
869-
{
870-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
871-
872-
if (zend_parse_parameters_none() == FAILURE) {
873-
RETURN_THROWS();
874-
}
875-
876-
RETURN_LONG(intern->current);
877-
}
878-
/* }}} */
879-
880-
/* {{{ Move to next entry */
881-
PHP_METHOD(SplFixedArray, next)
882-
{
883-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
884-
885-
if (zend_parse_parameters_none() == FAILURE) {
886-
RETURN_THROWS();
887-
}
888-
889-
intern->current++;
890-
}
891-
/* }}} */
892-
893-
/* {{{ Check whether the datastructure contains more entries */
894-
PHP_METHOD(SplFixedArray, valid)
895-
{
896-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
897-
898-
if (zend_parse_parameters_none() == FAILURE) {
899-
RETURN_THROWS();
900-
}
901-
902-
RETURN_BOOL(intern->current >= 0 && intern->current < intern->array.size);
903-
}
904-
/* }}} */
905-
906-
/* {{{ Rewind the datastructure back to the start */
907-
PHP_METHOD(SplFixedArray, rewind)
908-
{
909-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
910-
911-
if (zend_parse_parameters_none() == FAILURE) {
912-
RETURN_THROWS();
913-
}
914-
915-
intern->current = 0;
916-
}
917-
/* }}} */
918-
919-
/* {{{ Return current datastructure entry */
920-
PHP_METHOD(SplFixedArray, current)
921-
{
922-
zval zindex, *value;
923-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
924-
925-
if (zend_parse_parameters_none() == FAILURE) {
926-
RETURN_THROWS();
927-
}
928-
929-
ZVAL_LONG(&zindex, intern->current);
930-
931-
value = spl_fixedarray_object_read_dimension_helper(intern, &zindex);
932-
933-
if (value) {
934-
ZVAL_COPY_DEREF(return_value, value);
935-
} else {
936-
RETURN_NULL();
937-
}
806+
((spl_fixedarray_it*)iter)->current++;
938807
}
939808
/* }}} */
940809

@@ -963,12 +832,10 @@ zend_object_iterator *spl_fixedarray_get_iterator(zend_class_entry *ce, zval *ob
963832

964833
zend_iterator_init((zend_object_iterator*)iterator);
965834

966-
ZVAL_OBJ_COPY(&iterator->intern.it.data, Z_OBJ_P(object));
967-
iterator->intern.it.funcs = &spl_fixedarray_it_funcs;
968-
iterator->intern.ce = ce;
969-
ZVAL_UNDEF(&iterator->intern.value);
835+
ZVAL_OBJ_COPY(&iterator->intern.data, Z_OBJ_P(object));
836+
iterator->intern.funcs = &spl_fixedarray_it_funcs;
970837

971-
return &iterator->intern.it;
838+
return &iterator->intern;
972839
}
973840
/* }}} */
974841

@@ -990,7 +857,7 @@ PHP_MINIT_FUNCTION(spl_fixedarray)
990857
spl_handler_SplFixedArray.dtor_obj = zend_objects_destroy_object;
991858
spl_handler_SplFixedArray.free_obj = spl_fixedarray_object_free_storage;
992859

993-
REGISTER_SPL_IMPLEMENTS(SplFixedArray, Iterator);
860+
REGISTER_SPL_IMPLEMENTS(SplFixedArray, Aggregate);
994861
REGISTER_SPL_IMPLEMENTS(SplFixedArray, ArrayAccess);
995862
REGISTER_SPL_IMPLEMENTS(SplFixedArray, Countable);
996863

‎ext/spl/spl_fixedarray.stub.php

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
/** @generate-function-entries */
44

5-
class SplFixedArray implements Iterator, ArrayAccess, Countable
5+
class SplFixedArray implements IteratorAggregate, ArrayAccess, Countable
66
{
77
public function __construct(int $size = 0) {}
88

@@ -48,18 +48,5 @@ public function offsetSet($index, mixed $value) {}
4848
*/
4949
public function offsetUnset($index) {}
5050

51-
/** @return void */
52-
public function rewind() {}
53-
54-
/** @return mixed */
55-
public function current() {}
56-
57-
/** @return int */
58-
public function key() {}
59-
60-
/** @return void */
61-
public function next() {}
62-
63-
/** @return bool */
64-
public function valid() {}
51+
public function getIterator(): Iterator {}
6552
}

‎ext/spl/spl_fixedarray_arginfo.h

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 4b6c37c54416ee46f610baba2a8b2be45d1db96f */
2+
* Stub hash: 2075619a330ed636ce7f7c85aeb208a8f9f55ca7 */
33

44
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SplFixedArray___construct, 0, 0, 0)
55
ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, size, IS_LONG, 0, "0")
@@ -36,15 +36,8 @@ ZEND_END_ARG_INFO()
3636

3737
#define arginfo_class_SplFixedArray_offsetUnset arginfo_class_SplFixedArray_offsetExists
3838

39-
#define arginfo_class_SplFixedArray_rewind arginfo_class_SplFixedArray___wakeup
40-
41-
#define arginfo_class_SplFixedArray_current arginfo_class_SplFixedArray___wakeup
42-
43-
#define arginfo_class_SplFixedArray_key arginfo_class_SplFixedArray___wakeup
44-
45-
#define arginfo_class_SplFixedArray_next arginfo_class_SplFixedArray___wakeup
46-
47-
#define arginfo_class_SplFixedArray_valid arginfo_class_SplFixedArray___wakeup
39+
ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_class_SplFixedArray_getIterator, 0, 0, Iterator, 0)
40+
ZEND_END_ARG_INFO()
4841

4942

5043
ZEND_METHOD(SplFixedArray, __construct);
@@ -58,11 +51,7 @@ ZEND_METHOD(SplFixedArray, offsetExists);
5851
ZEND_METHOD(SplFixedArray, offsetGet);
5952
ZEND_METHOD(SplFixedArray, offsetSet);
6053
ZEND_METHOD(SplFixedArray, offsetUnset);
61-
ZEND_METHOD(SplFixedArray, rewind);
62-
ZEND_METHOD(SplFixedArray, current);
63-
ZEND_METHOD(SplFixedArray, key);
64-
ZEND_METHOD(SplFixedArray, next);
65-
ZEND_METHOD(SplFixedArray, valid);
54+
ZEND_METHOD(SplFixedArray, getIterator);
6655

6756

6857
static const zend_function_entry class_SplFixedArray_methods[] = {
@@ -77,10 +66,6 @@ static const zend_function_entry class_SplFixedArray_methods[] = {
7766
ZEND_ME(SplFixedArray, offsetGet, arginfo_class_SplFixedArray_offsetGet, ZEND_ACC_PUBLIC)
7867
ZEND_ME(SplFixedArray, offsetSet, arginfo_class_SplFixedArray_offsetSet, ZEND_ACC_PUBLIC)
7968
ZEND_ME(SplFixedArray, offsetUnset, arginfo_class_SplFixedArray_offsetUnset, ZEND_ACC_PUBLIC)
80-
ZEND_ME(SplFixedArray, rewind, arginfo_class_SplFixedArray_rewind, ZEND_ACC_PUBLIC)
81-
ZEND_ME(SplFixedArray, current, arginfo_class_SplFixedArray_current, ZEND_ACC_PUBLIC)
82-
ZEND_ME(SplFixedArray, key, arginfo_class_SplFixedArray_key, ZEND_ACC_PUBLIC)
83-
ZEND_ME(SplFixedArray, next, arginfo_class_SplFixedArray_next, ZEND_ACC_PUBLIC)
84-
ZEND_ME(SplFixedArray, valid, arginfo_class_SplFixedArray_valid, ZEND_ACC_PUBLIC)
69+
ZEND_ME(SplFixedArray, getIterator, arginfo_class_SplFixedArray_getIterator, ZEND_ACC_PUBLIC)
8570
ZEND_FE_END
8671
};

0 commit comments

Comments
 (0)