From ec251655baab9fe5f315de86e83a4312274b706f Mon Sep 17 00:00:00 2001 From: hafung <32428762+hafung@users.noreply.github.com> Date: Thu, 27 Aug 2026 10:38:48 +0800 Subject: [PATCH 1/2] Fix repeated array offset evaluation in compound assignments --- src/Parser/AssignOpTrait.php | 39 ++++++++++++++---- .../array/compound-offset-evaluated-once.phpt | 41 +++++++++++++++++++ 2 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 tests/compiler/array/compound-offset-evaluated-once.phpt diff --git a/src/Parser/AssignOpTrait.php b/src/Parser/AssignOpTrait.php index 4305914a..faeee423 100644 --- a/src/Parser/AssignOpTrait.php +++ b/src/Parser/AssignOpTrait.php @@ -823,6 +823,26 @@ trait AssignOpTrait protected function parseAssignOp(Expr\AssignOp $node, string $op): string { + // Every analysis and lowering phase must see the same array key. Some + // of those helpers parse dynamic GLOBALS keys while determining the + // target type, so stabilizing only the final read and write is too + // late for side-effecting offsets such as $i++. + if ($node->var instanceof Expr\ArrayDimFetch + && $node->var->dim !== null + && $this->shouldMaterializeOrderedOperand($node->var->dim) + ) { + $originalAccess = $node->var; + $dim = $this->addTmpVar(Type::VAR); + $this->context->beforeStmtLines[] = $dim . ' = ' + . $this->parseOrderedOperand($originalAccess->dim, false) . ';'; + $node = clone $node; + $node->var = new Expr\ArrayDimFetch( + $originalAccess->var, + new Variable($dim, $originalAccess->dim->getAttributes()), + $originalAccess->getAttributes(), + ); + } + $this->assertImmutableMutationTarget($node->var); $this->assertNativeArrayAccessDirectWrite($node->var, false); $this->assertNativeObjectOperatorOperandSupported($node->var, $node, $op); @@ -869,7 +889,8 @@ trait AssignOpTrait return $nativePropertyAssignOp; } - $var = $this->parseWritableIdentifier($node->var); + $arrayDimFetch = $this->isArrayDimFetch($node->var); + $var = $arrayDimFetch ? '' : $this->parseWritableIdentifier($node->var); $expr = $this->isAssignOpConcat($op) ? '' : (string) $this->parseIdentifier($node->expr); if ($this->isVarExpr($node->var)) { @@ -906,7 +927,7 @@ trait AssignOpTrait return $var . ' ' . $op . ' ' . $rightExprStr; } - if ($this->isArrayDimFetch($node->var)) { + if ($arrayDimFetch) { if ($this->isStdContainerExpr($node->var)) { return $this->parseStdContainerAssignOp($node, $op); } @@ -926,7 +947,8 @@ trait AssignOpTrait * $tmp_var = $count[$r] - 1; * $count[$r] = $tmp_var;. */ - $type = $this->detectVarType($node->var); + $isGlobals = $this->isVarExpr($node->var->var) && $node->var->var->name === 'GLOBALS'; + $type = $isGlobals ? Type::VAR : $this->detectVarType($node->var); $rightType = $this->detectTypeOfExpr($node->expr); $tmpVar = $this->genTmpVarName(); // PHP arrays are dynamically typed even when SSA can currently @@ -934,8 +956,9 @@ trait AssignOpTrait // Zend arithmetic promotes overflowing integers to float instead // of evaluating a signed C++ expression with undefined behavior. $this->addLocalVar($tmpVar, Type::VAR); - $dim = $this->parseIdentifier($node->var->dim); - $readVar = $this->parseArrayDimFetchRead($node->var); + $stableAccess = $node->var; + $dim = $this->parseIdentifier($node->var->dim); + $readVar = $this->parseArrayDimFetchRead($stableAccess); $binaryOp = $this->removeAssignOp($op); if ($binaryOp === '.') { @@ -953,10 +976,10 @@ trait AssignOpTrait $this->convertExprType($expr, $type, $rightType) . ';'; } - if ($this->isVarExpr($node->var->var) && $node->var->var->name === 'GLOBALS') { - return $var . ' = ' . $tmpVar; + if ($isGlobals) { + return $this->parseGlobalsArrayDimFetch($stableAccess) . ' = ' . $tmpVar; } - return '(' . $this->parseArrayDimStore($node->var->var, $dim, $tmpVar) . ', ' . $tmpVar . ')'; + return '(' . $this->parseArrayDimStore($stableAccess->var, $dim, $tmpVar) . ', ' . $tmpVar . ')'; } if ($this->isPropertyFetch($node->var) and !$this->isNativePropertyAccess($node->var)) { diff --git a/tests/compiler/array/compound-offset-evaluated-once.phpt b/tests/compiler/array/compound-offset-evaluated-once.phpt new file mode 100644 index 00000000..003bbbbe --- /dev/null +++ b/tests/compiler/array/compound-offset-evaluated-once.phpt @@ -0,0 +1,41 @@ +--TEST-- +Array compound assignment evaluates a side-effecting offset once +--FILE-- + +--EXPECT-- +int(1) +array(1) { + [0]=> + int(15) +} +int(1) +array(1) { + [0]=> + int(40) +} +int(40) +int(1) +int(7) From 8918fae3ba251c02773766204e176b6f1ca60d70 Mon Sep 17 00:00:00 2001 From: tianfenghan Date: Thu, 27 Aug 2026 16:53:36 +0800 Subject: [PATCH 2/2] fix(parser): stabilize compound array access paths --- src/Parser/AssignOpTrait.php | 52 +++++++++++++------ src/Parser/BinaryOpTrait.php | 4 +- .../array/compound-offset-evaluated-once.phpt | 51 ++++++++++++++++++ .../compound-offset-evaluated-once.phpt | 21 ++++++++ 4 files changed, 111 insertions(+), 17 deletions(-) create mode 100644 tests/compiler/std-vector/compound-offset-evaluated-once.phpt diff --git a/src/Parser/AssignOpTrait.php b/src/Parser/AssignOpTrait.php index faeee423..6e161de1 100644 --- a/src/Parser/AssignOpTrait.php +++ b/src/Parser/AssignOpTrait.php @@ -823,24 +823,11 @@ trait AssignOpTrait protected function parseAssignOp(Expr\AssignOp $node, string $op): string { - // Every analysis and lowering phase must see the same array key. Some - // of those helpers parse dynamic GLOBALS keys while determining the - // target type, so stabilizing only the final read and write is too - // late for side-effecting offsets such as $i++. if ($node->var instanceof Expr\ArrayDimFetch - && $node->var->dim !== null - && $this->shouldMaterializeOrderedOperand($node->var->dim) + && !$this->canUpdateKnownArraySlotInPlace($node, $op) ) { - $originalAccess = $node->var; - $dim = $this->addTmpVar(Type::VAR); - $this->context->beforeStmtLines[] = $dim . ' = ' - . $this->parseOrderedOperand($originalAccess->dim, false) . ';'; $node = clone $node; - $node->var = new Expr\ArrayDimFetch( - $originalAccess->var, - new Variable($dim, $originalAccess->dim->getAttributes()), - $originalAccess->getAttributes(), - ); + $node->var = $this->stabilizeAssignOpArrayAccess($node->var); } $this->assertImmutableMutationTarget($node->var); @@ -1013,6 +1000,41 @@ trait AssignOpTrait return $var . ' ' . $op . ' (' . $expr . ')'; } + /** + * Evaluate every dynamic dimension of a compound-assignment target once, + * from the innermost access to the outermost access, before evaluating the + * right-hand side. Both the read and write lowering then reuse the cloned + * access path and cannot observe keys changed by a later dimension or RHS. + */ + private function stabilizeAssignOpArrayAccess(Expr\ArrayDimFetch $access): Expr\ArrayDimFetch + { + $dimensions = []; + $base = $access; + while ($base instanceof Expr\ArrayDimFetch) { + $dimensions[] = $base; + $base = $base->var; + } + + $stableAccess = $base; + foreach (array_reverse($dimensions) as $dimension) { + $dim = $dimension->dim; + $literal = $dim instanceof Node\Scalar\LNumber + || $dim instanceof Node\Scalar\DNumber + || $dim instanceof Node\Scalar\String_; + if ($dim !== null && !$literal) { + $tmp = $this->parseOrderedOperand($dim, false, true); + $dim = new Variable($tmp, $dim->getAttributes()); + } + $stableAccess = new Expr\ArrayDimFetch( + $stableAccess, + $dim, + $dimension->getAttributes(), + ); + } + + return $stableAccess; + } + /** * Preserve PHP's concat-assignment operation for statically typed strings. * String::append() calls concat_function() with the target as both the diff --git a/src/Parser/BinaryOpTrait.php b/src/Parser/BinaryOpTrait.php index f186acae..30b05b57 100644 --- a/src/Parser/BinaryOpTrait.php +++ b/src/Parser/BinaryOpTrait.php @@ -521,10 +521,10 @@ trait BinaryOpTrait return $this->parseOrderedOperand($expr, true); } - protected function parseOrderedOperand(NodeAbstract $expr, bool $numeric): string + protected function parseOrderedOperand(NodeAbstract $expr, bool $numeric, bool $forceMaterialize = false): string { $this->assertExprCanBeUsedAsValue($expr, 'operand'); - if (!$this->shouldMaterializeOrderedOperand($expr)) { + if (!$forceMaterialize && !$this->shouldMaterializeOrderedOperand($expr)) { $value = $numeric ? $this->parseNumericIdentifier($expr) : $this->parseIdentifier($expr); return $this->normalizeNativeObjectValueExpr($expr, $value); } diff --git a/tests/compiler/array/compound-offset-evaluated-once.phpt b/tests/compiler/array/compound-offset-evaluated-once.phpt index 003bbbbe..d2039286 100644 --- a/tests/compiler/array/compound-offset-evaluated-once.phpt +++ b/tests/compiler/array/compound-offset-evaluated-once.phpt @@ -4,6 +4,17 @@ Array compound assignment evaluates a side-effecting offset once 10, 'b0' => 20]; + $interpolatedResult = ($interpolatedValues["{$offsetPart}0"] += changeCompoundOffsetPart($offsetPart)); + + var_dump($offsetPart, $interpolatedValues['a0'], $interpolatedValues['b0'], $interpolatedResult); } ?> --EXPECT-- @@ -39,3 +74,19 @@ array(1) { int(40) int(1) int(7) +int(2) +int(25) +int(30) +int(25) +int(2) +int(40) +int(30) +int(40) +int(1) +int(11) +int(20) +int(11) +string(1) "b" +int(15) +int(20) +int(15) diff --git a/tests/compiler/std-vector/compound-offset-evaluated-once.phpt b/tests/compiler/std-vector/compound-offset-evaluated-once.phpt new file mode 100644 index 00000000..e518490c --- /dev/null +++ b/tests/compiler/std-vector/compound-offset-evaluated-once.phpt @@ -0,0 +1,21 @@ +--TEST-- +std vector compound assignment evaluates its offset before the RHS +--FILE-- + +--EXPECT-- +int(1) +int(11) +int(20) +int(11)