Everything

php

[RFC] Iterable

php.internals

Aaron Piotrowski

10 years ago

Hello,

I would like to propose a new iterable pseudo-type that accepts both arrays and objects implementing Traversable. Values accepted by iterable can then be used with foreach and yield from, or help to reduce type-checking logic in general.

Please review the RFC here: https://wiki.php.net/rfc/iterable <https://wiki.php.net/rfc/iterable>

Thanks!

Aaron Piotrowski

Christoph Becker

10 years ago

On 11.06.2016 at 01:37, Aaron Piotrowski wrote:

> I would like to propose a new iterable pseudo-type that accepts both arrays and objects implementing Traversable. Values accepted by iterable can then be used with foreach and yield from, or help to reduce type-checking logic in general.
>
> Please review the RFC here: https://wiki.php.net/rfc/iterable <https://wiki.php.net/rfc/iterable>

Thanks, Aaron!  I definitely like this idea (I'm not sure about the
details yet – would have to thoroughly investigate), as it is apparently
solving a long-standing issue, and is in my opinion a cleaner solution
than (explicitly) having to resort to a union type.

--
Christoph M. Becker

Levi Morrison

10 years ago

On Mon, Jun 13, 2016 at 5:42 AM, Christoph Becker <cmbecker69@gmx.de> wrote:

> On 11.06.2016 at 01:37, Aaron Piotrowski wrote:
>> I would like to propose a new iterable pseudo-type that accepts both arrays and objects implementing Traversable. Values accepted by iterable can then be used with foreach and yield from, or help to reduce type-checking logic in general.
>>
>> Please review the RFC here: https://wiki.php.net/rfc/iterable <https://wiki.php.net/rfc/iterable>
>
> Thanks, Aaron!  I definitely like this idea (I'm not sure about the
> details yet – would have to thoroughly investigate), as it is apparently
> solving a long-standing issue, and is in my opinion a cleaner solution
> than (explicitly) having to resort to a union type.

My opinion is contrary: the union type is the cleaner solution as it
does not require special casing machinery in the engine.

Christoph Becker

10 years ago

On 13.06.2016 at 17:59, Levi Morrison wrote:

> On Mon, Jun 13, 2016 at 5:42 AM, Christoph Becker <cmbecker69@gmx.de> wrote:
>> On 11.06.2016 at 01:37, Aaron Piotrowski wrote:
>>> I would like to propose a new iterable pseudo-type that accepts both arrays and objects implementing Traversable. Values accepted by iterable can then be used with foreach and yield from, or help to reduce type-checking logic in general.
>>>
>>> Please review the RFC here: https://wiki.php.net/rfc/iterable <https://wiki.php.net/rfc/iterable>
>>
>> Thanks, Aaron!  I definitely like this idea (I'm not sure about the
>> details yet – would have to thoroughly investigate), as it is apparently
>> solving a long-standing issue, and is in my opinion a cleaner solution
>> than (explicitly) having to resort to a union type.
>
> My opinion is contrary: the union type is the cleaner solution as it
> does not require special casing machinery in the engine.

I agree that union types are the cleaner solution with regard to the (C)
implementation.  I don't think, though, that they are the cleaner
solution with regard to PHP code; a developer has to think twice which
guarantees a union types gives regarding the allowed operations.  For
instance, what would Foo|Bar guarantee?  I would have to look up both
respective definitions and find the intersection – and that still leaves
room for errors, as operations with the same name do not guarantee
equivalent behavior.  The special case of reading and understanding
`Traversable|array` might quickly get second nature for developers, but
it's still ugly to write and read, in my opinion.

--
Christoph M. Becker

Fleshgrinder

10 years ago

On 6/13/2016 5:59 PM, Levi Morrison wrote:

> My opinion is contrary: the union type is the cleaner solution as it
> does not require special casing machinery in the engine.
>

I am a huge fan of the union types but this is such a common requirement
that it requires a pseudo compound type in my opinion. The special
casing in the engine is trivial:

https://github.com/php/php-src/pull/1941

I also hope that this allows expansion in internals and enable us to
rewrite existing functions from array to iterable and directly iterate
them without special casing.

Furthermore I argue the we need more of those pseudo compound types that
are special in internals and userland and I might implemented them if I
am ever going to be granted some Karma:

number     := is_numeric || is_int || is_float
stringable := is_string  || method_exists(__toString)

These three things cannot even be easily expressed with union types.
(Well, the last one can if one creates a Stringable interface and you
end up having string|Stringable :P)

