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. Regards, Nikita
