Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110758
MIME-Version: 1.0
References: <CACsECNfs4ZDOzkpRh_TqD7O8Mh0FU9MP0he0JoRoVMiVWXbEeQ@mail.gmail.com>
In-Reply-To: <CACsECNfs4ZDOzkpRh_TqD7O8Mh0FU9MP0he0JoRoVMiVWXbEeQ@mail.gmail.com>
Date: Sun, 28 Jun 2020 14:41:02 +0200
Message-ID: <CAF+90c-6B9OSUUicH=LDuZ3gOrDzmfHrKs1Syu7MQ+3k+Y7P6g@mail.gmail.com>
To: Alex <alexinbeijing@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000037522905a9244227"
Subject: Re: [PHP-DEV] Convert SplFixedArray to Aggregate?
From: nikita.ppv@gmail.com (Nikita Popov)

--00000000000037522905a9244227
Content-Type: text/plain; charset="UTF-8"

On Fri, Jun 26, 2020 at 8: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
>

I agree that we should make this change for PHP 8. The current behavior is
pretty clearly a bug, and the practical backwards compatibility impact
should be rather limited (and only annoying to work around if you extend
SplFixedArray and override Iterator methods).

Nikita

--00000000000037522905a9244227--