Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:73318
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.180 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAHMUw2E3WyLYV_7LRXKANq89g27fVobxzK_uZdGb+hu6035PWg@mail.gmail.com>
References: <CAHMUw2EeYA3m4QnRtLYm1PRa3taGiS8Sx5GxdN==ZYTQoFV=vQ@mail.gmail.com>
	<532A3E88.20202@sugarcrm.com>
	<ECAD39ED-2A1D-4B54-80D5-4CB4FBA71364@gmail.com>
	<532A418A.8020607@sugarcrm.com>
	<CAHMUw2E3WyLYV_7LRXKANq89g27fVobxzK_uZdGb+hu6035PWg@mail.gmail.com>
Date: Thu, 20 Mar 2014 10:41:29 +0800
Message-ID: <CAHMUw2FfYDwCkTHB5h92xXegtZY4GVyZOWBEo4WrWv5m8TxxYg@mail.gmail.com>
To: Stas Malyshev <smalyshev@sugarcrm.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a1136526ad280d604f500b43a
Subject: Re: [PHP-DEV] Merge PR 621
From: tjerk.meesters@gmail.com (Tjerk Meesters)

--001a1136526ad280d604f500b43a
Content-Type: text/plain; charset=ISO-8859-1

Sorry, I hit "Send" too soon.

On Thu, Mar 20, 2014 at 10:36 AM, Tjerk Meesters
<tjerk.meesters@gmail.com>wrote:

>
>
>
> On Thu, Mar 20, 2014 at 9:16 AM, Stas Malyshev <smalyshev@sugarcrm.com>wrote:
>
>> Hi!
>>
>> > Yes, that's probably what it was supposed to do, but as you can see
>> > from bug 66834 that's clearly not the case. I had earlier approached
>> > the problem purely from SPL standpoint (see linked PR) but Etienne
>> > said that it would be a better idea to tackle this issue further up
>> > the chain so to speak :)
>>
>> I think we need to first find out why that code did not work, instead of
>> ripping out the code that already was supposed to do exactly what this
>> bug is saying and replacing it with other code.
>
>
> Here's my earlier work on this: https://github.com/php/php-src/pull/614
>
> It currently "doesn't work" because isset()/empty() work like
> array_key_exists() on objects that extend ArrayObject, which is in some
> cases not the desired behaviour; see also:
> http://lxr.php.net/xref/PHP_5_6/ext/spl/spl_array.c#604
>
> If 'offsetExists()' is not overridden, the logic is done correctly; see
> also: http://lxr.php.net/xref/PHP_5_6/ext/spl/spl_array.c#617
>

I forgot to mention that the current behaviour is not necessarily correct
if `offsetGet()` is overridden in a child class; the overridden method may
decide to return `null` or something empty.


> The behaviour is currently correct inside zend_std_has_dimension(), so
> making the fix in SPL alone (#614) would make performance "suffer" less
> unless offsetGet() is overridden.
>
> Personally I'm fine either way; like I said, my first patch was only done
> in SPL when Etienne suggested the approach in #621.
>
>
>> At least we need to know
>> the reason why that did not work. So please do not merge this patch
>> until we know what's going on there.
>>
>> --
>> Stanislav Malyshev, Software Architect
>> SugarCRM: http://www.sugarcrm.com/
>> (408)454-6900 ext. 227
>>
>
>
>
> --
> --
> Tjerk
>



-- 
--
Tjerk

--001a1136526ad280d604f500b43a--