Re: [PR 614] ArrayObject and isset() / empty() behaviour

On Mon, Mar 10, 2014 at 9:58 PM, Tjerk Meesters <tjerk.meesters@gmail.com>wrote: > Hi Etienne, > > > On Mon, Mar 10, 2014 at 9:26 PM, Etienne Kneuss <colder@php.net> wrote: > >> >> >> >> On Mon, Mar 10, 2014 at 1:43 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote: >> >>> >>> >>> >>> On Sat, Mar 8, 2014 at 10:56 AM, Tjerk Meesters < >>> tjerk.meesters@gmail.com> wrote: >>> >>>> On Sat, Mar 8, 2014 at 8:27 AM, Tjerk Meesters < >>>> tjerk.meesters@gmail.com>wrote: >>>> >>>> > Hi Etienne, >>>> > >>>> > Thanks for your input, much appreciated. >>>> > >>>> > I have moved the decision up the chain as you suggested; the results >>>> can >>>> > be found here: >>>> > >>>> > >>>> > https://github.com/datibbaw/php-src/compare/php:PHP-5.4...bug66834a >>>> > >>>> >>>> > >>>> > It's suffering from a few memory leaks which can be fixed, >>>> > >>>> >>>> They are fixed now and a new PR has been opened: >>>> >>>> https://github.com/php/php-src/pull/616 >>>> >>>> >>> Hi Etienne, >>> >>> Are you fine with the changes in the PR? >>> >>> >> Two things: >> >> 1) Where is this planned to be merged? Is it really 5.4 ? As is the PR is >> no longer a simple bug fix and while I agree it should be changed that way, >> it might introduce hard-to-track BC breaks. >> > > After a short discussion on IRC I'm moving the branch to target 5.6 > instead. > > >> 2) Seems to me that with this PR, part of the hack present in ArrayObject >> in the has_dimension handler is no longer necessary, as it might be doing >> to value-check twice now. >> > > I'll start looking into this now. Thanks for the heads up :) > I've updated and cleaned up ext/spl; the most notable change is code removal according to a changed definition of zend_object_has_dimension_t: typedef int (*zend_object_has_dimension_t)(zval *object, zval *member TSRMLS_DC); PR: https://github.com/php/php-src/pull/621 Let me know if you see any other issues; I'm fine with reverting the typedef change if that could be an issue. > > >> >> >> Best, >> >> -- >>> Ferenc Kovács >>> @Tyr43l - http://tyrael.hu >>> >> >> >> >> -- >> Etienne Kneuss >> http://www.colder.ch >> > > > > -- > -- > Tjerk > -- -- Tjerk