Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:110734
MIME-Version: 1.0
References: <CACsECNfs4ZDOzkpRh_TqD7O8Mh0FU9MP0he0JoRoVMiVWXbEeQ@mail.gmail.com>
In-Reply-To: <CACsECNfs4ZDOzkpRh_TqD7O8Mh0FU9MP0he0JoRoVMiVWXbEeQ@mail.gmail.com>
Reply-To: Levi Morrison <levi.morrison@datadoghq.com>
Date: Fri, 26 Jun 2020 07:56:55 -0600
Message-ID: <CAAKU0un+vUWi9PuSvAFNd=LhZJDuRvvDMGHT4WL7o4VyH2tZ3Q@mail.gmail.com>
To: Alex <alexinbeijing@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [PHP-DEV] Convert SplFixedArray to Aggregate?
From: internals@lists.php.net ("Levi Morrison via internals")

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?