From af466b067c5fdf45275949df3305649c85c22d93 Mon Sep 17 00:00:00 2001 From: tianfenghan Date: Tue, 1 Sep 2026 14:59:15 +0800 Subject: [PATCH] fix(parser): handle compound assignment operations on typed properties - Stabilize property receivers for compound assignments to ensure single evaluation - Implement checked int property compound assignments with proper PHP arithmetic - Add materialization of object receivers for property compound assignments - Support native types handling for compound assignment operations - Fix sequence handling for dynamic property fetch writes in compound assignments - Update tests to verify correct behavior of int property compound assignments - Modify native property assignment operations to use direct references - Add comprehensive test coverage for typed property compound assignment scenarios --- .../native-property-assign-op-class-const.php | 2 + .../code/native-property-assign-op-int.php | 2 + phpunit/src/NativePropertyTest.php | 15 +- src/Parser/AssignOpTrait.php | 134 +++++++++++++++++- .../int-property-compound-assignment.phpt | 116 +++++++++++++++ .../native-int-property-assign-op-var.phpt | 13 +- ...ypes-int-property-compound-assignment.phpt | 46 ++++++ 7 files changed, 310 insertions(+), 18 deletions(-) create mode 100644 tests/compiler/object_property/int-property-compound-assignment.phpt create mode 100644 tests/compiler/object_property/native-types-int-property-compound-assignment.phpt diff --git a/phpunit/code/native-property-assign-op-class-const.php b/phpunit/code/native-property-assign-op-class-const.php index c28ee39d..9676a0ac 100644 --- a/phpunit/code/native-property-assign-op-class-const.php +++ b/phpunit/code/native-property-assign-op-class-const.php @@ -1,5 +1,7 @@ assertStringContainsString('typephp_static_int_ref(this_.attr(', $code); - $this->assertStringContainsString('typephp_static_int_ref(box.attr(', $code); - $this->assertSame(2, substr_count($code, 'typephp_static_int_ref(')); - $this->assertStringNotContainsString('this_.attr(get_persistent_prop(0, get_str(0), 0, get_str(1)), true) +=', $code); - $this->assertStringNotContainsString('box.attr(get_persistent_prop(0, get_str(0), 0, get_str(1)), true) +=', $code); + $this->assertStringContainsString('php::Int &_object_prop_this___value = Z_LVAL_P(this_.attr(', $code); + $this->assertStringContainsString('php::Int &_object_prop_box__value = Z_LVAL_P(box.attr(', $code); + $this->assertStringContainsString('_object_prop_this___value += (2L);', $code); + $this->assertStringContainsString('_object_prop_box__value += (2L);', $code); + $this->assertSame(2, substr_count($code, 'Z_LVAL_P(')); + $this->assertStringNotContainsString('typephp_write_property_scoped(', $code); } public function testReadonlyPropertiesDoNotUseNativeScalarReferences(): void @@ -85,10 +86,10 @@ class NativePropertyTest extends \BaseTest } $code = file_get_contents($outputFile); - $this->assertStringContainsString('typephp_static_int_ref(this_.attr(', $code); + $this->assertStringContainsString('php::Int &_object_prop_this___flags = Z_LVAL_P(this_.attr(', $code); // A TypePHP class constant is available during conversion and is // folded before the native property operation is emitted. - $this->assertStringContainsString('&= (~php::toInt(1L));', $code); + $this->assertStringContainsString('_object_prop_this___flags &= (~php::toInt(1L));', $code); } public function testNativePropertyWriteConvertsOnlyWhenTypesDiffer(): void diff --git a/src/Parser/AssignOpTrait.php b/src/Parser/AssignOpTrait.php index 470490d1..16d1652d 100644 --- a/src/Parser/AssignOpTrait.php +++ b/src/Parser/AssignOpTrait.php @@ -844,6 +844,14 @@ trait AssignOpTrait $node->var = $this->stabilizeAssignOpArrayAccess($node->var); } + if ($node->var instanceof Expr\PropertyFetch && !$this->isVarExpr($node->var->var)) { + $stableProperty = $this->stabilizeAssignOpPropertyReceiver($node->var); + if ($stableProperty !== $node->var) { + $node = clone $node; + $node->var = $stableProperty; + } + } + $this->assertImmutableMutationTarget($node->var); $this->assertNativeArrayAccessDirectWrite($node->var, false); $this->assertNativeObjectOperatorOperandSupported($node->var, $node, $op); @@ -1001,8 +1009,14 @@ trait AssignOpTrait } else { $this->context->beforeStmtLines[] = "{$tmpVar} = {$readProperty} {$binaryOp} ({$expr});"; } - $this->context->afterStmtLines[] = $this->emitDynamicPropertyFetchWrite($node->var, $tmpVar, $propertyWriteTarget) . ';'; - return $tmpVar; + // The write is part of the compound-assignment expression. Apart + // from matching PHP's sequencing, this ensures a materialized + // receiver remains alive until the write has completed. + return '((' . $this->emitDynamicPropertyFetchWrite( + $node->var, + $tmpVar, + $propertyWriteTarget, + ) . '), ' . $tmpVar . ')'; } if ($this->isAssignOpConcat($op)) { @@ -1049,6 +1063,41 @@ trait AssignOpTrait return $stableAccess; } + /** + * Materialize a statically known object receiver used by a property + * compound assignment. The read and write phases may be emitted by + * different helpers, but PHP evaluates the receiver only once. + */ + private function stabilizeAssignOpPropertyReceiver(Expr\PropertyFetch $property): Expr\PropertyFetch + { + $receiverClass = $this->detectClassOfExpr($property->var); + if ($receiverClass === '') { + return $property; + } + + [$receiverExpr, $beforeStmts, $afterStmts] = $this->parseExprWithCapturedStmts($property->var); + $this->appendCapturedStmtLinesToContext($beforeStmts); + + if ($this->isNativeObjectClass($receiverClass)) { + $tmp = $this->genTmpVarName(); + $this->addLocalVar($tmp, $this->getNativeObjectPointerType($receiverClass)); + $this->addNativeObject($tmp, $receiverClass); + $cleanup = $tmp . ' = nullptr;'; + } else { + $tmp = $this->addTmpVar(Type::VAR); + $this->addObject($tmp, $receiverClass); + $cleanup = $tmp . '.unset();'; + } + + $this->context->beforeStmtLines[] = $tmp . ' = ' . $receiverExpr . ';'; + $this->appendCapturedStmtLinesToContext($afterStmts); + $this->context->afterStmtLines[] = $cleanup; + + $stableProperty = clone $property; + $stableProperty->var = new Variable($tmp, $property->var->getAttributes()); + return $stableProperty; + } + /** * Preserve PHP's concat-assignment operation for statically typed strings. * String::append() calls concat_function() with the target as both the @@ -1086,6 +1135,22 @@ trait AssignOpTrait } $rightType = $this->detectTypeOfExpr($node->expr); + + // In ordinary PHP mode, an int compound assignment must perform the + // arithmetic before the typed-property write is validated. The result + // may therefore be a float (division or integer overflow), in which + // case Zend rejects the write and leaves the old property value intact. + // A direct zend_long reference would bypass that behavior completely. + // Native objects cannot cross the Variant boundary and retain their + // native C++ property access path. + if (!$this->nativeTypes + && $def->type === Type::INT + && !$this->isNativeObjectClass($this->detectClassOfExpr($node->var->var)) + && in_array($op, ['+=', '-=', '*=', '/=', '%=', '**=', '<<=', '>>=', '&=', '|=', '^='], true) + ) { + return $this->parseCheckedIntPropertyAssignOp($node, $op); + } + if (in_array($def->type, [Type::BIGINT, Type::DECIMAL, Type::BIGFLOAT], true)) { $binaryOp = $this->removeAssignOp($op); $leftExpr = $this->parseWritableIdentifier($node->var); @@ -1146,12 +1211,75 @@ trait AssignOpTrait } return match ($propertyType) { - Type::INT => in_array($op, ['+=', '-=', '*=', '%=', '<<=', '>>=', '&=', '|=', '^='], true), + Type::INT => in_array($op, ['+=', '-=', '*=', '/=', '%=', '<<=', '>>=', '&=', '|=', '^='], true), Type::FLOAT => in_array($op, ['+=', '-=', '*=', '/='], true), default => false, }; } + /** + * Apply PHP arithmetic first, then let Zend validate the int property + * write. A complex receiver is materialized before the property read, so + * both the read and write refer to the same object and the receiver has + * exactly one observable evaluation. + */ + private function parseCheckedIntPropertyAssignOp(Expr\AssignOp $node, string $op): string + { + /** @var Expr\PropertyFetch $property */ + $property = $node->var; + $receiver = $property->var; + $receiverTmp = null; + + if ($this->isVarExpr($receiver)) { + $objectExpr = $this->parseIdentifier($receiver); + } else { + [$receiverExpr, $receiverBefore, $receiverAfter] = $this->parseExprWithCapturedStmts($receiver); + $this->appendCapturedStmtLinesToContext($receiverBefore); + + // Keep the object in a Variant: this path intentionally uses Zend + // object handlers, and a Variant also covers dynamically resolved + // receivers whose concrete class is known only at runtime. + $receiverTmp = $this->addTmpVar(Type::VAR); + $this->context->beforeStmtLines[] = $receiverTmp . ' = ' . $receiverExpr . ';'; + $this->appendCapturedStmtLinesToContext($receiverAfter); + $objectExpr = $receiverTmp; + } + + $target = new PropertyWriteTarget( + $property, + 'object property', + $objectExpr, + $this->propertyNameToStr($property->name, literal: true), + ); + + // Read before evaluating the RHS, matching PHP compound-assignment + // order even when the RHS itself emits prerequisite statements. + $current = $this->addTmpVar(Type::VAR); + $this->context->beforeStmtLines[] = $current . ' = ' + . $this->emitDynamicPropertyTargetRead($target) . ';'; + + [$rightExpr, $rightBefore, $rightAfter] = $this->parseExprWithCapturedStmts($node->expr); + $this->appendCapturedStmtLinesToContext($rightBefore); + + $result = $this->addTmpVar(Type::VAR); + $binaryOp = $this->removeAssignOp($op); + $value = $binaryOp === '**' + ? 'php::fn::pow(' . $current . ', ' . $rightExpr . ')' + : $current . ' ' . $binaryOp . ' (' . $rightExpr . ')'; + $this->context->beforeStmtLines[] = $result . ' = ' . $value . ';'; + $this->appendCapturedStmtLinesToContext($rightAfter); + + $this->context->afterStmtLines[] = $current . '.unset();'; + if ($receiverTmp !== null) { + $this->context->afterStmtLines[] = $receiverTmp . '.unset();'; + } + + // Execute the checked write at the expression point. If Zend rejects + // the result, the exception is raised before this expression yields and + // the original property zval remains unchanged. + return '((' . $this->emitDynamicPropertyTargetWrite($target, $result) . '), ' . $result . ')'; + } + protected function parseBigAssignOp(Expr\AssignOp $node, string $var, string $type, string $expr, string $rightType, string $op): string { $binaryOp = $this->removeAssignOp($op); diff --git a/tests/compiler/object_property/int-property-compound-assignment.phpt b/tests/compiler/object_property/int-property-compound-assignment.phpt new file mode 100644 index 00000000..5a84c88a --- /dev/null +++ b/tests/compiler/object_property/int-property-compound-assignment.phpt @@ -0,0 +1,116 @@ +--TEST-- +Typed int property compound assignments use PHP arithmetic and checked writes +--FILE-- +value = 8; + $result = $box->value /= 2; + var_dump($result, $box->value); + + $box->value = 8; + try { + $box->value /= 3; + } catch (TypeError $e) { + echo "fraction: ", $e::class, "\n"; + } + var_dump($box->value); + + $box->value = PHP_INT_MAX; + try { + $box->value += 1; + } catch (TypeError $e) { + echo "add overflow: ", $e::class, "\n"; + } + var_dump($box->value === PHP_INT_MAX); + + $box->value = PHP_INT_MAX; + try { + $box->value *= 2; + } catch (TypeError $e) { + echo "mul overflow: ", $e::class, "\n"; + } + var_dump($box->value === PHP_INT_MAX); + + $receiverCalls = 0; + $operandCalls = 0; + $box->value = 5; + $result = intCompoundReceiver($box, $receiverCalls)->value += intCompoundOperand(4, $operandCalls); + var_dump($result, $box->value, $receiverCalls, $operandCalls); + + $receiverCalls = 0; + $operandCalls = 0; + $box->value = 10; + try { + intCompoundReceiver($box, $receiverCalls)->value /= intCompoundOperand(3, $operandCalls); + } catch (TypeError $e) { + echo "receiver failure: ", $e::class, "\n"; + } + var_dump($box->value, $receiverCalls, $operandCalls); + + $box->value = 3; + $numericString = any('4'); + $box->value += $numericString; + var_dump($box->value); + + $box->value = 12; + $zero = 0; + try { + $box->value %= $zero; + } catch (DivisionByZeroError $e) { + echo "modulo zero: ", $e::class, "\n"; + } + var_dump($box->value); + + $negativeOne = -1; + try { + $box->value <<= $negativeOne; + } catch (ArithmeticError $e) { + echo "negative shift: ", $e::class, "\n"; + } + var_dump($box->value); +} +?> +--EXPECT-- +int(4) +int(4) +fraction: TypeError +int(8) +add overflow: TypeError +bool(true) +mul overflow: TypeError +bool(true) +int(9) +int(9) +int(1) +int(1) +receiver failure: TypeError +int(10) +int(1) +int(1) +int(7) +modulo zero: DivisionByZeroError +int(12) +negative shift: ArithmeticError +int(12) diff --git a/tests/compiler/object_property/native-int-property-assign-op-var.phpt b/tests/compiler/object_property/native-int-property-assign-op-var.phpt index da908531..9fa205e3 100644 --- a/tests/compiler/object_property/native-int-property-assign-op-var.phpt +++ b/tests/compiler/object_property/native-int-property-assign-op-var.phpt @@ -20,17 +20,14 @@ function main(): void var_dump($box->value); $text = any("3"); - try { - $box->value += $text; - } catch (TypeError $e) { - var_dump($e->getMessage()); - } + $box->value += $text; + var_dump($box->value); $bad = any("abc"); try { $box->value += $bad; } catch (TypeError $e) { - var_dump($e->getMessage()); + var_dump($e::class); } $selfBox = new NativeIntAssignOpBox(); @@ -41,6 +38,6 @@ function main(): void ?> --EXPECT-- int(3) -string(73) "Cannot assign string to property NativeIntAssignOpBox::$value of type int" -string(73) "Cannot assign string to property NativeIntAssignOpBox::$value of type int" +int(6) +string(9) "TypeError" int(6) diff --git a/tests/compiler/object_property/native-types-int-property-compound-assignment.phpt b/tests/compiler/object_property/native-types-int-property-compound-assignment.phpt new file mode 100644 index 00000000..3e5aa90d --- /dev/null +++ b/tests/compiler/object_property/native-types-int-property-compound-assignment.phpt @@ -0,0 +1,46 @@ +--TEST-- +Native-types int property compound assignments use C++ integer semantics +--FILE-- +value = 7; + $result = nativeTypesIntCompoundReceiver($box, $receiverCalls)->value + /= nativeTypesIntCompoundOperand(2, $operandCalls); + var_dump($result, $box->value, $receiverCalls, $operandCalls); + + $result = nativeTypesIntCompoundReceiver($box, $receiverCalls)->value *= 3; + var_dump($result, $box->value, $receiverCalls); +} +?> +--EXPECT-- +int(3) +int(3) +int(1) +int(1) +int(9) +int(9) +int(2)