diff --git a/phpunit/code/arrayaccess-coalesce-assign-codegen.php b/phpunit/code/arrayaccess-coalesce-assign-codegen.php new file mode 100644 index 00000000..4f1c2ddc --- /dev/null +++ b/phpunit/code/arrayaccess-coalesce-assign-codegen.php @@ -0,0 +1,53 @@ +value; + } +} + +function coalesceArrayAccess(CodegenArrayAccessBag $bag, string $key): mixed +{ + return $bag[$key] ??= 42; +} + +function coalesceMixedArrayAccess(mixed &$container, string $key): mixed +{ + return $container[$key] ??= 43; +} + +function coalesceMagicArrayAccess(CodegenArrayAccessHolder $holder, string $key): mixed +{ + return $holder->virtual[$key] ??= 44; +} + +function coalesceFixedArray(string $key): mixed +{ + $container = []; + return $container[$key] ??= 45; +} + +function replaceCodegenArray(mixed &$container, string &$key): int +{ + $container = new CodegenArrayAccessBag(); + $key = 'replacement'; + return 46; +} + +function coalesceMutableFixedArray(string $key): mixed +{ + $container = []; + return $container[$key] ??= replaceCodegenArray($container, $key); +} diff --git a/phpunit/src/ArrayAccessCoalesceAssignCodegenTest.php b/phpunit/src/ArrayAccessCoalesceAssignCodegenTest.php new file mode 100644 index 00000000..baa52485 --- /dev/null +++ b/phpunit/src/ArrayAccessCoalesceAssignCodegenTest.php @@ -0,0 +1,80 @@ +compileFixture(); + $body = $this->extractFunctionBody($code, 'php::Var php_coalescearrayaccess('); + + self::assertSame(1, substr_count($body, '.offsetExists(')); + self::assertSame(1, substr_count($body, '.offsetGet(')); + self::assertSame(1, substr_count($body, '.offsetSet(key,')); + self::assertStringContainsString('.isObject()', $body); + self::assertStringContainsString('.isArray()', $body); + } + + public function testMixedTargetRetainsArrayAndObjectDispatch(): void + { + $code = $this->compileFixture(); + $body = $this->extractFunctionBody($code, 'php::Var php_coalescemixedarrayaccess('); + + self::assertStringContainsString('.isObject()', $body); + self::assertStringContainsString('php::exists(', $body); + self::assertStringContainsString('.isArray()', $body); + self::assertStringContainsString('.offsetSet(key,', $body); + } + + public function testMagicContainerIsEvaluatedOncePerReadAndWritePhase(): void + { + $code = $this->compileFixture(); + $body = $this->extractFunctionBody($code, 'php::Var php_coalescemagicarrayaccess('); + + self::assertSame(2, substr_count($body, 'typephp_read_property_cached(holder,')); + self::assertStringContainsString('[&](auto &&', $body); + } + + public function testFixedArrayKeepsDirectFastPathWhenItsTypeCannotChange(): void + { + $code = $this->compileFixture(); + $body = $this->extractFunctionBody($code, 'php::Var php_coalescefixedarray('); + + self::assertStringContainsString('.item(key, true)', $body); + self::assertStringNotContainsString('.isObject()', $body); + self::assertStringNotContainsString('.isArray()', $body); + } + + public function testFixedArrayUsesRuntimeDispatchWhenRhsCanReplaceIt(): void + { + $code = $this->compileFixture(); + $body = $this->extractFunctionBody($code, 'php::Var php_coalescemutablefixedarray('); + + self::assertStringContainsString('.isObject()', $body); + self::assertStringContainsString('.isArray()', $body); + self::assertStringContainsString('.offsetSet(key,', $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 + { + $compiler = CompilerTest::create(TYPEPHP_ROOT_PATH); + $source = TYPEPHP_ROOT_PATH . '/phpunit/code/arrayaccess-coalesce-assign-codegen.php'; + $compiler->addFiles([$source]); + $compiler->prepareFile($source); + $generated = $compiler->convertFile($source); + $code = file_get_contents($generated); + + self::assertIsString($code); + return $code; + } +} diff --git a/src/Parser/AssignOpTrait.php b/src/Parser/AssignOpTrait.php index dd7731a5..694f025e 100644 --- a/src/Parser/AssignOpTrait.php +++ b/src/Parser/AssignOpTrait.php @@ -1625,6 +1625,12 @@ protected function parseAssignOpCoalesce(Expr\AssignOp\Coalesce $expr): string $this->assertNativeArrayAccessDirectWrite($expr->var, false); $this->checkLeftValue($expr->var); + $arrayContainerCanChange = $expr->var instanceof Expr\ArrayDimFetch + && $expr->var->dim !== null + && ($this->shouldMaterializeOrderedOperand($expr->var->var) + || $this->shouldMaterializeOrderedOperand($expr->var->dim) + || $this->shouldMaterializeOrderedOperand($expr->expr)); + // Zend evaluates the target's receiver and array keys exactly once, // before the isset check and regardless of its outcome. The lowering // below mentions the target several times (isset, read, write), so @@ -1686,11 +1692,19 @@ protected function parseAssignOpCoalesce(Expr\AssignOp\Coalesce $expr): string } } - $isset = $var !== null && $this->isNativeObjectVar($var) - ? $var . ' != nullptr' - : $this->parseChainedExpr($expr->var, self::OP_ISSET); - - $var ??= $this->parseWritableIdentifier($expr->var); + $arrayAccessTarget = $this->resolveCoalesceArrayAccessTarget( + $expr->var, + $arrayContainerCanChange, + ); + if ($var !== null && $this->isNativeObjectVar($var)) { + $isset = $var . ' != nullptr'; + } elseif ($arrayAccessTarget !== null) { + $var = $this->addTmpVar(Type::VAR); + $isset = $this->parseArrayAccessCoalescePresence($arrayAccessTarget, $var); + } else { + $isset = $this->parseChainedExpr($expr->var, self::OP_ISSET); + $var ??= $this->parseWritableIdentifier($expr->var); + } $propertyWriteTarget = $this->preparePropertyWriteTarget($expr->var); if ($propertyWriteTarget !== null) { @@ -1719,7 +1733,6 @@ protected function parseAssignOpCoalesce(Expr\AssignOp\Coalesce $expr): string $this->errorUndefinedVariable($expr->expr); } - $arrayAccessTarget = $this->resolveCoalesceArrayAccessTarget($expr->var); if ($arrayAccessTarget !== null) { return $this->emitCoalesceArrayAccessAssignment( $arrayAccessTarget, @@ -1775,79 +1788,149 @@ protected function parseAssignOpCoalesce(Expr\AssignOp\Coalesce $expr): string } /** - * ArrayAccess dimensions do not expose writable buckets: offsetGet() - * returns a value, while a write must dispatch through offsetSet(). Keep - * ordinary arrays on the existing lvalue path so assignments into array - * references continue to update the referenced bucket in place. - * - * @return array{container: string, key: string, objectCondition: string}|null + * A fixed php::Array exposes a real bucket lvalue through item(), but an + * object or dynamically represented container may dispatch [] through + * ArrayAccess. Its offsetGet() result is a value, not the write target; + * the not-set branch must therefore use offsetSet(). */ - private function resolveCoalesceArrayAccessTarget(Expr $target): ?array - { + private function resolveCoalesceArrayAccessTarget( + Expr $target, + bool $includeFixedArray, + ): ?Expr\ArrayDimFetch { if (!$target instanceof Expr\ArrayDimFetch || $target->dim === null - || !$this->isVarExpr($target->var) || $this->isStdContainerExpr($target) + || $this->isNativeObjectClass($this->detectClassOfExpr($target->var)) ) { return null; } - $container = $this->parseIdentifier($target->var); - $containerType = $this->getVarType($container); - if (!in_array($containerType, [Type::OBJECT, Type::VAR, Type::REF], true)) { - return null; + $types = [Type::OBJECT, Type::VAR, Type::REF]; + if ($includeFixedArray) { + // A key/container expression or the RHS can pass the source + // variable by reference and replace an inferred php::Array with an + // ArrayAccess object. Keep the direct array lvalue fast path only + // when no intervening expression can change its representation. + $types[] = Type::ARRAY; } - - return [ - 'container' => $container, - 'key' => $this->parseIdentifier($target->dim), - // Even a statically object-typed PHP variable may currently hold - // null. PHP converts that null to an array on dimension write, so - // only the runtime object case may dispatch through offsetSet(). - 'objectCondition' => $container . '.isObject()', - ]; + return in_array($this->detectTypeOfExpr($target->var), $types, true) + ? $target + : null; } /** - * @param array{container: string, key: string, objectCondition: string} $target + * Keep the coalesce read and write operations separate for a target that + * can be ArrayAccess at runtime. The presence expression supplies the + * already-read hit value. The miss branch completes the RHS before + * dispatching the write, exactly like the general captured-RHS path. + * * @param list $rightBefore * @param list $rightAfter */ private function emitCoalesceArrayAccessAssignment( - array $target, + Expr\ArrayDimFetch $target, string $isset, - string $readTarget, + string $selectedValue, string $right, array $rightBefore, array $rightAfter, ): string { - $current = $this->genTmpVarName(); - $rhs = $this->genTmpVarName(); - $container = $target['container']; - $key = $target['key']; - $isObject = $this->genTmpVarName(); + $rhs = $this->addTmpVar(Type::VAR); + $store = $this->parseArrayAccessCoalesceStore($target, $rhs); $code = '[&]() -> php::Var {' . PHP_EOL; - $code .= $this->getIndent() . 'const bool ' . $isObject . ' = ' - . $target['objectCondition'] . ';' . PHP_EOL; - $code .= $this->getIndent() . 'if (' . $isset . ') {' . PHP_EOL; - $code .= $this->getIndent(2) . 'php::Var ' . $current . ' = ' . $isObject - . ' ? ' . $container . '.offsetGet(' . $key . ') : ' . $readTarget . ';' . PHP_EOL; - $code .= $this->getIndent(2) . 'if (!' . $current . '.isNull()) {' . PHP_EOL; - $code .= $this->getIndent(3) . 'return ' . $current . ';' . PHP_EOL; - $code .= $this->getIndent(2) . '}' . PHP_EOL; - $code .= $this->getIndent() . '}' . PHP_EOL; + $code .= $this->getIndent() . 'if (' . $isset . ') { return ' . $selectedValue . '; }' . PHP_EOL; $code .= $this->formatCapturedStmtLines($rightBefore); - $code .= $this->getIndent() . 'php::Var ' . $rhs . ' = ' . $right . ';' . PHP_EOL; + $code .= $this->getIndent() . $rhs . ' = ' . $right . ';' . PHP_EOL; $code .= $this->formatCapturedStmtLines($rightAfter); - $code .= $this->getIndent() . 'if (' . $isObject . ') {' . PHP_EOL; - $code .= $this->getIndent(2) . $container . '.offsetSet(' . $key . ', ' . $rhs . ');' . PHP_EOL; - $code .= $this->getIndent() . '} else {' . PHP_EOL; - $code .= $this->getIndent(2) . $readTarget . ' = ' . $rhs . ';' . PHP_EOL; - $code .= $this->getIndent() . '}' . PHP_EOL; + $code .= $this->getIndent() . $store . ';' . PHP_EOL; $code .= $this->getIndent() . 'return ' . $rhs . ';' . PHP_EOL; - $code .= $this->getIndent() . '}()'; - return $code; + return $code . $this->getIndent() . '}()'; + } + + /** + * Read a stabilized ArrayAccess-capable target without routing the object + * through php::exists(..., result). That generic chain first invokes the + * object's has-dimension handler and then performs an IS-mode read, which + * invokes offsetExists() a second time. Zend's ??= calls offsetExists() + * once, calls offsetGet() only after a positive result, and still treats a + * null offsetGet() result as a miss. + * + * A referenced source variable can also change representation while a key + * expression or offsetExists() runs. Keep non-object values on the generic + * chain path; only the phase snapshot's runtime object uses the explicit + * ArrayAccess sequence. + */ + private function parseArrayAccessCoalescePresence( + Expr\ArrayDimFetch $target, + string $selectedValue, + ): string { + if ($this->isVarExpr($target->var)) { + $container = $this->parseIdentifier($target->var); + $this->checkVarMustExist($target->var, $container); + $containerPresence = null; + } elseif ($target->var instanceof Expr\ArrayDimFetch && $target->var->dim !== null) { + // Apply the same phase semantics recursively. The generic chain + // walker performs an IS-mode read after its has-dimension check, + // which invokes offsetExists() twice on intermediate ArrayAccess. + $container = $this->addTmpVar(Type::VAR); + $containerPresence = $this->parseArrayAccessCoalescePresence( + $target->var, + $container, + ); + } else { + // A property/call result is evaluated as a value. A writable read + // here would create missing outer array buckets before the RHS. + $container = $this->parseIdentifier($target->var); + $containerPresence = null; + } + $key = $this->parseIdentifier($target->dim); + + // Snapshot the container and key for the complete presence/read + // phase. offsetExists() may rebind either source variable, but Zend + // still invokes offsetGet() on the same object with the same key. The + // write phase deliberately evaluates the source expressions again. + $stableContainer = $this->genTmpVarName(); + $stableKey = $this->genTmpVarName(); + + $objectPresence = '(' . $stableContainer . '.offsetExists(' . $stableKey . ')' + . ' && ((' . $selectedValue . ' = ' . $stableContainer . '.offsetGet(' . $stableKey . ')),' + . ' !' . $selectedValue . '.isNull()))'; + $otherPresence = 'php::exists(' . $stableContainer . ', ' + . '{{php::ArrayDimFetch, ' . Type::VAR . '(' . $stableKey . ')}}, ' + . $selectedValue . ')'; + $presence = '(' . $stableContainer . '.isObject() ? ' + . $objectPresence . ' : ' . $otherPresence . ')'; + + $probe = '([&](' . Type::VAR . ' ' . $stableContainer . ') { ' + . Type::VAR . ' ' . $stableKey . ' = ' . $key . '; ' + . 'return ' . $presence . '; })(' . $container . ')'; + return $containerPresence === null + ? $probe + : '(' . $containerPresence . ' && ' . $probe . ')'; + } + + /** + * Write a stabilized array-dimension target without treating offsetGet() + * as an lvalue. The key expression, offsetExists(), or RHS may replace the + * source container through an alias, so dispatch using the value that is + * current at the write phase. item(..., true) retains reference-bucket + * semantics for the runtime array case. + */ + private function parseArrayAccessCoalesceStore(Expr\ArrayDimFetch $target, string $value): string + { + $container = $this->parseWritableIdentifier($target->var); + $key = $this->parseIdentifier($target->dim); + $stableContainer = $this->genTmpVarName(); + + $arrayWrite = 'static_cast(' . $stableContainer . '.item(' + . $key . ', true) = ' . $value . ')'; + $objectWrite = $stableContainer . '.offsetSet(' . $key . ', ' . $value . ')'; + $store = '(' . $stableContainer . '.isArray() ? ' + . $arrayWrite . ' : ' . $objectWrite . ')'; + + return '[&](auto &&' . $stableContainer . ') { ' . $store . '; }(' + . $container . ')'; } /** diff --git a/tests/compiler/coalesce/assign-coalesce-arrayaccess.phpt b/tests/compiler/coalesce/assign-coalesce-arrayaccess.phpt new file mode 100644 index 00000000..ddbc96ed --- /dev/null +++ b/tests/compiler/coalesce/assign-coalesce-arrayaccess.phpt @@ -0,0 +1,185 @@ +--TEST-- +ArrayAccess ??= preserves indirect targets and mutable phase dependencies +--FILE-- +calls[] = "exists:$offset"; + if ($this->onExists !== null) { + ($this->onExists)(); + } + return array_key_exists($offset, $this->data); + } + + public function offsetGet(mixed $offset): mixed + { + $this->calls[] = "get:$offset"; + return $this->data[$offset] ?? null; + } + + public function offsetSet(mixed $offset, mixed $value): void + { + $this->calls[] = "set:$offset"; + $this->data[$offset] = $value; + } + + public function offsetUnset(mixed $offset): void + { + unset($this->data[$offset]); + } +} + +final class IndirectCoalesceHolder +{ + public int $gets = 0; + public mixed $value = []; + + public function __construct(public IndirectCoalesceBag $bag) + { + } + + public function __get(string $name): mixed + { + $this->gets++; + return $this->bag; + } +} + +function rebindCoalesceToArray(mixed &$container, string &$key): int +{ + $container = []; + $key = 'array-key'; + return 81; +} + +function rebindCoalesceToObject( + mixed &$container, + string &$key, + IndirectCoalesceBag $replacement, +): int { + $container = $replacement; + $key = 'object-key'; + return 82; +} + +function rebindCoalesceKeyToObject(mixed &$container, IndirectCoalesceBag $replacement): string +{ + $container = $replacement; + return 'key'; +} + +function inspectNestedCoalesceRhs(array $container): int +{ + echo 'nested-rhs:', json_encode($container), "\n"; + return 54; +} + +function showIndirectCoalesce(string $label, mixed $value): void +{ + echo $label, ':', json_encode($value), "\n"; +} + +function main(): void +{ + $nestedBag = new IndirectCoalesceBag(); + $nested = ['bag' => $nestedBag]; + $result = ($nested['bag']['key'] ??= 51); + showIndirectCoalesce('nested', [$result, $nestedBag->data, $nestedBag->calls]); + + $innerBag = new IndirectCoalesceBag(); + $outerBag = new IndirectCoalesceBag(['bag' => $innerBag]); + $result = ($outerBag['bag']['key'] ??= 55); + showIndirectCoalesce('nested-access', [ + $result, + $outerBag->calls, + $innerBag->data, + $innerBag->calls, + ]); + + $missingNested = []; + $result = ($missingNested['bag']['key'] ??= inspectNestedCoalesceRhs($missingNested)); + showIndirectCoalesce('nested-missing', [$result, $missingNested]); + + $magicBag = new IndirectCoalesceBag(); + $magic = new IndirectCoalesceHolder($magicBag); + $result = ($magic->virtual['key'] ??= 52); + showIndirectCoalesce('magic', [$result, $magic->gets, $magicBag->data, $magicBag->calls]); + + $property = new IndirectCoalesceHolder(new IndirectCoalesceBag()); + $result = ($property->value['key'] ??= 53); + showIndirectCoalesce('property-array', [$result, $property->value]); + + $phase = new IndirectCoalesceBag(['old' => 71]); + $phaseOriginal = $phase; + $phaseReplacement = new IndirectCoalesceBag(['new' => 72]); + $phaseKey = 'old'; + $phaseOriginal->onExists = function () use (&$phase, &$phaseKey, $phaseReplacement): void { + $phase = $phaseReplacement; + $phaseKey = 'new'; + }; + $result = ($phase[$phaseKey] ??= 73); + showIndirectCoalesce('phase-hit', [ + $result, + $phaseKey, + $phaseOriginal->calls, + $phaseReplacement->calls, + ]); + + $miss = new IndirectCoalesceBag(); + $missOriginal = $miss; + $missReplacement = new IndirectCoalesceBag(); + $missKey = 'old'; + $missOriginal->onExists = function () use (&$miss, &$missKey, $missReplacement): void { + $miss = $missReplacement; + $missKey = 'new'; + }; + $result = ($miss[$missKey] ??= 74); + showIndirectCoalesce('phase-miss', [ + $result, + $missKey, + $missOriginal->data, + $missOriginal->calls, + $missReplacement->data, + $missReplacement->calls, + ]); + + $objectToArray = new IndirectCoalesceBag(); + $objectOriginal = $objectToArray; + $key = 'old'; + $result = ($objectToArray[$key] ??= rebindCoalesceToArray($objectToArray, $key)); + showIndirectCoalesce('object-array', [$result, $key, $objectToArray, $objectOriginal->calls]); + + $arrayToObject = []; + $objectReplacement = new IndirectCoalesceBag(); + $key = 'old'; + $result = ($arrayToObject[$key] ??= rebindCoalesceToObject($arrayToObject, $key, $objectReplacement)); + showIndirectCoalesce('array-object', [$result, $key, $objectReplacement->data, $objectReplacement->calls]); + + $keyRebound = []; + $keyReplacement = new IndirectCoalesceBag(); + $result = ($keyRebound[rebindCoalesceKeyToObject($keyRebound, $keyReplacement)] ??= 83); + showIndirectCoalesce('key-object', [$result, $keyReplacement->data, $keyReplacement->calls]); +} +?> +--EXPECT-- +nested:[51,{"key":51},["exists:key","set:key"]] +nested-access:[55,["exists:bag","get:bag","get:bag"],{"key":55},["exists:key","set:key"]] +nested-rhs:[] +nested-missing:[54,{"bag":{"key":54}}] +magic:[52,2,{"key":52},["exists:key","set:key"]] +property-array:[53,{"key":53}] +phase-hit:[71,"new",["exists:old","get:old"],[]] +phase-miss:[74,"new",[],["exists:old"],{"new":74},["set:new"]] +object-array:[81,"array-key",{"array-key":81},["exists:old"]] +array-object:[82,"object-key",{"object-key":82},["set:object-key"]] +key-object:[83,{"key":83},["exists:key","set:key"]]