Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110134
MIME-Version: 1.0
References: <CAF+90c8BeG9GwQOVypcNe3p5Yp9LefsWHfH5Wrjq1o6HZvrZVQ@mail.gmail.com>
In-Reply-To: <CAF+90c8BeG9GwQOVypcNe3p5Yp9LefsWHfH5Wrjq1o6HZvrZVQ@mail.gmail.com>
Date: Tue, 12 May 2020 10:26:24 +0200
Message-ID: <CAF+90c_mfx3fbwWGusKH+LdXcMV7g-EiX5H15-AAWmhPSxnLAA@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000003042b05a56f3960"
Subject: Re: Making all Traversables an Iterator or IteratorAggregate
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000003042b05a56f3960
Content-Type: text/plain; charset="UTF-8"

On Wed, Mar 11, 2020 at 10:50 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> Hi internals,
>
> Userland classes that implement Traversable must do so either through
> Iterator or IteratorAggregate. The same requirement does not exist for
> internal classes: They can implement the internal get_iterator mechanism,
> without exposing either the Iterator or IteratorAggregate APIs. This makes
> them usable in get_iterator(), but incompatible with any Iterator based
> APIs.
>

This should have said: "This makes them usable in foreach(), but
incompatible with any Iterator based APIs."

A lot of internal classes do this, because exposing the userland APIs is
> simply a lot of work. I would like to add a general mechanism to make this
> simpler: https://github.com/php/php-src/pull/5216 adds a generic
> "InternalIterator" class, that essentially converts the internal
> get_iterator interface into a proper Iterator. Internal classes then only
> need to a) implement the IteratorAggregate interface and b) add a
> getIterator() method with an implementation looking like this:
>
> // WeakMap::getIterator(): Iterator
> ZEND_METHOD(WeakMap, getIterator)
> {
>     if (zend_parse_parameters_none() == FAILURE) {
>         return;
>     }
>     zend_create_internal_iterator_zval(return_value, ZEND_THIS);
> }
>
> This allows internal classes to trivially implement IteratorAggregate, and
> as such allows us to enforce that all Traversables implement Iterator or
> IteratorAggregate.
>

Does anyone have thoughts on this change? Mostly this is a feature for
extensions, but also user-visible in that a bunch of classes will switch
from being Traversable to being IteratorAggregate.

We may also want to convert some existing Iterators to IteratorAggregates.
For example SplFixedArray currently implements Iterator, which means that
it's not possible to have nested loops over SplFixedArray. We could now
easily fix this by switching it to use IteratorAggregate, which will allow
multiple parallel iterators to work on the same array. Of course, there is
BC break potential in such a change.

There's some bikeshed potential here regarding the class name. I picked
"InternalIterator" as an iterator for internal classes, but "internal
iteration" is also a technical term (the opposite of "external iteration"),
so maybe that name isn't ideal.

Regards,
Nikita

--00000000000003042b05a56f3960--