Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:73005
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.49 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAHMUw2HPbFKX5pds-BY0NLjyfX_LGONzYYL68t_nj7fDgObzBQ@mail.gmail.com>
References: <CAHMUw2GjeDRveLK99Q8U08y-3zqftAN8mLKPt=yikjT2BrNdBQ@mail.gmail.com>
	<CAMUwpuTxoiR-QhniYsBAebec0rusiu+FHoXx2hY=VY=z9um_mQ@mail.gmail.com>
	<CACQesk5+Wvy_kY9qjR6Ma9ChFjyvRjJrXa9DQYjOh43_uNu3hA@mail.gmail.com>
	<CACQesk6jiYdzzibjzr2ba5XC3U1hSRCnj1gyaC1KNDJMaP1+sQ@mail.gmail.com>
	<CAHMUw2HAOpM8E93rEyo6YVQab36sYbi4ssLoQPX+Kk8MMEJ-nA@mail.gmail.com>
	<CAHMUw2HPbFKX5pds-BY0NLjyfX_LGONzYYL68t_nj7fDgObzBQ@mail.gmail.com>
Date: Sat, 8 Mar 2014 11:50:06 -0500
Message-ID: <CAL+t6cFvhGGd-y2jH=kAnXYO+-VwDt_eYEmTLgbpFKRtqedkZw@mail.gmail.com>
To: Tjerk Meesters <tjerk.meesters@gmail.com>
Cc: Etienne Kneuss <colder@php.net>, Julien Pauli <jpauli@php.net>, 
	PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7bf0c106a3d44404f41b29f7
Subject: Re: [PHP-DEV] [PR 614] ArrayObject and isset() / empty() behaviour
From: theanomaly.is@gmail.com (Sherif Ramadan)

--047d7bf0c106a3d44404f41b29f7
Content-Type: text/plain; charset=ISO-8859-1

On Sat, Mar 8, 2014 at 4: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
>
>
> > but would this be acceptable otherwise?
> >
> >
> >
> > On Sat, Mar 8, 2014 at 12:12 AM, Etienne Kneuss <colder@php.net> wrote:
> >
> >>
> >>
> >>
> >> On Fri, Mar 7, 2014 at 4:58 PM, Etienne Kneuss <colder@php.net> wrote:
> >>
> >>>
> >>>
> >>> On Fri, Mar 7, 2014 at 3:15 PM, Julien Pauli <jpauli@php.net> wrote:
> >>>
> >>>> On Fri, Mar 7, 2014 at 11:14 AM, Tjerk Meesters
> >>>> <tjerk.meesters@gmail.com> wrote:
> >>>> > Hi,
> >>>> >
> >>>> > I come across the following bug:
> >>>> https://bugs.php.net/bug.php?id=66834
> >>>> >
> >>>> > With an extended ArrayObject instance, the behaviour of `isset()`
> and
> >>>> > `empty()` is as follows:
> >>>> >
> >>>> > 1. Call `static::offsetExists()`
> >>>> > 2. If return value is truthy, then
> >>>> >   a. isset() returns true
> >>>> >   b. empty() returns false
> >>>> >   c. spl_array_has_property() returns true
> >>>> >
> >>>> > However, if the ArrayObject instance is not overridden:
> >>>> >
> >>>> > 1. Fetch value (either via string or long key)
> >>>> > 2. If key is found, then:
> >>>> >   a. isset() returns false if value is `null`, true otherwise.
> >>>> >   b. empty() returns true if value is truty, false otherwise.
> >>>> >   c. spl_array_has_property() returns true
> >>>> >
> >>>> > That's counterintuitive, so my patch addresses this by also calling
> >>>> > `offsetGet()` if applicable. I wanted to bring this to the attention
> >>>> of the
> >>>> > group first, because it may affect performance to achieve this goal:
> >>>>
> >>>
> >>> I have always been annoyed by this dichotomy between offsetExists()
> >>> which is functionning like array_key_exists() and isset() which is
> >>> returning false on defined, null values. Especially since isset() calls
> >>> offsetExists() only.
> >>>
> >>> I would argue that the correct way to fix it is to have isset()/empty()
> >>> call offsetGet() as well if it exists. So in that sense I feel like
> your
> >>> patch is not addressing the right issue. In a sense ArrayObject does a
> hack
> >>> to circumvent that in the first place, and you are trying to fix the
> hack
> >>> to make it look more like what is supposed to happen.
> >>>
> >>>
> >> Apart from this underlying "design" issue, it is true an overridden
> >> version of ArrayObject is not functionally equivalent to the original
> >> class. So it seems worth fixing independently from this hack.
> >>  --
> >> Etienne Kneuss
> >>
> >
> >
> >
> > --
> > --
> > Tjerk
> >
>
>
>
> --
> --
> Tjerk
>


Could you actually make that into a sane macro or would that just be more
trouble than it's worth?

--047d7bf0c106a3d44404f41b29f7--