Merge pull request #27 from Innmind/improve-deferred-sequence

Improve deferred sequence

Author: Baptouuuu

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Changed
 
 - Use `static` closures as much as possible to reduce the probability of creating circular references by capturing `$this` as it can lead to memory root buffer exhaustion.
+- Remove keeping intermediary values of a deferred `Sequence` that is referenced by no one.
 
 ### Fixed
 

proofs/sequence.php

@@ -97,4 +97,77 @@ static function($assert, $string, $chunk) {
             );
         },
     );
+
+    yield proof(
+        'Sequende::defer() holds intermediary values even when no longer used',
+        given(
+            Set\Sequence::of(Set\Type::any()),
+            Set\Sequence::of(Set\Type::any()),
+        ),
+        static function($assert, $prefix, $suffix) {
+            $initial = Sequence::defer((static function() use ($prefix, $suffix) {
+                foreach ($prefix as $value) {
+                    yield $value;
+                }
+
+                foreach ($suffix as $value) {
+                    yield $value;
+                }
+            })());
+
+            // This does a partial read on the generator
+            $assert->same(
+                $prefix,
+                $initial
+                    ->take(\count($prefix))
+                    ->toList(),
+            );
+
+            // The maps are only here to wrap the generator, it doesn't change
+            // the values
+            $another = $initial
+                ->map(static fn($value) => [$value])
+                ->map(static fn($values) => $values[0]);
+            unset($initial);
+
+            // If it didn't store the intermediary values the array would miss
+            // the prefix values due to the partial read on the initial
+            // generator due to the ->take()->toList() call above
+            $assert->same(
+                [...$prefix, ...$suffix],
+                $another->toList(),
+            );
+        },
+    );
+
+    yield proof(
+        "Sequence::defer() stack trace doesn't show intermediary sequences when not used",
+        given(Set\Integers::between(1, 10)),
+        static function($assert, $calls) {
+            $expected = null;
+            $sequence = Sequence::defer((static function() use (&$expected) {
+                yield null;
+
+                throw $expected = new Exception;
+            })());
+
+            for ($i = 0; $i < $calls; $i++) {
+                $sequence = $sequence->map(static fn($value) => $value);
+            }
+
+            try {
+                $sequence->toList();
+                $assert->fail('it should throw');
+            } catch (Exception $e) {
+                $assert->same($expected, $e);
+
+                $accumulations = \array_filter(
+                    $e->getTrace(),
+                    static fn($frame) => \str_ends_with($frame['file'] ?? '', 'src/Accumulate.php'),
+                );
+
+                $assert->count(1, $accumulations);
+            }
+        },
+    );
 };

src/Accumulate.php

@@ -7,23 +7,21 @@
  * Simple iterator to cache the results of a generator so it can be iterated
  * over multiple times
  *
- * @template T
  * @template S
- * @implements \Iterator<T, S>
+ * @implements \Iterator<S>
  * @internal Do not use this in your code
  * @psalm-immutable Not really immutable but to simplify declaring immutability of other structures
  */
 final class Accumulate implements \Iterator
 {
-    /** @var \Generator<T, S> */
+    /** @var \Generator<S> */
     private \Generator $generator;
-    /** @var list<T> */
-    private array $keys = [];
     /** @var list<S> */
     private array $values = [];
+    private bool $started = false;
 
     /**
-     * @param \Generator<T, S> $generator
+     * @param \Generator<S> $generator
      */
     public function __construct(\Generator $generator)
     {
@@ -35,27 +33,31 @@ public function __construct(\Generator $generator)
      */
     public function current(): mixed
     {
+        /** @psalm-suppress InaccessibleProperty */
+        $this->started = true;
         /** @psalm-suppress UnusedMethodCall */
         $this->pop();
 
         return \current($this->values);
     }
 
     /**
-     * @return T
+     * @return int<0, max>|null
      */
-    public function key(): mixed
+    public function key(): ?int
     {
+        /** @psalm-suppress InaccessibleProperty */
+        $this->started = true;
         /** @psalm-suppress UnusedMethodCall */
         $this->pop();
 
-        return \current($this->keys);
+        return \key($this->values);
     }
 
     public function next(): void
     {
         /** @psalm-suppress InaccessibleProperty */
-        \next($this->keys);
+        $this->started = true;
         /** @psalm-suppress InaccessibleProperty */
         \next($this->values);
 
@@ -68,13 +70,15 @@ public function next(): void
     public function rewind(): void
     {
         /** @psalm-suppress InaccessibleProperty */
-        \reset($this->keys);
+        $this->started = true;
         /** @psalm-suppress InaccessibleProperty */
         \reset($this->values);
     }
 
     public function valid(): bool
     {
+        /** @psalm-suppress InaccessibleProperty */
+        $this->started = true;
         /** @psalm-suppress ImpureMethodCall */
         $valid = !$this->reachedCacheEnd() || $this->generator->valid();
 
@@ -88,6 +92,11 @@ public function valid(): bool
         return $valid;
     }
 
+    public function started(): bool
+    {
+        return $this->started;
+    }
+
     private function reachedCacheEnd(): bool
     {
         return \key($this->values) === null;
@@ -96,11 +105,6 @@ private function reachedCacheEnd(): bool
     private function pop(): void
     {
         if ($this->reachedCacheEnd()) {
-            /**
-             * @psalm-suppress InaccessibleProperty
-             * @psalm-suppress ImpureMethodCall
-             */
-            $this->keys[] = $this->generator->key();
             /**
              * @psalm-suppress InaccessibleProperty
              * @psalm-suppress ImpureMethodCall