Change "foreach" statement behavior

[dstogov]

Implementation of https://wiki.php.net/rfc/php7_foreach

• "foreach by value" don't relay on internal array/object pointer and doesn't perform array duplication. It just locks it incrementing reference counter. If the original array is modified by some code, the copy on write is performed and "foreach" still work with the old copy.
• it makes no difference if array given to "foreach by value" is reference itself
• "foreach by reference" still use internal array/object pointer and should work similar to PHP-5 except for few inconsistent cases.

[nikic]

Why do we set nInternalPointer here / why are hash table iterators initialized with nInternalPointer? Wouldn't it be better to make it completely independent?

[dstogov]

Actually, I got this idea a day before and already have implementation, but I'm not sure if it's a right decision. I generalized API, so to switch to this behavior we would need just remove nInternalPointer assignments in FE_RESET/FE_FETCH. I'll rise a question on @Internals.

[nikic]

It would be nice to port ArrayIterator/ArrayObject to use HashTableIterator as well - it currently uses checks similar to what was done in foreach.

[dstogov]

I don't like to do it as a part of this RFC. SPL may be changed later.

On Sat, Jan 31, 2015 at 1:49 AM, Nikita Popov notifications@github.com
wrote:

It would be nice to port ArrayIterator/ArrayObject to use
HashTableIterator as well - it currently uses checks similar to what was
done in foreach.

—
Reply to this email directly or view it on GitHub
#1034 (comment).

[nikic]

I'm mainly asking about SPL, because I suspect it may have an impact on the design of HashTableIterator. The problem is that foreach uses an unusual iteration pattern for arrays:

reset();
while (get_current_data(&data) == SUCCESS) {
    move_forward(); // <--
    code(data);
}

While foreach over iterators (and all other iteration code) will have the move_forward() after the code(). The current HashTableIterator implementation is bound to the pattern that foreach uses, e.g. because it advances to the next element on hash_del. If the move_forward happened after the code, we'd have to go to the previous element instead.

My solution to this was to change foreach for arrays to the usual pattern - however this required having an extra "iterator before first position" state for hash positions (like "iterator after last position" is currently INVALID_IDX).

[dstogov]

Merged into master