Re: [PHP-DEV] Re: Making all Traversables an Iterator or IteratorAggregate

This is only part of a thread. view whole thread
  110136
May 12, 2020 12:49 pollita@php.net (Sara Golemon)
On Tue, May 12, 2020 at 3:26 AM Nikita Popov ppv@gmail.com> wrote:
> // WeakMap::getIterator(): Iterator > ZEND_METHOD(WeakMap, getIterator) > { > if (zend_parse_parameters_none() == FAILURE) { > return; > } > zend_create_internal_iterator_zval(return_value, ZEND_THIS); > } > Given that the body of this method seems to usually (always?) be the same,
why not define it in InternalIterator and allow it to be inherited?
> There's some bikeshed potential here regarding the class name. > Not personally over-picky, but I do agree that "Internal*" is a bit
awkward. Unfortunately there's not much that's better and appropriate for exposing to scripts. This might be one of those rare occasions where exposing "Zend" into userspace makes sense. "PHP" is overloaded into several meanings that are significant for userspace developers, but something like "ZendIterator" might convey the right level of "This has to do with the engine, please move along". Or maybe go verbose: 'IteratorForExtensionClassImplementations'. :p
> ZEND_ASSERT(scope->get_iterator != zend_user_it_get_new_iterator); > Does this mean that if I do `class Foo implements InternalIterator {}` in a
script, I can assert (crash) PHP? I don't see anything obvious (at a glance) preventing me from inheriting from InternalIterator. -Sara
  110138
May 12, 2020 13:08 bobwei9@hotmail.com (Bob Weinand)
Hey Sara,

> Am 12.05.2020 um 14:49 schrieb Sara Golemon <pollita@php.net>: > > On Tue, May 12, 2020 at 3:26 AM Nikita Popov ppv@gmail.com> wrote: >> // WeakMap::getIterator(): Iterator >> ZEND_METHOD(WeakMap, getIterator) >> { >> if (zend_parse_parameters_none() == FAILURE) { >> return; >> } >> zend_create_internal_iterator_zval(return_value, ZEND_THIS); >> } >> > Given that the body of this method seems to usually (always?) be the same, > why not define it in InternalIterator and allow it to be inherited?
Good idea.
>> There's some bikeshed potential here regarding the class name. >> > Not personally over-picky, but I do agree that "Internal*" is a bit > awkward. Unfortunately there's not much that's better and appropriate for > exposing to scripts. This might be one of those rare occasions where > exposing "Zend" into userspace makes sense. "PHP" is overloaded into > several meanings that are significant for userspace developers, but > something like "ZendIterator" might convey the right level of "This has to > do with the engine, please move along". Or maybe go verbose: > 'IteratorForExtensionClassImplementations'. :p
We call the engine Zend, but eih, that's a rather internal detail we mostly don't leak into userland, and I wouldn't do it here either. InternalIterator is the better choice I think.
>> ZEND_ASSERT(scope->get_iterator != zend_user_it_get_new_iterator); > > Does this mean that if I do `class Foo implements InternalIterator {}` in a > script, I can assert (crash) PHP? I don't see anything obvious (at a > glance) preventing me from inheriting from InternalIterator.
The class is marked final, so userland won't be able to directly extend it. (zend_ce_internal_iterator->ce_flags |= ZEND_ACC_FINAL;) Bob
  110139
May 12, 2020 13:36 Danack@basereality.com (Dan Ackroyd)
On Tue, 12 May 2020 at 13:49, Sara Golemon <pollita@php.net> wrote:
> > This might be one of those rare occasions where > exposing "Zend" into userspace makes sense.
I think 'Engine' would be easier for userland people to grok. Also, for those brave souls who are implementing PHP in other technologies, avoiding using an implementation detail of the common internal engine, would be better for them. cheers Dan Ack
  110141
May 12, 2020 13:57 derick@php.net (Derick Rethans)
On Tue, 12 May 2020, Sara Golemon wrote:

> On Tue, May 12, 2020 at 3:26 AM Nikita Popov ppv@gmail.com> wrote: > > // WeakMap::getIterator(): Iterator > > ZEND_METHOD(WeakMap, getIterator) > > { > > if (zend_parse_parameters_none() == FAILURE) { > > return; > > } > > zend_create_internal_iterator_zval(return_value, ZEND_THIS); > > } > > > Given that the body of this method seems to usually (always?) be the same, > why not define it in InternalIterator and allow it to be inherited? > > > There's some bikeshed potential here regarding the class name. > > > Not personally over-picky, but I do agree that "Internal*" is a bit > awkward. Unfortunately there's not much that's better and appropriate for > exposing to scripts. This might be one of those rare occasions where > exposing "Zend" into userspace makes sense. "PHP" is overloaded into > several meanings that are significant for userspace developers, but > something like "ZendIterator" might convey the right level of "This has to > do with the engine, please move along". Or maybe go verbose: > 'IteratorForExtensionClassImplementations'. :p
I do not believe we should expose the Zend term into userland. Let the bike shedding start: Perhaps just "EngineIterator". cheers, Derick -- PHP 7.4 Release Manager Host of PHP Internals News: https://phpinternals.news Like Xdebug? Consider supporting me: https://xdebug.org/support https://derickrethans.nl | https://xdebug.org | https://dram.io twitter: @derickr and @xdebug
  110142
May 12, 2020 14:02 nicolas.grekas+php@gmail.com (Nicolas Grekas)
> I do not believe we should expose the Zend term into userland. >
Dunno if it helps or not, but there is a precedent here: ReflectionZendExtension Nicolas
  110448
June 9, 2020 13:23 nikita.ppv@gmail.com (Nikita Popov)
On Tue, May 12, 2020 at 2:49 PM Sara Golemon <pollita@php.net> wrote:

> On Tue, May 12, 2020 at 3:26 AM Nikita Popov ppv@gmail.com> wrote: > > // WeakMap::getIterator(): Iterator > > ZEND_METHOD(WeakMap, getIterator) > > { > > if (zend_parse_parameters_none() == FAILURE) { > > return; > > } > > zend_create_internal_iterator_zval(return_value, ZEND_THIS); > > } > > > Given that the body of this method seems to usually (always?) be the same, > why not define it in InternalIterator and allow it to be inherited? > > > There's some bikeshed potential here regarding the class name. > > > Not personally over-picky, but I do agree that "Internal*" is a bit > awkward. Unfortunately there's not much that's better and appropriate for > exposing to scripts. This might be one of those rare occasions where > exposing "Zend" into userspace makes sense. "PHP" is overloaded into > several meanings that are significant for userspace developers, but > something like "ZendIterator" might convey the right level of "This has to > do with the engine, please move along". Or maybe go verbose: > 'IteratorForExtensionClassImplementations'. :p > > > ZEND_ASSERT(scope->get_iterator != zend_user_it_get_new_iterator); > > Does this mean that if I do `class Foo implements InternalIterator {}` in > a script, I can assert (crash) PHP? I don't see anything obvious (at a > glance) preventing me from inheriting from InternalIterator. >
InternalIterator isn't an interface, so it's not possible to implement it in this way. It's not possible to directly construct an object either. What can happen is that the getIterator() method is replaced in a child class, but this will get picked up automatically, and the get_iterator handler will be changed to zend_user_it_get_new_iterator automatically. Nikita