Skip to content

fix(parser): preserve indirect ArrayAccess ??= semantics - #71

Open
hafung wants to merge 2 commits into
swoole:masterfrom
hafung:fix/arrayaccess-coalesce-write
Open

fix(parser): preserve indirect ArrayAccess ??= semantics#71
hafung wants to merge 2 commits into
swoole:masterfrom
hafung:fix/arrayaccess-coalesce-write

Conversation

@hafung

@hafung hafung commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #70 and the direct-variable fix in 4e13e7b. This preserves PHP ??= semantics when an array-dimension target may dispatch through ArrayAccess indirectly or change representation between phases.

  • separate the presence/read phase from the write phase
  • invoke offsetExists() once and offsetGet() only for a present offset
  • dispatch a miss through the container that is current after RHS evaluation
  • preserve the fixed-array lvalue fast path when no intervening expression can replace the container
  • recursively preserve the same rules for nested ArrayAccess dimensions

Failure mechanism and invariant

An ArrayAccess offsetGet() result is a value, not a writable bucket. In addition, a key expression, offsetExists(), or the RHS can mutate an aliased key/container. The read phase must use one stabilized receiver/key pair, while the write phase must resolve the current target after the RHS. Missing outer dimensions must not be created before the RHS.

The previous lowering handled a plain variable but did not enforce that invariant for nested dimensions, property/magic-property targets, references, or object/array representation changes.

Coverage matrix

  • direct, mixed, and referenced containers
  • nested array-to-object and ArrayAccess-to-object targets
  • ordinary and magic-property targets
  • hit, null, and missing offsets
  • key/container mutation from offsetExists() and from the RHS
  • object-to-array and array-to-object rebinding
  • result-used and result-unused paths (with the existing suite)
  • fixed arrays retain their existing fast path unless an intervening operand can mutate the representation

Native objects and the features documented as incompatible remain outside this path.

Verification

  • reproducer and adversarial outputs compared exactly with Zend PHP 8.5
  • ArrayAccessCoalesceAssignCodegenTest: 5 tests, 32 assertions
  • target PHPT: 1/1 passed
  • complete coalesce PHPT directory: 21/21 passed
  • fresh Windows MSVC AOT artifacts built and executed at O0 and O3; both matched Zend output

@hafung
hafung force-pushed the fix/arrayaccess-coalesce-write branch 2 times, most recently from a65a361 to 753fcc6 Compare September 2, 2026 07:38
@hafung hafung changed the title fix(parser): preserve ArrayAccess writes in ??= fix(parser): preserve indirect ArrayAccess ??= semantics Sep 2, 2026

@matyhtf matyhtf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase this branch onto the latest master; #45, #52, #59, #63, and #66 have now been merged.

The write phase currently dispatches only as isArray() ? item(..., true) : offsetSet(...). The second branch is not equivalent to Zend dimension assignment for a dynamic mixed/php::Var container. For example:

function f(mixed $value): void {
    $value['k'] ??= 7;
}
f(1);

Zend throws Cannot use a scalar value as an array. Variant::offsetSet() currently does nothing for an integer, so this lowering can return 7 while leaving the target unchanged. Other runtime representations also differ: null must become an array, false follows PHP's deprecated conversion behavior, invalid scalars must throw, and string offsets have their own key/range errors.

Please route the write through a PHPX helper that implements Zend write-dimension semantics for the complete runtime value domain, while retaining the explicit ArrayAccess path only after proving the runtime value is an object. Add PHPT coverage for dynamic null, false, int/float/bool, string keys/offsets, arrays, references, and ArrayAccess objects.

@hafung

hafung commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the review in bd6b8be and rebased onto current master (e90d624).

Changes:

  • dynamic non-object writes now go through a Zend-compatible PHPX helper instead of Variant::offsetSet()
  • the explicit offsetSet() path is selected only after proving the runtime receiver is an object
  • assignment expressions return the actual write result (important for string-offset writes)
  • PHPT coverage now includes dynamic null, false, true/int/float, strings (valid/invalid keys and extension), arrays, references/aliases, and ArrayAccess

The required PHPX helper is proposed in swoole/phpx#102.

Verification completed locally:

  • Zend reproducer/output comparison: pass
  • targeted PHPUnit codegen suite: 5 tests, 32 assertions
  • compiler coalesce PHPT suite: 21/21 pass
  • fresh AOT O0 artifact: pass
  • fresh AOT O3 artifact: pass
  • generated code inspected for receiver/key/value ordering and object-vs-nonobject dispatch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants