Skip to content

Commit f6dbadd

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 7b4179e commit f6dbadd

8 files changed

+111
-369
lines changed

‎ext/spl/spl_fixedarray.c

Lines changed: 40 additions & 177 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);
@@ -238,31 +225,7 @@ static zend_object *spl_fixedarray_object_new_ex(zend_class_entry *class_type, z
238225
php_error_docref(NULL, E_COMPILE_ERROR, "Internal compiler error, Class is not child of SplFixedArray");
239226
}
240227

241-
funcs_ptr = class_type->iterator_funcs_ptr;
242-
if (!funcs_ptr->zf_current) {
243-
funcs_ptr->zf_rewind = zend_hash_str_find_ptr(&class_type->function_table, "rewind", sizeof("rewind") - 1);
244-
funcs_ptr->zf_valid = zend_hash_str_find_ptr(&class_type->function_table, "valid", sizeof("valid") - 1);
245-
funcs_ptr->zf_key = zend_hash_str_find_ptr(&class_type->function_table, "key", sizeof("key") - 1);
246-
funcs_ptr->zf_current = zend_hash_str_find_ptr(&class_type->function_table, "current", sizeof("current") - 1);
247-
funcs_ptr->zf_next = zend_hash_str_find_ptr(&class_type->function_table, "next", sizeof("next") - 1);
248-
}
249228
if (inherited) {
250-
if (funcs_ptr->zf_rewind->common.scope != parent) {
251-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_REWIND;
252-
}
253-
if (funcs_ptr->zf_valid->common.scope != parent) {
254-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_VALID;
255-
}
256-
if (funcs_ptr->zf_key->common.scope != parent) {
257-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_KEY;
258-
}
259-
if (funcs_ptr->zf_current->common.scope != parent) {
260-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_CURRENT;
261-
}
262-
if (funcs_ptr->zf_next->common.scope != parent) {
263-
intern->flags |= SPL_FIXEDARRAY_OVERLOADED_NEXT;
264-
}
265-
266229
intern->fptr_offset_get = zend_hash_str_find_ptr(&class_type->function_table, "offsetget", sizeof("offsetget") - 1);
267230
if (intern->fptr_offset_get->common.scope == parent) {
268231
intern->fptr_offset_get = NULL;
@@ -793,36 +756,36 @@ PHP_METHOD(SplFixedArray, offsetUnset)
793756

794757
} /* }}} */
795758

796-
static void spl_fixedarray_it_dtor(zend_object_iterator *iter) /* {{{ */
759+
/* {{{ proto Iterator SplFixedArray::getIterator()
760+
Create a new iterator from a SplFixedArray instance. */
761+
PHP_METHOD(SplFixedArray, getIterator)
797762
{
798-
spl_fixedarray_it *iterator = (spl_fixedarray_it *)iter;
763+
if (zend_parse_parameters_none() == FAILURE) {
764+
return;
765+
}
799766

800-
zend_user_it_invalidate_current(iter);
801-
zval_ptr_dtor(&iterator->intern.it.data);
767+
zend_create_internal_iterator_zval(return_value, ZEND_THIS);
802768
}
803769
/* }}} */
804770

805-
static void spl_fixedarray_it_rewind(zend_object_iterator *iter) /* {{{ */
771+
static void spl_fixedarray_it_dtor(zend_object_iterator *iter) /* {{{ */
806772
{
807-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
773+
zval_ptr_dtor(&iter->data);
774+
}
775+
/* }}} */
808776

809-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_REWIND) {
810-
zend_user_it_rewind(iter);
811-
} else {
812-
object->current = 0;
813-
}
777+
static void spl_fixedarray_it_rewind(zend_object_iterator *iter) /* {{{ */
778+
{
779+
((spl_fixedarray_it*)iter)->current = 0;
814780
}
815781
/* }}} */
816782

817783
static int spl_fixedarray_it_valid(zend_object_iterator *iter) /* {{{ */
818784
{
819-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
785+
spl_fixedarray_it *iterator = (spl_fixedarray_it*)iter;
786+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
820787

821-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_VALID) {
822-
return zend_user_it_valid(iter);
823-
}
824-
825-
if (object->current >= 0 && object->current < object->array.size) {
788+
if (iterator->current >= 0 && iterator->current < object->array.size) {
826789
return SUCCESS;
827790
}
828791

@@ -832,127 +795,29 @@ static int spl_fixedarray_it_valid(zend_object_iterator *iter) /* {{{ */
832795

833796
static zval *spl_fixedarray_it_get_current_data(zend_object_iterator *iter) /* {{{ */
834797
{
835-
zval zindex;
836-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
837-
838-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_CURRENT) {
839-
return zend_user_it_get_current_data(iter);
840-
} else {
841-
zval *data;
798+
zval zindex, *data;
799+
spl_fixedarray_it *iterator = (spl_fixedarray_it*)iter;
800+
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
842801

843-
ZVAL_LONG(&zindex, object->current);
802+
ZVAL_LONG(&zindex, iterator->current);
803+
data = spl_fixedarray_object_read_dimension_helper(object, &zindex);
844804

845-
data = spl_fixedarray_object_read_dimension_helper(object, &zindex);
846-
847-
if (data == NULL) {
848-
data = &EG(uninitialized_zval);
849-
}
850-
return data;
805+
if (data == NULL) {
806+
data = &EG(uninitialized_zval);
851807
}
808+
return data;
852809
}
853810
/* }}} */
854811

855812
static void spl_fixedarray_it_get_current_key(zend_object_iterator *iter, zval *key) /* {{{ */
856813
{
857-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
858-
859-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_KEY) {
860-
zend_user_it_get_current_key(iter, key);
861-
} else {
862-
ZVAL_LONG(key, object->current);
863-
}
814+
ZVAL_LONG(key, ((spl_fixedarray_it*)iter)->current);
864815
}
865816
/* }}} */
866817

