Everything

php

[RFC] Allow non-scalar keys in foreach

php.internals

Nikita Popov

13 years ago

Hi internals!

This RFC proposes to remove the type-restrictions on Iterator keys used in
foreach:

https://wiki.php.net/rfc/foreach-non-scalar-keys

I took over Levi's RFC and added a patch for it.

Thoughts?
Nikita

Derick Rethans

13 years ago

On Tue, 19 Feb 2013, Nikita Popov wrote:

> This RFC proposes to remove the type-restrictions on Iterator keys
> used in foreach:
>
> https://wiki.php.net/rfc/foreach-non-scalar-keys
>
> I took over Levi's RFC and added a patch for it.

Under "Open Questions" you write:

> What should be done with the keys that are valid in the iterator, but not in
> the array? I think the best approach would be to just set the array keys with
> the exact same semantics as PHP would do (i.e. with all casts and warnings).

Would __toString be called in case the key was an object?

cheers,
Derick

--
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
Posted with an email client that doesn't mangle email: alpine

Nikita Popov

13 years ago

On Tue, Feb 19, 2013 at 2:25 PM, Derick Rethans <derick@php.net> wrote:

> On Tue, 19 Feb 2013, Nikita Popov wrote:
>
> > This RFC proposes to remove the type-restrictions on Iterator keys
> > used in foreach:
> >
> > https://wiki.php.net/rfc/foreach-non-scalar-keys
> >
> > I took over Levi's RFC and added a patch for it.
>
> Under "Open Questions" you write:
>
> > What should be done with the keys that are valid in the iterator, but
> not in
> > the array? I think the best approach would be to just set the array keys
> with
> > the exact same semantics as PHP would do (i.e. with all casts and
> warnings).
>
> Would __toString be called in case the key was an object?
>

Current behavior of PHP can be found here:
http://lxr.php.net/xref/PHP_TRUNK/Zend/zend_execute.c#996 So if one goes
with that approach, then no, __toString won't be called, it'll throw a "*
Warning*: Illegal offset type" and not set the key. Don't know whether
that's good or bad ^^

Nikita

Etienne Kneuss

13 years ago

I think the warning can stay as-is, __toString is not necessarily available
so casting to string in all occasions is probably not what we want.

I'm glad somebody could take this over and provide a complete patch, thanks
Nikita!


On Tue, Feb 19, 2013 at 2:25 PM, Derick Rethans <derick@php.net> wrote:

> On Tue, 19 Feb 2013, Nikita Popov wrote:
>
> > This RFC proposes to remove the type-restrictions on Iterator keys
> > used in foreach:
> >
> > https://wiki.php.net/rfc/foreach-non-scalar-keys
> >
> > I took over Levi's RFC and added a patch for it.
>
> Under "Open Questions" you write:
>
> > What should be done with the keys that are valid in the iterator, but
> not in
> > the array? I think the best approach would be to just set the array keys
> with
> > the exact same semantics as PHP would do (i.e. with all casts and
> warnings).
>
> Would __toString be called in case the key was an object?
>
> cheers,
> Derick
>
> --
> http://derickrethans.nl | http://xdebug.org
> Like Xdebug? Consider a donation: http://xdebug.org/donate.php
> twitter: @derickr and @xdebug
> Posted with an email client that doesn't mangle email: alpine
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

--
Etienne Kneuss
http://www.colder.ch

Derick Rethans

13 years ago

Gah, no top posting!

On Tue, 19 Feb 2013, Etienne Kneuss wrote:

> On Tue, Feb 19, 2013 at 2:25 PM, Derick Rethans <derick@php.net> wrote:
>
> > On Tue, 19 Feb 2013, Nikita Popov wrote:
> >
> > > This RFC proposes to remove the type-restrictions on Iterator keys
> > > used in foreach:
> > >
> > > https://wiki.php.net/rfc/foreach-non-scalar-keys
> > >
> > > I took over Levi's RFC and added a patch for it.
> >
> > Under "Open Questions" you write:
> >
> > > What should be done with the keys that are valid in the iterator,
> > > but not in the array? I think the best approach would be to just
> > > set the array keys with the exact same semantics as PHP would do
> > > (i.e. with all casts and warnings).
> >
> > Would __toString be called in case the key was an object?

> I think the warning can stay as-is, __toString is not necessarily
> available so casting to string in all occasions is probably not what
> we want.

I think it should cast to a string if possible. You are now making
iterator_to_array not work with the new feature, and I find that a bit
silly. Whether it should be __toString (or __toKey) I don't really care,
but this new support for objects should be supported everywhere (and
that includes iterator_to_array).

cheers,
Derick

Etienne Kneuss

13 years ago

On Tue, Feb 19, 2013 at 2:55 PM, Derick Rethans <derick@php.net> wrote:

> Gah, no top posting!
>
> On Tue, 19 Feb 2013, Etienne Kneuss wrote:
>
> > On Tue, Feb 19, 2013 at 2:25 PM, Derick Rethans <derick@php.net> wrote:
> >
> > > On Tue, 19 Feb 2013, Nikita Popov wrote:
> > >
> > > > This RFC proposes to remove the type-restrictions on Iterator keys
> > > > used in foreach:
> > > >
> > > > https://wiki.php.net/rfc/foreach-non-scalar-keys
> > > >
> > > > I took over Levi's RFC and added a patch for it.
> > >
> > > Under "Open Questions" you write:
> > >
> > > > What should be done with the keys that are valid in the iterator,
> > > > but not in the array? I think the best approach would be to just
> > > > set the array keys with the exact same semantics as PHP would do
> > > > (i.e. with all casts and warnings).
> > >
> > > Would __toString be called in case the key was an object?
>
> > I think the warning can stay as-is, __toString is not necessarily
> > available so casting to string in all occasions is probably not what
> > we want.
>
> I think it should cast to a string if possible. You are now making
> iterator_to_array not work with the new feature, and I find that a bit
> silly. Whether it should be __toString (or __toKey) I don't really care,
> but this new support for objects should be supported everywhere (and
> that includes iterator_to_array).
>

I think this RFC is orthogonal to iterator_to_array supporting conversions
from objects to keys.

$array[$obj] = 42; does not call __toString (or __toKey), it throws a
catchable fatal. IMO it would be inconsistent if iterator_to_array
gracefully accepted objects as keys.

Levi Morrison

13 years ago

>> > I think the warning can stay as-is, __toString is not necessarily
>> > available so casting to string in all occasions is probably not what
>> > we want.
>>
>> I think it should cast to a string if possible. You are now making
>> iterator_to_array not work with the new feature, and I find that a bit
>> silly. Whether it should be __toString (or __toKey) I don't really care,
>> but this new support for objects should be supported everywhere (and
>> that includes iterator_to_array).
>>
>
> I think this RFC is orthogonal to iterator_to_array supporting conversions
> from objects to keys.
>
> $array[$obj] = 42; does not call __toString (or __toKey), it throws a
> catchable fatal. IMO it would be inconsistent if iterator_to_array
> gracefully accepted objects as keys.

Personally I think the real problem here is that `iterator_to_array`
should *not* copy keys by default. However, what's done is done and we
need to thoughtfully consider this situation.

You already CAN return objects from an iterator; you just can't use it
in a foreach.  If you try to use that iterator in `iteator_to_array`
you get a lovely warning:

Warning: Illegal type returned from %s::key() in %s on line %d

I'm all for *not* changing that behavior; if they want it ignore keys
then they can pass `false` as a second parameter to
`iterator_to_array`.

Nikita Popov

13 years ago

On Tue, Feb 19, 2013 at 3:12 PM, Etienne Kneuss <colder@php.net> wrote:

>
>
>
> On Tue, Feb 19, 2013 at 2:55 PM, Derick Rethans <derick@php.net> wrote:
>
>> Gah, no top posting!
>>
>> On Tue, 19 Feb 2013, Etienne Kneuss wrote:
>>
>> > On Tue, Feb 19, 2013 at 2:25 PM, Derick Rethans <derick@php.net> wrote:
>> >
>> > > On Tue, 19 Feb 2013, Nikita Popov wrote:
>> > >
>> > > > This RFC proposes to remove the type-restrictions on Iterator keys
>> > > > used in foreach:
>> > > >
>> > > > https://wiki.php.net/rfc/foreach-non-scalar-keys
>> > > >
>> > > > I took over Levi's RFC and added a patch for it.
>> > >
>> > > Under "Open Questions" you write:
>> > >
>> > > > What should be done with the keys that are valid in the iterator,
>> > > > but not in the array? I think the best approach would be to just
>> > > > set the array keys with the exact same semantics as PHP would do
>> > > > (i.e. with all casts and warnings).
>> > >
>> > > Would __toString be called in case the key was an object?
>>
>> > I think the warning can stay as-is, __toString is not necessarily
>> > available so casting to string in all occasions is probably not what
>> > we want.
>>
>> I think it should cast to a string if possible. You are now making
>> iterator_to_array not work with the new feature, and I find that a bit
>> silly. Whether it should be __toString (or __toKey) I don't really care,
>> but this new support for objects should be supported everywhere (and
>> that includes iterator_to_array).
>>
>
> I think this RFC is orthogonal to iterator_to_array supporting conversions
> from objects to keys.
>
> $array[$obj] = 42; does not call __toString (or __toKey), it throws a
> catchable fatal. IMO it would be inconsistent if iterator_to_array
> gracefully accepted objects as keys.
>

I updated the patch to use the usual array assignment behavior. I.e. the
behavior will be the same as the following code:

function iterator_to_array($iter) {
foreach ($iter as $k => $v) {
$array[$k] = $v;
}
return $array;
}

I think it's reasonable to expect that the code will behave this way.

If there are no objections I'd like to vote on this already on February
28th or March 1st. It is a few days shy of the 2 week discussion period,
but I'd like to have this in 5.5 and the beta is tagged on the 7th already
:)

Nikita