--
Richard "Fleshgrinder" Fussenegger

Aaron Piotrowski

10 years ago

Hi all,

I plan on bringing the iterable RFC (https://wiki.php.net/rfc/iterable) to a vote in about a week, does anyone have any further feedback on this proposal?

Thanks!

Aaron Piotrowski

Dan Ackroyd

10 years ago

Hi Aaron,

> does anyone have any further feedback on this proposal?

What is the performance impact of the RFC on the standard performance
benchmarks?

And can you comment on the performance of using iterable as a type for
parameters/return types, compared to normal classes?

cheers
Dan

Aaron Piotrowski

10 years ago

> On Jun 18, 2016, at 6:11 PM, Dan Ackroyd <danack@basereality.com> wrote:
>
> Hi Aaron,
>
>> does anyone have any further feedback on this proposal?
>
> What is the performance impact of the RFC on the standard performance
> benchmarks?
>
> And can you comment on the performance of using iterable as a type for
> parameters/return types, compared to normal classes?
>
> cheers
> Dan

Hi Dan,

Great questions!

This RFC has litte to no impact to the standard performance benchmarks. These benchmarks do not have functions with type declarations, so they are largely unaffected by an of the code changed by this RFC.

The largest impact is to functions with scalar parameter or return type declarations. An extra integer comparison is performed during the compilation step to check if the type declaration equals IS_ITERABLE. [1] for parameter and [2] for return types. If a parameter is not a class name and has a default argument, another integer compare is performed [3].

The patch also performs an additional integer compare to IS_ITERABLE when a type not directly matching the type declaration is provided and the type declaration is a scalar type. This applies to both parameter [4] and return type declarations [5].

User code using `iterable` would only be slightly slower than using `array` or `Traversable`, as a few extra integer comparisons are made and additional function call is made to verify the type matches one of the two allowed types (see [4] and [5] again). However, the flexibility provided by `iterable` is well worth this very small penalty. For comparison, this overhead is far less than the `callable` pseudo-type as the value is not examined, only the type.

In my tests against type declarations using other interfaces such as `Iterator` or `IteratorAggregate`, using `iterable` was about equal in performance. [6]

Overall in my opinion, the performance impact of this RFC is negligible.

Dmitry, could you take a look at the patch and do some of your own performance checks?

Thanks!

Aaron Piotrowski

[1] https://github.com/php/php-src/pull/1941/files#diff-9760ee109b1c5922071fac1e19d117dfR332
[2] https://github.com/php/php-src/pull/1941/files#diff-9760ee109b1c5922071fac1e19d117dfR360
[3] https://github.com/php/php-src/pull/1941/files#diff-3a8139128d4026ce0cb0c86beba4e6b9R5082
[4] https://github.com/php/php-src/pull/1941/files#diff-a5fb5fd8c4f7311c6d0788763e665daaR857
[5] https://github.com/php/php-src/pull/1941/files#diff-a5fb5fd8c4f7311c6d0788763e665daaR1080
[6] https://gist.github.com/trowski/6f4b8b689228c7ba930fa658bb3c87f9

Dmitry Stogov

10 years ago

From the first look, the patch shouldn't make any visible effect on performance.

According to the proposed new pseudo type - I don't have any strong opinion.


Thanks. Dmitry.

________________________________
From: Aaron Piotrowski <aaron@trowski.com>
Sent: Sunday, June 19, 2016 7:35:50 PM
To: Dan Ackroyd
Cc: internals@lists.php.net; Dmitry Stogov
Subject: Re: [PHP-DEV] [RFC] Iterable

> On Jun 18, 2016, at 6:11 PM, Dan Ackroyd <danack@basereality.com> wrote:
>
> Hi Aaron,
>
>> does anyone have any further feedback on this proposal?
>
> What is the performance impact of the RFC on the standard performance
> benchmarks?
>
> And can you comment on the performance of using iterable as a type for
> parameters/return types, compared to normal classes?
>
> cheers
> Dan

Hi Dan,

Great questions!

This RFC has litte to no impact to the standard performance benchmarks. These benchmarks do not have functions with type declarations, so they are largely unaffected by an of the code changed by this RFC.

The largest impact is to functions with scalar parameter or return type declarations. An extra integer comparison is performed during the compilation step to check if the type declaration equals IS_ITERABLE. [1] for parameter and [2] for return types. If a parameter is not a class name and has a default argument, another integer compare is performed [3].

The patch also performs an additional integer compare to IS_ITERABLE when a type not directly matching the type declaration is provided and the type declaration is a scalar type. This applies to both parameter [4] and return type declarations [5].

User code using `iterable` would only be slightly slower than using `array` or `Traversable`, as a few extra integer comparisons are made and additional function call is made to verify the type matches one of the two allowed types (see [4] and [5] again). However, the flexibility provided by `iterable` is well worth this very small penalty. For comparison, this overhead is far less than the `callable` pseudo-type as the value is not examined, only the type.

In my tests against type declarations using other interfaces such as `Iterator` or `IteratorAggregate`, using `iterable` was about equal in performance. [6]

Overall in my opinion, the performance impact of this RFC is negligible.

Dmitry, could you take a look at the patch and do some of your own performance checks?

Thanks!

Aaron Piotrowski

[1] https://github.com/php/php-src/pull/1941/files#diff-9760ee109b1c5922071fac1e19d117dfR332
[2] https://github.com/php/php-src/pull/1941/files#diff-9760ee109b1c5922071fac1e19d117dfR360
[3] https://github.com/php/php-src/pull/1941/files#diff-3a8139128d4026ce0cb0c86beba4e6b9R5082
[4] https://github.com/php/php-src/pull/1941/files#diff-a5fb5fd8c4f7311c6d0788763e665daaR857
[5] https://github.com/php/php-src/pull/1941/files#diff-a5fb5fd8c4f7311c6d0788763e665daaR1080
[6] https://gist.github.com/trowski/6f4b8b689228c7ba930fa658bb3c87f9

Jeremy Mikola

10 years ago

On Sat, Jun 18, 2016 at 11:34 AM, Aaron Piotrowski <aaron@trowski.com>
wrote:

>
> I plan on bringing the iterable RFC (https://wiki.php.net/rfc/iterable)
> to a vote in about a week, does anyone have any further feedback on this
> proposal?
>

Was there any discussion about a special allowance being made for stdClass?

I've noted the is_iterable(new stdClass) example and ensuing "Object
Iteration" section:

PHP allows any object to be used with foreach. However, iterable does not

> accept any object, only those implementing Traversable. Values accepted
> by iterable should be designed for iteration, not any set of values (such
> as the public properties of an object or a string).
>

I'm on the fence if that second sentence applies to stdClass. Based on how
users typically end up with stdClass instances (e.g. casting arrays to an
object, json_decode()), I tend to think of them as logically equivalent to
associative arrays. To that end, I'd consider stdClass to be as "designed
for iteration" as any associative array -- if we can even say a stdClass
was designed at all :)

As-is, the RFC requires users to either cast stdClass instances to an array
or decorate them with an Iterator (e.g. ArrayObject). I find this a bit
irksome, but it's certainly easy to work around.

That said, I realize that voting is in progress and it's not my intention
to interrupt anything. I just wanted to relay a viewpoint that might not
have come up.

--
jeremy mikola

Nikita Popov

10 years ago

On Thu, Jun 30, 2016 at 10:01 PM, Jeremy Mikola <jmikola@gmail.com> wrote:

> On Sat, Jun 18, 2016 at 11:34 AM, Aaron Piotrowski <aaron@trowski.com>
> wrote:
>
> >
> > I plan on bringing the iterable RFC (https://wiki.php.net/rfc/iterable)
> > to a vote in about a week, does anyone have any further feedback on this
> > proposal?
> >
>
> Was there any discussion about a special allowance being made for stdClass?
>
> I've noted the is_iterable(new stdClass) example and ensuing "Object
> Iteration" section:
>
> PHP allows any object to be used with foreach. However, iterable does not
> > accept any object, only those implementing Traversable. Values accepted
> > by iterable should be designed for iteration, not any set of values (such
> > as the public properties of an object or a string).
> >
>
> I'm on the fence if that second sentence applies to stdClass. Based on how
> users typically end up with stdClass instances (e.g. casting arrays to an
> object, json_decode()), I tend to think of them as logically equivalent to
> associative arrays. To that end, I'd consider stdClass to be as "designed
> for iteration" as any associative array -- if we can even say a stdClass
> was designed at all :)
>
> As-is, the RFC requires users to either cast stdClass instances to an array
> or decorate them with an Iterator (e.g. ArrayObject). I find this a bit
> irksome, but it's certainly easy to work around.
>
> That said, I realize that voting is in progress and it's not my intention
> to interrupt anything. I just wanted to relay a viewpoint that might not
> have come up.
>
> --
> jeremy mikola
>

Instead of adding a special case to iterable, we could also make stdClass
implement Traversable. (If we agree that stdClass is explicitly intended
for iteration -- not so sure on that.)

Nikita

Aaron Piotrowski

10 years ago

Hi Jeremy,

> On Jun 30, 2016, at 3:01 PM, Jeremy Mikola <jmikola@gmail.com> wrote:
>
> Was there any discussion about a special allowance being made for stdClass?
>
> I've noted the is_iterable(new stdClass) example and ensuing "Object Iteration" section:
>
> PHP allows any object to be used with foreach. However, iterable does not accept any object, only those implementing Traversable. Values accepted by iterable should be designed for iteration, not any set of values (such as the public properties of an object or a string).
>
> I'm on the fence if that second sentence applies to stdClass. Based on how users typically end up with stdClass instances (e.g. casting arrays to an object, json_decode()), I tend to think of them as logically equivalent to associative arrays. To that end, I'd consider stdClass to be as "designed for iteration" as any associative array -- if we can even say a stdClass was designed at all :)
>
> As-is, the RFC requires users to either cast stdClass instances to an array or decorate them with an Iterator (e.g. ArrayObject). I find this a bit irksome, but it's certainly easy to work around.
>
> That said, I realize that voting is in progress and it's not my intention to interrupt anything. I just wanted to relay a viewpoint that might not have come up.
>

