On Tue, Jun 9, 2020 at 3:33 PM Nikita Popov <nikita....@gmail.com> wrote:
> On Tue, May 12, 2020 at 10:26 AM Nikita Popov <nikita....@gmail.com> > wrote: > >> On Wed, Mar 11, 2020 at 10:50 AM Nikita Popov <nikita....@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. > I went ahead and changed the implementation to use IteratorForExtensions. Is anyone overly unhappy with that one? @Michal: "ExtensionsIterator" to me sounds like an iterator that iterates over extensions. Regards, Nikita
