Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110449
MIME-Version: 1.0
References: <CAF+90c8BeG9GwQOVypcNe3p5Yp9LefsWHfH5Wrjq1o6HZvrZVQ@mail.gmail.com>
 <CAF+90c_mfx3fbwWGusKH+LdXcMV7g-EiX5H15-AAWmhPSxnLAA@mail.gmail.com>
In-Reply-To: <CAF+90c_mfx3fbwWGusKH+LdXcMV7g-EiX5H15-AAWmhPSxnLAA@mail.gmail.com>
Date: Tue, 9 Jun 2020 15:33:13 +0200
Message-ID: <CAF+90c__FQ3FQXTDTeJ8cV9Nhwni0XUGBf4WeaTRXkDzWasCRw@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000d8fd9105a7a6c578"
Subject: Re: Making all Traversables an Iterator or IteratorAggregate
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000d8fd9105a7a6c578
Content-Type: text/plain; charset="UTF-8"

On Tue, May 12, 2020 at 10:26 AM Nikita Popov <nikita.ppv@gmail.com> wrote:

> 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.
>

Unfortunately this bikeshed remains unpainted... The proposed names were:

 1. InternalIterator
 2. ZendIterator
 3. IteratorForExtensionClassImplementations
 4. EngineIterator

I'm somewhat partial to the third option, with a less verbose name:
IteratorForExtensions.

Regards,
Nikita

--000000000000d8fd9105a7a6c578--