Generally when I've seen an object that did not implement Traversable used with foreach it has been an error. The exception of course is stdClass, particularly because of json_decode(). There was no discussion on the list of allowing stdClass, but I did discuss it with some people in chat. Our consensus was that a casting to an array was a reasonable and simple requirement to allow an instance of stdClass to be iterable. I do not think stdClass was designed for iteration, but rather is just used this way because of the behavior of foreach. I'd prefer json_decode() to return an iterable object... maybe an RFC for another day.

Aaron Piotrowski

Jeremy Mikola

10 years ago

On Saturday, July 2, 2016, Aaron Piotrowski <aaron@trowski.com> wrote:

>
>
>

There was no discussion on the list of allowing stdClass, but I did discuss

> it with some people in chat. Our consensus was that a casting to an array
> was a reasonable and simple requirement to allow an instance of stdClass to
> be iterable.
>

Sounds good to me. Happy to hear it was discussed. Thanks!

--
jeremy mikola

Stas Malyshev

10 years ago

Hi!

> Generally when I've seen an object that did not implement Traversable
> used with foreach it has been an error. The exception of course is
> stdClass, particularly because of json_decode(). There was no
> discussion on the list of allowing stdClass, but I did discuss it
> with some people in chat. Our consensus was that a casting to an
> array was a reasonable and simple requirement to allow an instance of
> stdClass to be iterable. I do not think stdClass was designed for

