Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110735
MIME-Version: 1.0
References: <CACsECNfs4ZDOzkpRh_TqD7O8Mh0FU9MP0he0JoRoVMiVWXbEeQ@mail.gmail.com>
 <CAAKU0un+vUWi9PuSvAFNd=LhZJDuRvvDMGHT4WL7o4VyH2tZ3Q@mail.gmail.com>
In-Reply-To: <CAAKU0un+vUWi9PuSvAFNd=LhZJDuRvvDMGHT4WL7o4VyH2tZ3Q@mail.gmail.com>
Date: Fri, 26 Jun 2020 16:04:30 +0200
Message-ID: <CACsECNcVpjMXqRCx1FJaK8RVrARhw=MZOUAktyamHcL1-Yu4jQ@mail.gmail.com>
To: Levi Morrison <levi.morrison@datadoghq.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] Convert SplFixedArray to Aggregate?
From: alexinbeijing@gmail.com (Alex)

Dear Levi, I will add a test case to the PR to ensure that things work
properly in the described situation.

On Fri, Jun 26, 2020 at 3:57 PM Levi Morrison
<levi.morrison@datadoghq.com> wrote:
>
> On Fri, Jun 26, 2020 at 12:45 AM Alex <alexinbeijing@gmail.com> wrote:
> >
> > Dear PHP Internals,
> >
> > I would like to propose a backwards-incompatible change to
> > SplFixedArray which fixes the strange and almost certainly unintended
> > behavior reported in Bug 79404
> > (https://bugs.php.net/bug.php?id=79404).
> >
> > In short: Because SplFixedArray is an Iterator, and stores its own
> > iteration state, it cannot be used in nested foreach loops. If you
> > try, the inner loop will overwrite the iteration position of the outer
> > loop, so that the outer loop 'thinks' it is finished after the inner
> > loop finishes.
> >
> > To illustrate:
> >
> > $spl = SplFixedArray::fromArray([0, 1]);
> > foreach ($spl as $a) {
> >   foreach ($spl as $b) {
> >     echo "$a $b";
> >   }
> > }
> >
> > Only prints two lines:
> >
> > 0 0
> > 0 1
> >
> > The fix is to convert SplFixedArray to an IteratorAggregate, so each
> > nested foreach loop operates on a different iterator object and thus
> > nested loops don't interfere with each other.
> >
> > Now, it would be possible to do this while still defining key(),
> > current(), next(), etc. methods on SplFixedArray. However, that would
> > cause an even more insidious BC break, since these methods would no
> > longer work as formerly when called in the body of a foreach loop.
> > Therefore, I think it better to remove them. This may break a few
> > users' code, but in a way which will be easier for them to debug than
> > if the old methods were kept but subtly changed in behavior.
> >
> > Code to implement this change is here:
> > https://github.com/php/php-src/pull/5384/files
> >
> > Your comments will be appreciated,
> > Alex Dowad
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List
> > To unsubscribe, visit: https://www.php.net/unsub.php
> >
>
> As someone who has patches in the SPL, I support the idea of this
> change. One gotcha of aggregates is iterator invalidation; are you
> sure the patch takes care of it? If you have an array of size 3, get
> an iterator, iterate to offset 2, shrink the size with `setSize`, and
> use the iterator at offset 2, will everything be okay?