Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions phpunit/code/closure-repeated-coalesce-codegen.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

function repeatedCoalesceClosure(string $text): string
{
return preg_replace_callback('/(X)|(q)(.+?)\2/', function ($m) {
if (($m[1] ?? '') !== '') {
return '[' . $m[1] . ']';
}
return '<' . $m[2] . $m[3] . '>';
}, $text) ?? $text;
}
47 changes: 47 additions & 0 deletions phpunit/src/ClosureRepeatedCoalesceCodegenTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

use TypePhp\CompilerTest;

/**
* An outer `??` re-parses its left operand (the preg_replace_callback call),
* so the callback closure used to be emitted several times. Only the last
* emission is wired into newClosureWithParameters(); the earlier generations
* were discarded, and with them the materialized `tmp = php::exists(...)`
* statement of the inner `??` guard, leaving the live copy reading an
* uninitialized temporary (guard always true). The closure must be emitted
* exactly once and its `??` guard must keep the php::exists() materialization.
*/
final class ClosureRepeatedCoalesceCodegenTest extends \BaseTest
{
public function testEveryClosureGenerationMaterializesCoalesceGuard(): void
{
$code = $this->compileFixture();

// An outer ?? re-parses the callback call three times: the eager left
// operand parse, checkVarMustExist()'s warm-up, and the chain walker.
$generations = substr_count($code, 'php::ClosureFn');
self::assertSame(3, $generations);

self::assertSame(
$generations,
substr_count($code, 'php::exists(m,'),
'every generation of the closure must materialize its ?? guard; a later one reusing a stale replace attribute reads an uninitialized temporary and inverts the guard',
);
}

private function compileFixture(): string
{
global $translator;

$compiler = CompilerTest::create(TYPEPHP_ROOT_PATH);
$translator = $compiler;
$source = TYPEPHP_ROOT_PATH . '/phpunit/code/closure-repeated-coalesce-codegen.php';
$compiler->addFiles([$source]);
$compiler->prepareFile($source);
$generated = $compiler->convertFile($source);
$code = file_get_contents($generated);

self::assertIsString($code);
return $code;
}
}
26 changes: 26 additions & 0 deletions src/Generator/ClosureGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ protected function genClosure(Expr\ArrowFunction|Expr\Closure $expr, array $para

private function doGenClosure(Expr\ArrowFunction|Expr\Closure $expr, array $params, array $uses = []): string
{
$this->clearReplaceAttributes($expr);
// Closure signatures flow through the same declaration validation in
// parseTypeDecl() as named functions (e.g. callable inside an
// intersection or DNF member). Bare class names are skipped here: the
Expand Down Expand Up @@ -639,6 +640,31 @@ private function doGenClosure(Expr\ArrowFunction|Expr\Closure $expr, array $para
);
}

/**
* Drop parseValueSelection()'s "replace" shortcuts from a closure's subtree.
*
* parseExpr() honours a node's replace attribute as a same-context
* deduplication: the second parse of an expression returns the temporary
* its first parse lowered into instead of emitting the materialization
* twice. A closure body, however, is regenerated into a fresh lambda
* whenever the expression holding the closure is parsed again (for
* example parseValueSelection lowering its left operand), while the
* temporaries recorded by the previous generation live in the previous,
* discarded lambda body. Honouring them would read an uninitialized
* temporary instead of re-emitting the materialization, so each
* generation rebuilds those statements.
*/
protected function clearReplaceAttributes(NodeAbstract $expr): void
{
$nodes = (new NodeFinder())->find(
$expr,
static fn (Node $node): bool => $node->hasAttribute('replace'),
);
foreach ($nodes as $node) {
$node->setAttributes(array_diff_key($node->getAttributes(), ['replace' => true]));
}
}

protected function closureContainsYield(Expr\ArrowFunction|Expr\Closure $expr): bool
{
if ($expr instanceof Expr\ArrowFunction) {
Expand Down
22 changes: 22 additions & 0 deletions tests/compiler/coalesce/closure-repeated-parse.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
Closure ?? guard stays correct when an outer ?? re-parses the callback call
--FILE--
<?php

function renderRepeatedCoalesce(string $text): string
{
return preg_replace_callback('/(X)|(q)(.+?)\2/', function ($m) {
if (($m[1] ?? '') !== '') {
return '[' . $m[1] . ']';
}
return '<' . $m[2] . $m[3] . '>';
}, $text) ?? $text;
}

function main()
{
var_dump(renderRepeatedCoalesce('X q-mid-q Z'));
}
?>
--EXPECT--
string(14) "[X] <q-mid-> Z"
Loading