Skip to content

fix(ir,codegen): autovivify missing parents in nested array writes#570

Open
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/555-autovivify-missing-parent
Open

fix(ir,codegen): autovivify missing parents in nested array writes#570
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/555-autovivify-missing-parent

Conversation

@mirchaemanuel

Copy link
Copy Markdown
Contributor

fix(ir,codegen): autovivify missing parents in nested array writes

Fixes #555.

Symptom

$a = ['x'];
$a[7][1] = 'patched';   // PHP: autovivifies $a[7] as an array, no warning
echo $a[7][1];          // elephc: empty line, "Undefined array key 7" twice,
                        // and the dropped value leaked

Root cause

The parent chain of a nested assignment was lowered as a plain read. A missing element produced a detached Mixed(null) box (with an undefined-key warning), and __rt_mixed_array_set released the value in its incompatible-target path. Nothing installed a container into the missing slot: there were no write-context lookup semantics for intermediate elements. Additionally, a string key on an indexed payload branched to the same drop path, losing legal writes like $a[0]['new'] = 'v'.

Fix

The parent chain now lowers with fetch-for-write semantics:

  • New __rt_mixed_array_get_for_write autovivifies missing indexed elements (grow, zero-fill, length update), null gap slots, boxed-null slots, and missing hash keys, installing an empty-array cell into the parent storage and returning the STORED cell (retained) so the three-operand set writes through the parent. Indexed payloads are first normalized via __rt_array_to_mixed (COW split + boxed slots, republished into the cell) — which also lands writes through concrete intermediates of 3+ level chains, the residual limitation documented in fix(ir): write nested array assignments through the parent cell (#529) #553.
  • Concrete array<mixed>/assoc locals go through a new __rt_array_ensure_elem_for_write wrapper plus a local storeback; a literal string key on an indexed local promotes it via Op::ArrayToHash.
  • __rt_mixed_array_set now promotes indexed payloads to hash storage for string keys (new __rt_mixed_cell_promote_to_hash) instead of dropping the write, matching PHP array-key semantics.
  • Write-context reads emit no undefined-key warning, matching PHP.
  • Both AArch64 and x86_64 emitters implemented symmetrically.

Tests

tests/codegen/arrays/nested_autovivify.rs (14 tests): the exact issue repro, append position, null gap and boxed-null parents, assoc missing key, multi-level int and string chains from [], Mixed indexed/assoc receivers, string-key child promotion, existing-parent and shared-array COW controls, and function-return visibility; all assert no warning and heap-debug cleanliness. (The json_decode variant asserts stdout/warnings only — Mixed builtin results stored into locals leak one owner reference on current main independently of nested writes, reproducible with a bare read.) 13/13 of the original tests failed before the fix.

Limitations (documented)

  • Incompatible scalar intermediates are intentionally not converted (the write is still dropped; PHP raises Error: Cannot use a scalar value as an array — no error machinery added here).
  • Negative indexes on dense indexed storage stay unsupported (pre-existing storage model).
  • Statically-Mixed key expressions on concrete locals and ArrayAccess-object intermediates keep the previous plain-read path.
  • Autovivified gaps remain dense (zero-filled) per elephc's array model, so count() can differ from PHP's sparse arrays (pre-existing).

QA

  • Issue repro prints patched, zero warnings, leak summary: clean, under --ir-opt=on and =off.
  • After rebasing onto current main (post-fix(ir): write nested array assignments through the parent cell (#529) #553, including its extended nested_mixed_write suite): nested_autovivify 14, nested_mixed_write 8, arrays:: 366, mixed 262 — all green. Pre-rebase full slices: nested 155, json 445, cargo test --lib 930 (one pre-existing environment-dependent DNS test failure unrelated to this change, fails identically on pristine main). No new clippy warnings.

https://claude.ai/code/session_01QyzntRke1Cgxgq1ueuz4X9

@github-actions github-actions Bot added area:codegen Touches target-aware assembly or backend lowering. area:eir Touches EIR definitions, lowering, validation, or passes. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. size:m Medium-sized pull request. type:fix Corrects broken or incompatible behavior. labels Jul 19, 2026
…llegalstudio#555)

A nested assignment through a missing intermediate element
($a[7][1] = 'patched' when $a[7] does not exist) lowered the parent
chain as a plain READ: the miss produced a detached Mixed(null) box
plus an "Undefined array key" warning, __rt_mixed_array_set hit its
incompatible-target drop path, and the write was silently lost. PHP
autovivifies the missing element as an array and emits no warning.

Lower the parent chain with fetch-for-write semantics instead:

- New runtime helper __rt_mixed_array_get_for_write mirrors the plain
  reader but installs an empty-array cell into missing indexed slots
  (growing and zero-filling gaps), null gap slots, boxed-null slots,
  and missing hash keys, and returns the STORED cell retained so the
  three-operand set writes through the parent storage. Indexed
  payloads are normalized through __rt_array_to_mixed first, which
  also fixes write-through for concrete intermediates of 3+ level
  chains (the residual limitation documented in the issue illegalstudio#529 fix).
  Non-container receivers tail-call the plain reader unchanged.
- New wrapper __rt_array_ensure_elem_for_write applies the same logic
  to concrete array<mixed>/assoc locals through a transient stack
  cell and returns the possibly promoted or reallocated container for
  the local storeback (prepare/store_prepared bracketing, so boxed
  Mixed-storage slot owners are released exactly once).
- New helper __rt_mixed_cell_promote_to_hash converts an indexed cell
  payload to hash storage (array_to_hash + hash_to_mixed, COW-safe);
  __rt_mixed_array_set now uses it for string keys on indexed
  payloads instead of dropping the write, matching PHP array-key
  semantics. A literal string key on an indexed local promotes the
  local through Op::ArrayToHash before the ensure call.
- The write-context parent read emits no undefined-key warning, and
  the post-ensure re-read is in bounds, so legal autovivifying writes
  are silent like PHP.

Both AArch64 and x86_64 emitters are implemented symmetrically.
Incompatible scalar intermediates are intentionally not converted
(the leaf set still drops the write; PHP raises an Error there).

Claude-Session: https://claude.ai/code/session_01QyzntRke1Cgxgq1ueuz4X9
@mirchaemanuel
mirchaemanuel force-pushed the fix/555-autovivify-missing-parent branch from 3ac326a to 074c195 Compare July 20, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:codegen Touches target-aware assembly or backend lowering. area:eir Touches EIR definitions, lowering, validation, or passes. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. size:m Medium-sized pull request. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chained writes through a missing array element fail to autovivify the parent

1 participant