867818
static void spl_fixedarray_it_move_forward(zend_object_iterator *iter) /* {{{ */
868819
{
869-
spl_fixedarray_object *object = Z_SPLFIXEDARRAY_P(&iter->data);
870-
871-
if (object->flags & SPL_FIXEDARRAY_OVERLOADED_NEXT) {
872-
zend_user_it_move_forward(iter);
873-
} else {
874-
zend_user_it_invalidate_current(iter);
875-
object->current++;
876-
}
877-
}
878-
/* }}} */
879-
880-
/* {{{ proto int SplFixedArray::key()
881-
Return current array key */
882-
PHP_METHOD(SplFixedArray, key)
883-
{
884-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
885-
886-
if (zend_parse_parameters_none() == FAILURE) {
887-
RETURN_THROWS();
888-
}
889-
890-
RETURN_LONG(intern->current);
891-
}
892-
/* }}} */
893-
894-
/* {{{ proto void SplFixedArray::next()
895-
Move to next entry */
896-
PHP_METHOD(SplFixedArray, next)
897-
{
898-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
899-
900-
if (zend_parse_parameters_none() == FAILURE) {
901-
RETURN_THROWS();
902-
}
903-
904-
intern->current++;
905-
}
906-
/* }}} */
907-
908-
/* {{{ proto bool SplFixedArray::valid()
909-
Check whether the datastructure contains more entries */
910-
PHP_METHOD(SplFixedArray, valid)
911-
{
912-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
913-
914-
if (zend_parse_parameters_none() == FAILURE) {
915-
RETURN_THROWS();
916-
}
917-
918-
RETURN_BOOL(intern->current >= 0 && intern->current < intern->array.size);
919-
}
920-
/* }}} */
921-
922-
/* {{{ proto void SplFixedArray::rewind()
923-
Rewind the datastructure back to the start */
924-
PHP_METHOD(SplFixedArray, rewind)
925-
{
926-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
927-
928-
if (zend_parse_parameters_none() == FAILURE) {
929-
RETURN_THROWS();
930-
}
931-
932-
intern->current = 0;
933-
}
934-
/* }}} */
935-
936-
/* {{{ proto mixed|NULL SplFixedArray::current()
937-
Return current datastructure entry */
938-
PHP_METHOD(SplFixedArray, current)
939-
{
940-
zval zindex, *value;
941-
spl_fixedarray_object *intern = Z_SPLFIXEDARRAY_P(ZEND_THIS);
942-
943-
if (zend_parse_parameters_none() == FAILURE) {
944-
RETURN_THROWS();
945-
}
946-
947-
ZVAL_LONG(&zindex, intern->current);
948-
949-
value = spl_fixedarray_object_read_dimension_helper(intern, &zindex);
950-
951-
if (value) {
952-
ZVAL_COPY_DEREF(return_value, value);
953-
} else {
954-
RETURN_NULL();
955-
}
820+
((spl_fixedarray_it*)iter)->current++;
956821
}
957822
/* }}} */
958823

@@ -980,12 +845,10 @@ zend_object_iterator *spl_fixedarray_get_iterator(zend_class_entry *ce, zval *ob
980845

981846
zend_iterator_init((zend_object_iterator*)iterator);
982847

983-
ZVAL_OBJ_COPY(&iterator->intern.it.data, Z_OBJ_P(object));
984-
iterator->intern.it.funcs = &spl_fixedarray_it_funcs;
985-
iterator->intern.ce = ce;
986-
ZVAL_UNDEF(&iterator->intern.value);
848+
ZVAL_OBJ_COPY(&iterator->intern.data, Z_OBJ_P(object));
849+
iterator->intern.funcs = &spl_fixedarray_it_funcs;
987850

988-
return &iterator->intern.it;
851+
return &iterator->intern;
989852
}
990853
/* }}} */
991854

@@ -1007,7 +870,7 @@ PHP_MINIT_FUNCTION(spl_fixedarray)
1007870
spl_handler_SplFixedArray.dtor_obj = zend_objects_destroy_object;
1008871
spl_handler_SplFixedArray.free_obj = spl_fixedarray_object_free_storage;
1009872

1010-
REGISTER_SPL_IMPLEMENTS(SplFixedArray, Iterator);
873+
REGISTER_SPL_IMPLEMENTS(SplFixedArray, Aggregate);
1011874
REGISTER_SPL_IMPLEMENTS(SplFixedArray, ArrayAccess);
1012875
REGISTER_SPL_IMPLEMENTS(SplFixedArray, Countable);
1013876

‎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 Aggregate, ArrayAccess, Countable
66
{
77
public function __construct(int $size = 0) {}
88

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

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

‎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: a14156d542422823fcee53eb8a151576d055f38f */
2+
* Stub hash: 3a866e7b05d16331596fcf25e146d7e037fa03e0 */
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)