Skip to content

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

Open
hafung wants to merge 1 commit into
swoole:masterfrom
hafung:fix/arrayaccess-coalesce-write
Open

fix(parser): preserve indirect ArrayAccess ??= semantics#71
hafung wants to merge 1 commit 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.

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