Skip to content

Commit 15bbf6f

Browse files
nikicnikic
committed
Automatically determine whether to reuse get_iterator()
Same as with the IteratorAggregate case, allow reusing get_iterator if none of the Iterator methods are overridden. Drop the REUSE_GET_ITERATOR flag that previously allowed ArrayIterator to opt-in to unconditional get_iterator reuse, and drop the override handling it did, in favor of the automated approach.
1 parent d0dbf72 commit 15bbf6f

6 files changed

+24
-145
lines changed

‎Zend/tests/foreach_004.phpt

Lines changed: 0 additions & 65 deletions
This file was deleted.

‎Zend/zend_compile.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ typedef struct _zend_oparray_context {
240240
/* or IS_CONSTANT_VISITED_MARK | | | */
241241
#define ZEND_CLASS_CONST_IS_CASE (1 << 6) /* | | | X */
242242
/* | | | */
243-
/* Class Flags (unused: 15,21,30,31) | | | */
243+
/* Class Flags (unused: 15,16,21,30,31) | | | */
244244
/* =========== | | | */
245245
/* | | | */
246246
/* Special class types | | | */
@@ -269,9 +269,6 @@ typedef struct _zend_oparray_context {
269269
/* User class has methods with static variables | | | */
270270
#define ZEND_HAS_STATIC_IN_METHODS (1 << 14) /* X | | | */
271271
/* | | | */
272-
/* Children must reuse parent get_iterator() | | | */
273-
#define ZEND_ACC_REUSE_GET_ITERATOR (1 << 16) /* X | | | */
274-
/* | | | */
275272
/* Parent class is resolved (CE). | | | */
276273
#define ZEND_ACC_RESOLVED_PARENT (1 << 17) /* X | | | */
277274
/* | | | */

‎Zend/zend_interfaces.c

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -332,20 +332,23 @@ static int zend_implement_iterator(zend_class_entry *interface, zend_class_entry
332332
ZEND_ASSERT(class_type->type == ZEND_INTERNAL_CLASS);
333333
return SUCCESS;
334334
}
335-
/* Otherwise get_iterator was inherited from the parent by default. */
336-
}
337335

338-
if (class_type->parent && (class_type->parent->ce_flags & ZEND_ACC_REUSE_GET_ITERATOR)) {
339-
/* Keep the inherited get_iterator handler. */
340-
class_type->ce_flags |= ZEND_ACC_REUSE_GET_ITERATOR;
341-
} else {
342-
class_type->get_iterator = zend_user_it_get_iterator;
336+
/* None of the Iterator methods have been overwritten, use inherited get_iterator(). */
337+
if (zf_rewind->common.scope != class_type && zf_valid->common.scope != class_type &&
338+
zf_key->common.scope != class_type && zf_current->common.scope != class_type &&
339+
zf_next->common.scope != class_type) {
340+
return SUCCESS;
341+
}
342+
343+
/* One of the Iterator methods has been overwritten,
344+
* switch to zend_user_it_get_new_iterator. */
343345
}
344346

345347
ZEND_ASSERT(!class_type->iterator_funcs_ptr && "Iterator funcs already set?");
346348
zend_class_iterator_funcs *funcs_ptr = class_type->type == ZEND_INTERNAL_CLASS
347349
? pemalloc(sizeof(zend_class_iterator_funcs), 1)
348350
: zend_arena_alloc(&CG(arena), sizeof(zend_class_iterator_funcs));
351+
class_type->get_iterator = zend_user_it_get_iterator;
349352
class_type->iterator_funcs_ptr = funcs_ptr;
350353

351354
memset(funcs_ptr, 0, sizeof(zend_class_iterator_funcs));

‎ext/spl/spl_array.c

Lines changed: 9 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,6 @@ PHPAPI zend_class_entry *spl_ce_RecursiveArrayIterator;
4444
#define SPL_ARRAY_STD_PROP_LIST 0x00000001
4545
#define SPL_ARRAY_ARRAY_AS_PROPS 0x00000002
4646
#define SPL_ARRAY_CHILD_ARRAYS_ONLY 0x00000004
47-
#define SPL_ARRAY_OVERLOADED_REWIND 0x00010000
48-
#define SPL_ARRAY_OVERLOADED_VALID 0x00020000
49-
#define SPL_ARRAY_OVERLOADED_KEY 0x00040000
50-
#define SPL_ARRAY_OVERLOADED_CURRENT 0x00080000
51-
#define SPL_ARRAY_OVERLOADED_NEXT 0x00100000
5247
#define SPL_ARRAY_IS_SELF 0x01000000
5348
#define SPL_ARRAY_USE_OTHER 0x02000000
5449
#define SPL_ARRAY_INT_MASK 0xFFFF0000
@@ -229,19 +224,6 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o
229224
intern->fptr_count = NULL;
230225
}
231226
}
232-
/* Cache iterator functions if ArrayIterator or derived. Check current's */
233-
/* cache since only current is always required */
234-
if (intern->std.handlers == &spl_handler_ArrayIterator) {
235-
zend_class_iterator_funcs *funcs_ptr = class_type->iterator_funcs_ptr;
236-
237-
if (inherited) {
238-
if (funcs_ptr->zf_rewind->common.scope != parent) intern->ar_flags |= SPL_ARRAY_OVERLOADED_REWIND;
239-
if (funcs_ptr->zf_valid->common.scope != parent) intern->ar_flags |= SPL_ARRAY_OVERLOADED_VALID;
240-
if (funcs_ptr->zf_key->common.scope != parent) intern->ar_flags |= SPL_ARRAY_OVERLOADED_KEY;
241-
if (funcs_ptr->zf_current->common.scope != parent) intern->ar_flags |= SPL_ARRAY_OVERLOADED_CURRENT;
242-
if (funcs_ptr->zf_next->common.scope != parent) intern->ar_flags |= SPL_ARRAY_OVERLOADED_NEXT;
243-
}
244-
}
245227