I'm not sure I understand this part. Do you mean that this code:

$a = json_decode('{"a": 1, "b": 2, "c": 3}');
foreach($a as $b) {
var_dump($b);
}

will no longer work, or that it will still work, but stdClass would not
be instanceof Iterable?

--
Stas Malyshev
smalyshev@gmail.com

Aaron Piotrowski

10 years ago

Hi Stas,

> On Jul 2, 2016, at 7:00 PM, Stanislav Malyshev <smalyshev@gmail.com> wrote:
>
> Do you mean that this code:
>
> $a = json_decode('{"a": 1, "b": 2, "c": 3}');
> foreach($a as $b) {
>    var_dump($b);
> }
>
> will no longer work, or that it will still work, but stdClass would not
> be instanceof Iterable?

That code will continue to work just as it always has. stdClass will not be accepted by the iterable type.

Aaron Piotrowski

Sara Golemon

10 years ago

On Sun, Jul 3, 2016 at 9:50 AM, Aaron Piotrowski <aaron@trowski.com> wrote:

>> Do you mean that this code:
>>
>> $a = json_decode('{"a": 1, "b": 2, "c": 3}');
>> foreach($a as $b) {
>>    var_dump($b);
>> }
>>
>> will no longer work, or that it will still work, but stdClass would not
>> be instanceof Iterable?
>
> That code will continue to work just as it always has. stdClass will not be accepted by the iterable type.
>

As, it's worth noting, it was never accepted for Traversable.
https://3v4l.org/XJnNi

-Sara