diff --git a/CHANGELOG.md b/CHANGELOG.md index d21c88d0..69f13ced 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/proofs/sequence.php b/proofs/sequence.php index 606515f8..3fdd02f8 100644 --- a/proofs/sequence.php +++ b/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); + } + }, + ); }; diff --git a/src/Accumulate.php b/src/Accumulate.php index 829b3a6b..89a650f3 100644 --- a/src/Accumulate.php +++ b/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 + * @implements \Iterator * @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 */ + /** @var \Generator */ private \Generator $generator; - /** @var list */ - private array $keys = []; /** @var list */ private array $values = []; + private bool $started = false; /** - * @param \Generator $generator + * @param \Generator $generator */ public function __construct(\Generator $generator) { @@ -35,6 +33,8 @@ public function __construct(\Generator $generator) */ public function current(): mixed { + /** @psalm-suppress InaccessibleProperty */ + $this->started = true; /** @psalm-suppress UnusedMethodCall */ $this->pop(); @@ -42,20 +42,22 @@ public function current(): mixed } /** - * @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 diff --git a/src/Sequence/Defer.php b/src/Sequence/Defer.php index c9c32071..ec4e93fc 100644 --- a/src/Sequence/Defer.php +++ b/src/Sequence/Defer.php @@ -20,14 +20,18 @@ */ final class Defer implements Implementation { - /** @var \Iterator */ - private \Iterator $values; + /** @var Accumulate */ + private Accumulate $values; + /** @var \Generator */ + private \Generator $generator; + /** + * @param \Generator $generator + */ public function __construct(\Generator $generator) { /** * @psalm-suppress ImpureFunctionCall - * @var \Iterator */ $this->values = new Accumulate((static function(\Generator $generator): \Generator { /** @var T $value */ @@ -35,6 +39,7 @@ public function __construct(\Generator $generator) yield $value; } })($generator)); + $this->generator = $generator; } /** @@ -44,22 +49,26 @@ public function __construct(\Generator $generator) */ public function __invoke($element): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, mixed $element): \Generator { + (static function(mixed $element) use ($captured): \Generator { + $values = self::detonate($captured); + /** @var T $value */ foreach ($values as $value) { yield $value; } yield $element; - })($this->values, $element), + })($element), ); } public function size(): int { - return $this->load()->size(); + return $this->memoize()->size(); } public function count(): int @@ -68,7 +77,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { @@ -80,10 +89,12 @@ public function iterator(): \Iterator */ public function get(int $index): Maybe { - $values = $this->values; + $captured = $this->capture(); - return Maybe::defer(static function() use ($values, $index) { + return Maybe::defer(static function() use ($captured, $index) { $iteration = 0; + /** @var \Iterator */ + $values = self::detonate($captured); foreach ($values as $value) { if ($index === $iteration) { @@ -116,13 +127,16 @@ public function diff(Implementation $sequence): Implementation */ public function distinct(): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values): \Generator { + (static function() use ($captured): \Generator { /** @var list */ $uniques = []; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { if (!\in_array($value, $uniques, true)) { $uniques[] = $value; @@ -130,7 +144,7 @@ public function distinct(): Implementation yield $value; } } - })($this->values), + })(), ); } @@ -139,12 +153,15 @@ public function distinct(): Implementation */ public function drop(int $size): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, int $toDrop): \Generator { + (static function(int $toDrop) use ($captured): \Generator { $dropped = 0; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { if ($dropped < $toDrop) { ++$dropped; @@ -154,7 +171,7 @@ public function drop(int $size): Implementation yield $value; } - })($this->values, $size), + })($size), ); } @@ -167,7 +184,7 @@ public function dropEnd(int $size): Implementation { // this cannot be optimised as the whole generator needs to be loaded // in order to know the elements to drop - return $this->load()->dropEnd($size); + return $this->memoize()->dropEnd($size); } /** @@ -175,7 +192,7 @@ public function dropEnd(int $size): Implementation */ public function equals(Implementation $sequence): bool { - return $this->load()->equals($sequence); + return $this->memoize()->equals($sequence); } /** @@ -185,16 +202,20 @@ public function equals(Implementation $sequence): bool */ public function filter(callable $predicate): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $predicate): \Generator { - /** @var T $value */ + (static function(callable $predicate) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { if ($predicate($value)) { yield $value; } } - })($this->values, $predicate), + })($predicate), ); } @@ -220,7 +241,7 @@ public function foreach(callable $function): SideEffect public function groupBy(callable $discriminator): Map { /** @var Map> */ - return $this->load()->groupBy($discriminator); + return $this->memoize()->groupBy($discriminator); } /** @@ -228,9 +249,12 @@ public function groupBy(callable $discriminator): Map */ public function first(): Maybe { - $values = $this->values; + $captured = $this->capture(); + + return Maybe::defer(static function() use ($captured) { + /** @var \Iterator */ + $values = self::detonate($captured); - return Maybe::defer(static function() use ($values) { foreach ($values as $value) { return Maybe::just($value); } @@ -245,10 +269,12 @@ public function first(): Maybe */ public function last(): Maybe { - $values = $this->values; + $captured = $this->capture(); - return Maybe::defer(static function() use ($values) { + return Maybe::defer(static function() use ($captured) { $loaded = false; + /** @var \Iterator */ + $values = self::detonate($captured); foreach ($values as $value) { $loaded = true; @@ -288,10 +314,12 @@ public function contains($element): bool */ public function indexOf($element): Maybe { - $values = $this->values; + $captured = $this->capture(); - return Maybe::defer(static function() use ($values, $element) { + return Maybe::defer(static function() use ($captured, $element) { $index = 0; + /** @var \Iterator */ + $values = self::detonate($captured); foreach ($values as $value) { if ($value === $element) { @@ -314,18 +342,22 @@ public function indexOf($element): Maybe */ public function indices(): Implementation { + $captured = $this->capture(); + /** * @psalm-suppress ImpureFunctionCall * @var Implementation<0|positive-int> */ return new self( - (static function(\Iterator $values): \Generator { + (static function() use ($captured): \Generator { $index = 0; + /** @var \Iterator */ + $values = self::detonate($captured); foreach ($values as $_) { yield $index++; } - })($this->values), + })(), ); } @@ -338,14 +370,18 @@ public function indices(): Implementation */ public function map(callable $function): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $map): \Generator { - /** @var T $value */ + (static function(callable $map) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $map($value); } - })($this->values, $function), + })($function), ); } @@ -360,10 +396,14 @@ public function map(callable $function): Implementation */ public function flatMap(callable $map, callable $exfiltrate): self { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $map, callable $exfiltrate): \Generator { - /** @var T $value */ + (static function(callable $map, callable $exfiltrate) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { /** * @var callable(T): C $map @@ -373,7 +413,7 @@ public function flatMap(callable $map, callable $exfiltrate): self yield $inner; } } - })($this->values, $map, $exfiltrate), + })($map, $exfiltrate), ); } @@ -384,10 +424,14 @@ public function flatMap(callable $map, callable $exfiltrate): self */ public function pad(int $size, $element): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, int $toPad, mixed $element): \Generator { - /** @var T $value */ + (static function(int $toPad, mixed $element) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $value; --$toPad; @@ -397,7 +441,7 @@ public function pad(int $size, $element): Implementation yield $element; --$toPad; } - })($this->values, $size, $element), + })($size, $element), ); } @@ -409,7 +453,7 @@ public function pad(int $size, $element): Implementation public function partition(callable $predicate): Map { /** @var Map> */ - return $this->load()->partition($predicate); + return $this->memoize()->partition($predicate); } /** @@ -417,12 +461,15 @@ public function partition(callable $predicate): Map */ public function slice(int $from, int $until): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, int $from, int $until): \Generator { + (static function(int $from, int $until) use ($captured): \Generator { $index = 0; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { if ($index >= $from && $index < $until) { yield $value; @@ -430,7 +477,7 @@ public function slice(int $from, int $until): Implementation ++$index; } - })($this->values, $from, $until), + })($from, $until), ); } @@ -439,12 +486,15 @@ public function slice(int $from, int $until): Implementation */ public function take(int $size): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, int $size): \Generator { + (static function(int $size) use ($captured): \Generator { $taken = 0; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { if ($taken >= $size) { return; @@ -453,7 +503,7 @@ public function take(int $size): Implementation yield $value; ++$taken; } - })($this->values, $size), + })($size), ); } @@ -466,7 +516,7 @@ public function takeEnd(int $size): Implementation { // this cannot be optimised as the whole generator needs to be loaded // in order to know the elements to drop - return $this->load()->takeEnd($size); + return $this->memoize()->takeEnd($size); } /** @@ -476,10 +526,14 @@ public function takeEnd(int $size): Implementation */ public function append(Implementation $sequence): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, Implementation $sequence): \Generator { - /** @var T $value */ + (static function(Implementation $sequence) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $value; } @@ -488,7 +542,7 @@ public function append(Implementation $sequence): Implementation foreach ($sequence->iterator() as $value) { yield $value; } - })($this->values, $sequence), + })($sequence), ); } @@ -499,19 +553,23 @@ public function append(Implementation $sequence): Implementation */ public function prepend(Implementation $sequence): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, Implementation $sequence): \Generator { + (static function(Implementation $sequence) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + /** @var T $value */ foreach ($sequence->iterator() as $value) { yield $value; } - /** @var T $value */ foreach ($values as $value) { yield $value; } - })($this->values, $sequence), + })($sequence), ); } @@ -535,14 +593,17 @@ public function intersect(Implementation $sequence): Implementation */ public function sort(callable $function): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $function): \Generator { + (static function(callable $function) use ($captured): \Generator { /** @var callable(T, T): int $sorter */ $sorter = $function; $loaded = []; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { $loaded[] = $value; } @@ -552,7 +613,7 @@ public function sort(callable $function): Implementation foreach ($loaded as $value) { yield $value; } - })($this->values, $function), + })($function), ); } @@ -588,12 +649,15 @@ public function clear(): Implementation */ public function reverse(): Implementation { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values): \Generator { + (static function() use ($captured): \Generator { $reversed = []; + /** @var \Iterator */ + $values = self::detonate($captured); - /** @var T $value */ foreach ($values as $value) { \array_unshift($reversed, $value); } @@ -601,7 +665,7 @@ public function reverse(): Implementation foreach ($reversed as $value) { yield $value; } - })($this->values), + })(), ); } @@ -625,14 +689,18 @@ public function toIdentity(): Identity */ public function toSequence(): Sequence { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return Sequence::defer( - (static function(\Iterator $values): \Generator { - /** @var T $value */ + (static function() use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $value; } - })($this->values), + })(), ); } @@ -641,22 +709,29 @@ public function toSequence(): Sequence */ public function toSet(): Set { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return Set::defer( - (static function(\Iterator $values): \Generator { - /** @var T $value */ + (static function() use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { yield $value; } - })($this->values), + })(), ); } public function find(callable $predicate): Maybe { - $values = $this->values; + $captured = $this->capture(); + + return Maybe::defer(static function() use ($captured, $predicate) { + /** @var \Iterator */ + $values = self::detonate($captured); - return Maybe::defer(static function() use ($values, $predicate) { foreach ($values as $value) { /** @psalm-suppress ImpureFunctionCall */ if ($predicate($value) === true) { @@ -689,13 +764,17 @@ public function match(callable $wrap, callable $match, callable $empty) */ public function zip(Implementation $sequence): Implementation { + $captured = $this->capture(); + /** * @psalm-suppress ImpureFunctionCall * @var Implementation */ return new self( - (static function(\Iterator $self, \Iterator $other) { + (static function(\Iterator $other) use ($captured) { /** @var \Iterator $self */ + $self = self::detonate($captured); + foreach ($self as $value) { if (!$other->valid()) { return; @@ -704,7 +783,7 @@ public function zip(Implementation $sequence): Implementation yield [$value, $other->current()]; $other->next(); } - })($this->values, $sequence->iterator()), + })($sequence->iterator()), ); } @@ -717,17 +796,21 @@ public function zip(Implementation $sequence): Implementation */ public function safeguard($carry, callable $assert): self { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, mixed $carry, callable $assert): \Generator { - /** @var T $value */ + (static function(mixed $carry, callable $assert) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { /** @var R */ $carry = $assert($carry, $value); yield $value; } - })($this->values, $carry, $assert), + })($carry, $assert), ); } @@ -752,7 +835,13 @@ public function aggregate(callable $map, callable $exfiltrate): self */ public function memoize(): Implementation { - return $this->load(); + $values = []; + + foreach ($this->values as $value) { + $values[] = $value; + } + + return new Primitive($values); } /** @@ -762,8 +851,13 @@ public function memoize(): Implementation */ public function dropWhile(callable $condition): self { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ - return new self((static function(\Iterator $values, callable $condition) { + return new self((static function(callable $condition) use ($captured) { + /** @var \Iterator */ + $values = self::detonate($captured); + /** @psalm-suppress ImpureMethodCall */ while ($values->valid()) { /** @@ -790,7 +884,7 @@ public function dropWhile(callable $condition): self /** @psalm-suppress ImpureMethodCall */ $values->next(); } - })($this->values, $condition)); + })($condition)); } /** @@ -800,10 +894,14 @@ public function dropWhile(callable $condition): self */ public function takeWhile(callable $condition): self { + $captured = $this->capture(); + /** @psalm-suppress ImpureFunctionCall */ return new self( - (static function(\Iterator $values, callable $condition): \Generator { - /** @var T $value */ + (static function(callable $condition) use ($captured): \Generator { + /** @var \Iterator */ + $values = self::detonate($captured); + foreach ($values as $value) { if (!$condition($value)) { return; @@ -811,21 +909,41 @@ public function takeWhile(callable $condition): self yield $value; } - })($this->values, $condition), + })($condition), ); } /** - * @return Implementation + * @return array{\WeakReference>, \Iterator} */ - private function load(): Implementation + private function capture(): array { - $values = []; + /** @psalm-suppress ImpureMethodCall */ + return [ + \WeakReference::create($this), + match ($this->values->started()) { + true => $this->values, + false => $this->generator, + }, + ]; + } - foreach ($this->values as $value) { - $values[] = $value; + /** + * @template V + * + * @param array{\WeakReference>, \Iterator} $captured + * + * @return \Iterator + */ + private static function detonate(array $captured): \Iterator + { + [$ref, $generator] = $captured; + $self = $ref->get(); + + if (\is_null($self)) { + return $generator; } - return new Primitive($values); + return $self->values; } } diff --git a/src/Sequence/Implementation.php b/src/Sequence/Implementation.php index c46df4ba..54484232 100644 --- a/src/Sequence/Implementation.php +++ b/src/Sequence/Implementation.php @@ -33,7 +33,7 @@ public function __invoke($element): self; public function size(): int; /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator; diff --git a/src/Sequence/Lazy.php b/src/Sequence/Lazy.php index 8b5b11fd..645a5d80 100644 --- a/src/Sequence/Lazy.php +++ b/src/Sequence/Lazy.php @@ -69,7 +69,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { @@ -904,7 +904,7 @@ private function load(): Implementation * * @param Implementation $sequence * - * @return \Iterator + * @return \Iterator */ private static function open( Implementation $sequence, diff --git a/src/Sequence/Primitive.php b/src/Sequence/Primitive.php index 6be04f7b..a0536a76 100644 --- a/src/Sequence/Primitive.php +++ b/src/Sequence/Primitive.php @@ -54,7 +54,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { diff --git a/src/Set/Defer.php b/src/Set/Defer.php index 45f6fd45..10e04b81 100644 --- a/src/Set/Defer.php +++ b/src/Set/Defer.php @@ -63,7 +63,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { diff --git a/src/Set/Implementation.php b/src/Set/Implementation.php index a6d33d74..8ddf3715 100644 --- a/src/Set/Implementation.php +++ b/src/Set/Implementation.php @@ -32,7 +32,7 @@ public function __invoke($element): self; public function size(): int; /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator; diff --git a/src/Set/Lazy.php b/src/Set/Lazy.php index 1f1034b7..4f8793ff 100644 --- a/src/Set/Lazy.php +++ b/src/Set/Lazy.php @@ -64,7 +64,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator { diff --git a/src/Set/Primitive.php b/src/Set/Primitive.php index 9ff2819f..9eebf2aa 100644 --- a/src/Set/Primitive.php +++ b/src/Set/Primitive.php @@ -69,7 +69,7 @@ public function count(): int } /** - * @return \Iterator + * @return \Iterator */ public function iterator(): \Iterator {