Re: Convert SplFixedArray to Aggregate?

From: Date: Fri, 26 Jun 2020 13:56:55 +0000
Subject: Re: Convert SplFixedArray to Aggregate?
References: 1  Groups: php.internals 
Request: Send a blank email to internals+get-110734@lists.php.net to get a copy of this message
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?

« previous php.internals (#110734) next »