246228
intern->ht_iter = (uint32_t)-1;
247229
return &intern->std;
@@ -952,56 +934,36 @@ static int spl_array_it_valid(zend_object_iterator *iter) /* {{{ */
952934
{
953935
spl_array_object *object = Z_SPLARRAY_P(&iter->data);
954936
HashTable *aht = spl_array_get_hash_table(object);
955-
956-
if (object->ar_flags & SPL_ARRAY_OVERLOADED_VALID) {
957-
return zend_user_it_valid(iter);
958-
} else {
959-
return zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, object));
960-
}
937+
return zend_hash_has_more_elements_ex(aht, spl_array_get_pos_ptr(aht, object));
961938
}
962939
/* }}} */
963940

964941
static zval *spl_array_it_get_current_data(zend_object_iterator *iter) /* {{{ */
965942
{
966943
spl_array_object *object = Z_SPLARRAY_P(&iter->data);
967944
HashTable *aht = spl_array_get_hash_table(object);
968-
969-
if (object->ar_flags & SPL_ARRAY_OVERLOADED_CURRENT) {
970-
return zend_user_it_get_current_data(iter);
971-
} else {
972-
zval *data = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, object));
973-
if (data && Z_TYPE_P(data) == IS_INDIRECT) {
974-
data = Z_INDIRECT_P(data);
975-
}
976-
return data;
945+
zval *data = zend_hash_get_current_data_ex(aht, spl_array_get_pos_ptr(aht, object));
946+
if (data && Z_TYPE_P(data) == IS_INDIRECT) {
947+
data = Z_INDIRECT_P(data);
977948
}
949+
return data;
978950
}
979951
/* }}} */
980952

981953
static void spl_array_it_get_current_key(zend_object_iterator *iter, zval *key) /* {{{ */
982954
{
983955
spl_array_object *object = Z_SPLARRAY_P(&iter->data);
984956
HashTable *aht = spl_array_get_hash_table(object);
985-
986-
if (object->ar_flags & SPL_ARRAY_OVERLOADED_KEY) {
987-
zend_user_it_get_current_key(iter, key);
988-
} else {
989-
zend_hash_get_current_key_zval_ex(aht, key, spl_array_get_pos_ptr(aht, object));
990-
}
957+
zend_hash_get_current_key_zval_ex(aht, key, spl_array_get_pos_ptr(aht, object));
991958
}
992959
/* }}} */
993960

994961
static void spl_array_it_move_forward(zend_object_iterator *iter) /* {{{ */
995962
{
996963
spl_array_object *object = Z_SPLARRAY_P(&iter->data);
997964
HashTable *aht = spl_array_get_hash_table(object);
998-
999-
if (object->ar_flags & SPL_ARRAY_OVERLOADED_NEXT) {
1000-
zend_user_it_move_forward(iter);
1001-
} else {
1002-
zend_user_it_invalidate_current(iter);
1003-
spl_array_next_ex(object, aht);
1004-
}
965+
zend_user_it_invalidate_current(iter);
966+
spl_array_next_ex(object, aht);
1005967
}
1006968
/* }}} */
1007969

