Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:66597
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain zend.com does not designate 209.85.214.169 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAF+90c-8gpQN9N-tvSROg=taW4d3JeG5cDjhF=Cnft3H64jLsw@mail.gmail.com>
References: <CAF+90c-E7n-WZcqNg28bX+G7zgbzhKoNCwR6hf++tfAgSzmtTw@mail.gmail.com>
	<CAF+90c-wSqeff_2oY8HeY+gDn-ZnwJeXKQ+cz-1W6xcEBjs+xw@mail.gmail.com>
	<CA+9eiLtiOcFuxRXhpGXe5b9SpyVPB1_ixJ4t1GQ3J1bvBURMKA@mail.gmail.com>
	<CAF+90c92AwjX7Niro_tT5vOv+vHeY-AmgfizL5f9i1FYhKTz2A@mail.gmail.com>
	<CA+9eiLsWNV7H1zAr1U-wPafz9SzB=eR_C33RQKgqy1Tx1+X8gw@mail.gmail.com>
	<CAF+90c_Nt=Q-vTHO3iqC24mkwULMvV+KxpYpy1ug5=7em-x1eg@mail.gmail.com>
	<CA+9eiLuV_af9wAS_1zd_NYw=NXo1kJs990s5+bb_ZQ6h8=yW4Q@mail.gmail.com>
	<CA+9eiLvnmHA7fQonaLj-oRWp_d7HmEbtoejfqWrKQxK-DOTjMg@mail.gmail.com>
	<CAF+90c9oX_cr4Ah33BF5sYTbAicuZM6ibtXnWv4QCYW=6GGp=w@mail.gmail.com>
	<CA+9eiLs3hWwqLqBD+ozRLuOmfTZ8Zv-RRZqXJfUMe1zr=w1GTw@mail.gmail.com>
	<CAF+90c-8gpQN9N-tvSROg=taW4d3JeG5cDjhF=Cnft3H64jLsw@mail.gmail.com>
Date: Tue, 12 Mar 2013 21:18:25 +0400
Message-ID: <CA+9eiLtmaR_vakCva6zGV+uWe71xbmqbacH_a3bC5qT4EivFQQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>, Zeev Suraski <zeev@zend.com>, Andi Gutmans <andi@zend.com>
Content-Type: multipart/alternative; boundary=e89a8ff1c8aa2ef3e804d7bd7a73
Subject: Re: [PHP-DEV] Re: [VOTE] Allow non-scalar keys in foreach
From: dmitry@zend.com (Dmitry Stogov)

--e89a8ff1c8aa2ef3e804d7bd7a73
Content-Type: text/plain; charset=UTF-8

Hi Nikita,

I suppose it must fine now, but let me take a quick look tomorrow morning.

Thanks. Dmitry.

On Tue, Mar 12, 2013 at 8:43 PM, Nikita Popov <nikita.ppv@gmail.com> wrote:

> On Mon, Mar 11, 2013 at 8:42 PM, Dmitry Stogov <dmitry@zend.com> wrote:
>
>>
>>
>> On Mon, Mar 11, 2013 at 10:27 PM, Nikita Popov <nikita.ppv@gmail.com>wrote:
>>
>>> On Mon, Mar 11, 2013 at 9:50 AM, Dmitry Stogov <dmitry@zend.com> wrote:
>>>
>>>> Hi Nikita,
>>>>
>>>> The patch looks good. I have just few comments
>>>>
>>>> - In ZEND_FE_FETCH handler PLAIN_OBJECT may have only STRING keys. I
>>>> didn't get why you added unreachable code for INT and NULL.
>>>>
>>>
>>> You are right about the NULL case, that can indeed not occur. But INT is
>>> possible, e.g. consider this:
>>>
>>> <?php
>>> foreach ((object) ['x','y','z'] as $k => $v) {
>>>     var_dump($k);
>>> }
>>>
>>> this will give you keys int(0), int(1), int(2).
>>>
>>
>> Agree. I missed it.
>>
>>
>>> I'll remove the check for NULL.
>>>
>>>
>>> - At first, I fought, that it might be a good idea to change
>>>> zend_user_it_get_current_key() to return SUCCESS/FAILURE instead of
>>>> returning IS_NULL that has a special meaning. But after looking into the
>>>> FE_FETCH code before the commit I understood where this NULL came from. I
>>>> know that NULL key may not appear for plain array and objects but I'm not
>>>> sure about iterators and generators. Now IS_NULL keys may mean that
>>>> iterator returned it directly IS_NULL or may be it was returned because of
>>>> some error conditions. Probably it's not a problem. What do you think?
>>>>
>>>
>>> In foreach IS_NULL can't occur in an error condition, because the loop
>>> is already aborted when get_current_data provides NULL. IS_NULL can only
>>> happen when its explicitly provided (or handlers are incorrectly coded). I
>>> think the IS_NULL fallback is mainly important when the iterator is used
>>> for other things (like a dual it), to be sure that it'll always work. I
>>> don't think that it's important to distinguish between explicit NULL and
>>> failure NULL.
>>>
>>
>> Agree as well.
>>
>>
>>>  I have one more question: Right now I added the
>>> zend_hash_get_current_key_zval API in zend_hash.c/h, which is a bit ugly
>>> because it has a zval dependency (unlike all the other code in there) and
>>> requires me to forward-declare the zval struct. Would it be better to move
>>> this somewhere else, e.g. zend_API.c/h? If so, can I still keep the same
>>> name (with the zend_hash_ prefix)? If I move it to zend_API, I won't be
>>> able to do the IS_CONSISTENT check anymore, is that a problem?
>>>
>>
>> I think we can move zval forward declaration (typedef struct _zval_struct
>> zval;) from zend.h into zend_type.h.
>>
>
> I now merged the changeset in
> https://github.com/php/php-src/commit/fcc6611de9054327441786e52444b5f8eecdd525(for PHP-5.5 and master) with the forward declaration moved. Also updated
> some array functions to use the new get_current_key_zval function :)
>
> Nikita
>

--e89a8ff1c8aa2ef3e804d7bd7a73--