fix(codegen): evaluate compound ??= RHS only when the target is not set (#47)
* fix(codegen): evaluate compound ??= RHS only when the target is not set
PHP evaluates the right-hand side of ??= lazily: `$a = 1;
$a ??= sideEffect() + 1;` never calls sideEffect(). When the RHS was a
compound expression the compiler materialized its lowered statements
(the call result temporary) into the enclosing statement context, so
the generated C++ executed the side-effecting call unconditionally
before the isset check.
Generalize the conditional-lambda lowering that already protected
native-object targets: whenever the RHS captured before/after
statements, emit an immediately-invoked lambda whose not-set branch
contains those statements, the assignment and the cleanup. The simple
inline form (`$b ??= f()`) keeps its existing conditional-expression
codegen unchanged.
* fix(codegen): stabilize ??= targets and finish the RHS before assigning
Zend evaluates a coalesce-assignment target's receiver and array keys
exactly once, before the isset check and regardless of its outcome; the
string-based lowering mentioned the target on every use (isset, read,
write, returned value), so a side-effecting receiver ran twice when the
target was set and three times when it was not. Side-effecting target
subexpressions are now materialized into temporaries in source order
(array containers keep their original variable — writing through a
copied temporary would write to the copy — while object receivers are
handles) and the rewritten target reuses them everywhere.
The captured branch also assigned the target before running the RHS's
deferred write-backs, so a postfix increment on the RHS finished after
the outer assignment — observable by a set hook on the target. The RHS
now completes into a temporary (write-backs included) before the target
is written, and the assignment expression itself is returned so the
target is not read again afterwards.
* fix(codegen): stabilize every ??= target subexpression, bound temporary lifetimes
Three target-stabilization gaps in the coalesce-assignment lowering:
- A value-producing array container (makeArray()[keyName()] ??= 42) was
left unstabilized: the dimension was materialized first, reversing
PHP's container-then-key source order, and the container ran once for
the isset check and again for the write. Non-variable containers are
now materialized in source order; plain-variable containers keep
write-through semantics, and a value-producing container is itself
the temporary PHP writes into.
- Dynamic property names were re-evaluated on every mention:
$box->{propertyName()} ran the name expression twice per branch, and
StaticPropertyFetch was not handled at all. Dynamic instance names,
static class expressions and static property names are now
materialized once, in PHP evaluation order (receiver, name, then
dimension), recursively through chained targets.
- The materialized temporaries were function-scoped, deferring the
receiver's destructor to function exit where PHP destroys it at the
end of the statement. Temporaries now follow the established lifetime
idiom (stabilizeAssignOpPropertyReceiver): zval-owning Variants are
.unset() at statement end and Native pointer temporaries reset to
nullptr.
master
Alessio Giacobbe
11 hours ago
committed by
GitHub
parent
0c69b8b357
commit
20f0b5a284
8 changed files with 359 additions and 3 deletions
@ -0,0 +1,21 @@ |
||||
<?php |
||||
|
||||
function sideEffectCall(): int |
||||
{ |
||||
echo "side effect!\n"; |
||||
return 41; |
||||
} |
||||
|
||||
function coalesceCompoundRhs(): int |
||||
{ |
||||
$target = 1; |
||||
$target ??= sideEffectCall() + 1; |
||||
return $target; |
||||
} |
||||
|
||||
function coalesceSimpleRhs(): int |
||||
{ |
||||
$target = 1; |
||||
$target ??= sideEffectCall(); |
||||
return $target; |
||||
} |
||||
@ -0,0 +1,64 @@ |
||||
<?php |
||||
|
||||
use TypePhp\CompilerTest; |
||||
|
||||
/** |
||||
* PHP evaluates the RHS of ??= only when the target is not set. When the RHS |
||||
* is a compound expression, its lowered statements (the side-effecting call) |
||||
* must be emitted inside the not-set branch, never unconditionally before |
||||
* the isset check. |
||||
*/ |
||||
final class CoalesceAssignSideEffectCodegenTest extends \BaseTest |
||||
{ |
||||
public function testCompoundRhsCallIsEmittedOnlyInsideNotSetBranch(): void |
||||
{ |
||||
$code = $this->compileFixture(); |
||||
|
||||
$body = $this->extractFunctionBody($code, 'php::Int php_coalescecompoundrhs()'); |
||||
|
||||
// The call must appear after the early-return isset guard of the |
||||
// conditional lambda, not as a plain statement before it. |
||||
$callPos = strpos($body, 'php_sideeffectcall()'); |
||||
self::assertIsInt($callPos); |
||||
$guardPos = strpos($body, 'if (php::exists(target)) { return target; }'); |
||||
self::assertIsInt($guardPos, 'expected the isset guard inside a conditional lambda'); |
||||
self::assertGreaterThan($guardPos, $callPos, 'RHS call must be inside the not-set branch'); |
||||
} |
||||
|
||||
public function testSimpleRhsKeepsPlainConditionalExpression(): void |
||||
{ |
||||
$code = $this->compileFixture(); |
||||
|
||||
$body = $this->extractFunctionBody($code, 'php::Int php_coalescesimplerhs()'); |
||||
|
||||
self::assertStringContainsString( |
||||
'(php::exists(target)?target:(target = php_sideeffectcall()))', |
||||
$body, |
||||
); |
||||
} |
||||
|
||||
private function extractFunctionBody(string $code, string $signature): string |
||||
{ |
||||
$start = strpos($code, $signature); |
||||
self::assertIsInt($start, "missing function: {$signature}"); |
||||
$end = strpos($code, "\n}", $start); |
||||
self::assertIsInt($end); |
||||
return substr($code, $start, $end - $start); |
||||
} |
||||
|
||||
private function compileFixture(): string |
||||
{ |
||||
global $translator; |
||||
|
||||
$compiler = CompilerTest::create(TYPEPHP_ROOT_PATH); |
||||
$translator = $compiler; |
||||
$source = TYPEPHP_ROOT_PATH . '/phpunit/code/coalesce-assign-side-effect-codegen.php'; |
||||
$compiler->addFiles([$source]); |
||||
$compiler->prepareFile($source); |
||||
$generated = $compiler->convertFile($source); |
||||
$code = file_get_contents($generated); |
||||
|
||||
self::assertIsString($code); |
||||
return $code; |
||||
} |
||||
} |
||||
@ -0,0 +1,22 @@ |
||||
--TEST-- |
||||
??= evaluates a side-effecting array key exactly once, on both branches |
||||
--FILE-- |
||||
<?php |
||||
function arrayKey(): string { echo "KEY\n"; return "k"; } |
||||
function sideEffect(): int { echo "SIDE\n"; return 41; } |
||||
|
||||
function main(): void |
||||
{ |
||||
$arr = []; |
||||
$arr[arrayKey()] ??= sideEffect() + 1; |
||||
var_dump($arr["k"]); |
||||
$arr[arrayKey()] ??= sideEffect() + 1; |
||||
var_dump($arr["k"]); |
||||
} |
||||
?> |
||||
--EXPECT-- |
||||
KEY |
||||
SIDE |
||||
int(42) |
||||
KEY |
||||
int(42) |
||||
@ -0,0 +1,32 @@ |
||||
--TEST-- |
||||
??= does not evaluate a compound side-effecting RHS when the target is set |
||||
--FILE-- |
||||
<?php |
||||
declare(strict_types=1); |
||||
|
||||
function sideEffect(): int |
||||
{ |
||||
echo "side effect!\n"; |
||||
return 41; |
||||
} |
||||
|
||||
function main(): void |
||||
{ |
||||
$a = 1; |
||||
$a ??= sideEffect() + 1; |
||||
var_dump($a); |
||||
|
||||
$b = null; |
||||
$b ??= sideEffect() + 1; |
||||
var_dump($b); |
||||
|
||||
$c = 'set'; |
||||
$c ??= sideEffect() . '-suffix'; |
||||
var_dump($c); |
||||
} |
||||
?> |
||||
--EXPECT-- |
||||
int(1) |
||||
side effect! |
||||
int(42) |
||||
string(3) "set" |
||||
@ -0,0 +1,29 @@ |
||||
--TEST-- |
||||
??= completes the RHS postfix write-back before the target assignment |
||||
--FILE-- |
||||
<?php |
||||
class State { public static mixed $assigned = null; } |
||||
|
||||
class Source { |
||||
private int $stored = 5; |
||||
public int $value { |
||||
get { return $this->stored; } |
||||
set { |
||||
var_dump(State::$assigned); |
||||
$this->stored = $value; |
||||
} |
||||
} |
||||
} |
||||
|
||||
function main(): void |
||||
{ |
||||
$source = new Source(); |
||||
State::$assigned ??= $source->value++; |
||||
var_dump(State::$assigned); |
||||
var_dump($source->value); |
||||
} |
||||
?> |
||||
--EXPECT-- |
||||
NULL |
||||
int(5) |
||||
int(6) |
||||
@ -0,0 +1,24 @@ |
||||
--TEST-- |
||||
??= evaluates a side-effecting property receiver exactly once, on both branches |
||||
--FILE-- |
||||
<?php |
||||
class Box { public mixed $value = null; } |
||||
|
||||
function receiver(object $b): object { echo "RECV\n"; return $b; } |
||||
function sideEffect(): int { echo "SIDE\n"; return 41; } |
||||
|
||||
function main(): void |
||||
{ |
||||
$box = new Box(); |
||||
receiver($box)->value ??= sideEffect() + 1; |
||||
var_dump($box->value); |
||||
receiver($box)->value ??= sideEffect() + 1; |
||||
var_dump($box->value); |
||||
} |
||||
?> |
||||
--EXPECT-- |
||||
RECV |
||||
SIDE |
||||
int(42) |
||||
RECV |
||||
int(42) |
||||
@ -0,0 +1,52 @@ |
||||
--TEST-- |
||||
??= stabilizes value containers, dynamic property names, and temporary lifetimes |
||||
--FILE-- |
||||
<?php |
||||
function makeArray(): array { echo "ARRAY\n"; return []; } |
||||
function keyName(): string { echo "KEY\n"; return 'value'; } |
||||
|
||||
class Box { |
||||
public mixed $value = null; |
||||
public static mixed $slot = null; |
||||
public function __destruct() { echo "DESTRUCT\n"; } |
||||
} |
||||
|
||||
function makeBox(): object { echo "MAKE\n"; return new Box(); } |
||||
function propertyName(): string { echo "NAME\n"; return 'value'; } |
||||
function slotName(): string { echo "SLOT\n"; return 'slot'; } |
||||
function rhs(): int { echo "RHS\n"; return 42; } |
||||
|
||||
function main(): void |
||||
{ |
||||
// A value-producing container is evaluated once, before the key. |
||||
var_dump(makeArray()[keyName()] ??= 42); |
||||
// The materialized receiver dies at the end of the statement. |
||||
makeBox()->value ??= 42; |
||||
echo "AFTER\n"; |
||||
// A dynamic instance property name is evaluated once, on both branches. |
||||
$box = new Box(); |
||||
$box->{propertyName()} ??= rhs(); |
||||
var_dump($box->value); |
||||
$box->{propertyName()} ??= rhs(); |
||||
var_dump($box->value); |
||||
// A dynamic static property name is evaluated once. |
||||
Box::${slotName()} ??= rhs(); |
||||
var_dump(Box::$slot); |
||||
} |
||||
?> |
||||
--EXPECT-- |
||||
ARRAY |
||||
KEY |
||||
int(42) |
||||
MAKE |
||||
DESTRUCT |
||||
AFTER |
||||
NAME |
||||
RHS |
||||
int(42) |
||||
NAME |
||||
int(42) |
||||
SLOT |
||||
RHS |
||||
int(42) |
||||
DESTRUCT |
||||
Loading…
Reference in new issue