@@ -1021,13 +983,8 @@ static void spl_array_rewind(spl_array_object *intern) /* {{{ */
1021983
static void spl_array_it_rewind(zend_object_iterator *iter) /* {{{ */
1022984
{
1023985
spl_array_object *object = Z_SPLARRAY_P(&iter->data);
1024-
1025-
if (object->ar_flags & SPL_ARRAY_OVERLOADED_REWIND) {
1026-
zend_user_it_rewind(iter);
1027-
} else {
1028986
zend_user_it_invalidate_current(iter);
1029987
spl_array_rewind(object);
1030-
}
1031988
}
1032989
/* }}} */
1033990

@@ -1099,16 +1056,7 @@ static const zend_object_iterator_funcs spl_array_it_funcs = {
10991056

11001057
zend_object_iterator *spl_array_get_iterator(zend_class_entry *ce, zval *object, int by_ref) /* {{{ */
11011058
{
1102-
zend_user_iterator *iterator;
1103-
spl_array_object *array_object = Z_SPLARRAY_P(object);
1104-
1105-
if (by_ref && (array_object->ar_flags & SPL_ARRAY_OVERLOADED_CURRENT)) {
1106-
zend_throw_error(NULL, "An iterator cannot be used with foreach by reference");
1107-
return NULL;
1108-
}
1109-
1110-
iterator = emalloc(sizeof(zend_user_iterator));
1111-
1059+
zend_user_iterator *iterator = emalloc(sizeof(zend_user_iterator));
11121060
zend_iterator_init(&iterator->it);
11131061

11141062
ZVAL_OBJ_COPY(&iterator->it.data, Z_OBJ_P(object));
@@ -1864,7 +1812,6 @@ PHP_MINIT_FUNCTION(spl_array)
18641812
spl_ce_ArrayIterator = register_class_ArrayIterator(spl_ce_SeekableIterator, zend_ce_arrayaccess, zend_ce_serializable, zend_ce_countable);
18651813
spl_ce_ArrayIterator->create_object = spl_array_object_new;
18661814
spl_ce_ArrayIterator->get_iterator = spl_array_get_iterator;
1867-
spl_ce_ArrayIterator->ce_flags |= ZEND_ACC_REUSE_GET_ITERATOR;
18681815

18691816
memcpy(&spl_handler_ArrayIterator, &spl_handler_ArrayObject, sizeof(zend_object_handlers));
18701817

@@ -1877,7 +1824,6 @@ PHP_MINIT_FUNCTION(spl_array)
18771824
spl_ce_RecursiveArrayIterator = register_class_RecursiveArrayIterator(spl_ce_ArrayIterator, spl_ce_RecursiveIterator);
18781825
spl_ce_RecursiveArrayIterator->create_object = spl_array_object_new;
18791826
spl_ce_RecursiveArrayIterator->get_iterator = spl_array_get_iterator;
1880-
spl_ce_RecursiveArrayIterator->ce_flags |= ZEND_ACC_REUSE_GET_ITERATOR;
18811827

18821828
REGISTER_SPL_CLASS_CONST_LONG(RecursiveArrayIterator, "CHILD_ARRAYS_ONLY", SPL_ARRAY_CHILD_ARRAYS_ONLY);
18831829

‎ext/spl/tests/bug54281.phpt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ foreach($it as $k=>$v) { }
1212

1313
?>
1414
--EXPECTF--
15-
Fatal error: Uncaught Error: The object is in an invalid state as the parent constructor was not called in %s:%d
15+
Fatal error: Uncaught Error: Object is not initialized in %s:%d
1616
Stack trace:
17-
#0 %s%ebug54281.php(8): RecursiveIteratorIterator->rewind()
18-
#1 {main}
17+
#0 {main}
1918
thrown in %s%ebug54281.php on line 8

‎ext/spl/tests/recursiveIteratorIterator_endchildren_error.phpt

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ foreach ($recItIt2 as $val) echo "$val\n";
3636

3737
Fatal error: Uncaught Exception in %s
3838
Stack trace:
39-
#0 [internal function]: MyRecursiveIteratorIterator->endchildren()
40-
#1 %s: RecursiveIteratorIterator->next()
41-
#2 {main}
39+
#0 %s(%d): MyRecursiveIteratorIterator->endchildren()
40+
#1 {main}
4241
thrown in %s on line %d

0 commit comments

Comments
 (0)