fix(ir_lower): widen loop-reassigned scalar arrays to boxed storage

[mirchaemanuel]

Summary

Reassigning a raw-scalar array local to a literal rebuilt from its own elements
inside a loop read freed/misrepresented storage. $r = [$r[0] - 1, 0] printed
heap addresses from the second iteration, and the same rebind in a while
condition never terminated (the corrupted read is always truthy). A pure swap
$r = [$r[1], $r[0]] was corrupted the same way.

Fixes #594.

Root cause

A constant initializer [3, 0] builds an array<int> whose elements are raw
scalars. The self-referential rebind produces a wider element representation —
array<mixed> (the $r[0] - 1 overflow-checked subtraction boxes) or
array<int|null> (each $r[i] array read is the inline tagged scalar). But a
read of $r[0] at the top of the single-pass loop body is still typed against
the entry array<int>, so array_get is lowered as a raw-int element read.
From iteration 2 it reads the promoted (boxed/tagged) cell as a raw scalar, so a
heap pointer surfaces as an int.

Seeding the initializer with a runtime value ([$argc + 2, 0]) was already
correct because the entry array is array<mixed> from the start, so the entry
type and the rebind agree — which is why the bug looked specific to a
compile-time-constant initializer.

The loop-entry array widening added for issue #452
(loop_grown_mixed_array_pushes) only covered in-place growth sites
($a[] =, $a[$i] =, array_push), so a full reassignment $r = [...] was
never widened.

Fix

Add loop_reassigned_mixed_arrays to the shared loop-widening prescan. It
reports a local that holds a raw-scalar indexed array at loop entry and is
reassigned in the loop body to a literal whose element lowers to a boxed mixed
or tagged int|null cell. Both the type checker (TypeEnv) and EIR lowering
(local_types plus the existing up-front ArrayToMixed promotion) consume it,
so every read of the local uses the boxed representation the back edge produces,
and the entry array is converted in place before the body runs.

A self-referential rebind reads $r's own elements, so once $r is widened its
reads return mixed and the rebuilt literal is itself array<mixed> — no
per-element coercion is needed. A same-representation rebind
($r = [count($r), 0], still array<int>) is deliberately left untouched, and
the promotion converts the previous array in place without leaking or
double-freeing it.

Tests

Added 7 regression tests in tests/codegen/array_basics.rs next to the #452
tests: the two issue repros (for-loop garbage sequence and terminating while),
a self-referential swap, a float flavour, the runtime-seeded control, a
same-representation guard that must not widen, and a heap-clean (allocs == frees)
guard. Verified red without the fix and green with it. The sibling #452 tests
and the array_basics / runtime_gc / foreach / control_flow / optimizer
slices remain green.

Not fixed (out of scope, separate mechanisms — filed as follow-ups)

• Cascading widening through an external variable that becomes mixed inside the
  loop ($v = $v - 1; $r = [$v, 0]) needs a real fixpoint, not a single-pass
  prescan.
• A non-self-referential rebind that changes between two raw scalar reprs
  (array<string> → array<int>) would additionally need the rebind coerced to
  mixed storage.
• Issue Reassigning an array variable to [] corrupts implode() (compile error) and glob()/scandir() reads (wrong data) #509 (reassigning an array to []) is a distinct element-typing gap
  ([] → array<never>/Void) with no loop or self-reference; it is not
  addressed here.

https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L

[mirchaemanuel]

The three out-of-scope items from the PR description are now filed with reverified repros: #607 (cascading widening through an external variable — needs a fixpoint, not a single-pass prescan), #608 (raw→raw representation rebind, string→int, silently empty output) and #609 (residual 4-block leak in the while variant, pre-existing and independent of this fix). Also posted a status update on #509 with its current-main symptoms — different root cause, not addressed here.

[nahime0]

Follow-up after merging the latest main: the original targeted rebind prescan has been generalized into a checker-owned fixed-point storage analysis (33fa7a377).

The updated implementation now:

• joins both array growth and full rebind evidence until the loop storage types stabilize;
• handles cascading promotions (Loop-widening prescan misses cascading widening through an external variable feeding an array rebind #607), non-literal RHSs (including locals and call results), and raw-to-raw representation changes (Loop rebind between two raw scalar array representations (string→int) silently misreads elements #608);
• covers indexed and associative arrays, including COW-safe ArrayToMixed / HashToMixed materialization;
• records one scoped checker contract per function/closure and loop, which EIR lowering consumes instead of repeating RHS inference;
• keeps same-representation rebinds unpromoted.

Validation completed on the published head:

• cargo build
• cargo check --tests
• codegen::array_basics: 109/109 passed
• loop-reassignment regressions: 14/14 passed, also with ELEPHC_IR_OPT=off
• existing Growing a Mixed-promoted array in a loop then iterating+consuming it corrupts the heap #452 loop-growth regressions: 13/13 passed
• git diff --check

Issues #509 and #609 remain separate mechanisms and are intentionally not claimed as fixed by this change.

[mirchaemanuel]

Picking this up. CI is red on the current head 33fa7a377; it was green on 16e4caa0e (2026-07-23), with failures on all three architectures.

Failing tests: codegen::io::streams::test_stream_context_create_returns_resource, codegen::io::streams::test_stream_context_set_default_returns_resource, codegen::exceptions::test_exception_constructor_accepts_previous, codegen::runtime_gc::regressions::test_regression_escaped_caught_exception_retained_once, codegen::runtime_gc::regressions::test_regression_408_string_key_promotion_does_not_leak, codegen::regressions::arrays::test_array_reassign_after_function_growth.

As an observation only: the two stream_context resource failures look similar in shape to the resource-typing regression already being handled on #596.

This comes from the CI logs; we have not reproduced it locally yet and are not claiming a cause.

[nahime0]

Hi @mirchaemanuel, I'm ooo. Yes if you can push this to green it'll be great. Thanks

[mirchaemanuel]

I dug into the 20 red checks here. There are three distinct root causes, not one. I've pushed a fix for the first; the other two I'm deliberately leaving to you, because repairing them means finishing a design decision rather than fixing a defect, and that's your call.

Short version: I'd suggest not merging 33fa7a377 tomorrow as it stands. One of its failure modes is a silent miscompile with the diagnostic swallowed. Details below so you can judge for yourself.

---

1. Fixed and pushed — 4690681fa

contextualize_local_assignment (src/ir_lower/stmt/mod.rs:372, :442) returned codegen_repr() as the local's recorded PHP type on the fall-through path. That collapses Resource(_)→Int, Union(_)→Mixed/TaggedScalar, False→Bool for every assignment, not just loop-carried ones. Minimal repro, no loop involved:

$c = stream_context_create();
var_dump($c);   // int(1)  — should be resource

It also explains the exception crash: $prev = $e->getPrevious() is ?Throwable, i.e. a Union, collapsed to Mixed, so the null-check lowers against the wrong storage shape. The fix keeps the collapsed form for the storage-contract comparisons and returns the uncollapsed type otherwise — what the pre-rewrite function did. 7 lines, none of your new coercion arms touched.

This clears test_stream_context_create_returns_resource, test_stream_context_set_default_returns_resource and test_exception_constructor_accepts_previous.

2. Not fixed — the fixed point feeds itself through global checker state

stabilize_loop_storage calls checker.infer_type() on loop-body RHS expressions. For a call expression that re-enters resolve_function_signature, which mutates global state. So speculative, deliberately-widened intermediate types flow into signature resolution and back into the fixed point. Instrumented trace for $arr = grow($arr) in a loop:

assign arr: existing=Array(Int)   incoming=Array(Mixed) merged=Array(Mixed)
resolve grow FAILED with params=[("arr", Mixed)]; leaving provisional sig Some(Int)
assign arr: existing=Array(Mixed) incoming=Mixed        merged=Mixed
assign arr: existing=Mixed        incoming=Int          merged=Mixed
final: fn grow: params=[Mixed] -> Int

Three things interact: signature.rs:116 inserts a provisional return_type: Int recursion placeholder; :175-177 returns Err on a failed body check without rolling that placeholder back, so params=[Mixed] -> Int sticks; and stabilize_loop_storage's .ok() discards the error so no diagnostic ever surfaces, while apply_assignment_evidence's .unwrap_or(Mixed) reads "inference failed" as "could be anything" and widens, closing the loop.

EIR then lowers return $arr in grow as v13: I64 = cast v12 I64 — an array cast to an integer — and count() reads 0. Silent miscompile, no diagnostic. That's test_array_reassign_after_function_growth.

Rolling back the provisional signature is correct on its own merits and I'd suggest it regardless, but it doesn't fix this: .ok() → None → unwrap_or(Mixed) still widens. Making "no evidence" stop widening changes the lattice and risks re-opening #452/#594, which this PR exists to fix.

3. Not fixed — the storage contract isn't honoured by the write side

stabilize_loop_storage does env.insert(name, storage_type), so the contract overwrites the checker's semantic type and lowering trusts it. But the fixed point models payload widening only — not container-kind change — and the element-write paths weren't updated to match.

test_regression_escaped_caught_exception_retained_once: contract for $keep is Array(Mixed), but $keep[] = $e pushes a raw object pointer into a boxed-payload array, and the read comes back as a box:

array_push v14 v15                     ; raw Throwable pointer
v28: Heap(Mixed) = array_get v26 v27   ; read back as a box
v30: Str = method_call v28 getMessage  ; dereferences the object header as a Mixed box

This is exactly the hazard your own docstring on coerce_container_to_mixed_payload describes — $keep[] = $e just never applies it.

test_regression_408_string_key_promotion_does_not_leak: contract says Array(Mixed) while the real type is AssocArray{Mixed,Mixed}. The contract wins, so $g = [1,2] is no longer converted to hash storage at the assignment; the promotion moves into the loop where array_to_hash allocates after release already dropped the source. 5005 allocs vs 5004 frees.

Repairing this properly means auditing every element-write lowering path — array_push of objects, string-key promotion, and by the same argument ref-assign, list-unpack, foreach-by-ref — to box values into the contract's payload representation. That's the missing half of the contract, and it's your design to complete, not mine to guess at.

---

Green-head control

Built 16e4caa0e in the same worktree and ran the same repros: $arr = grow($arr) loop → 641 (branch: 0); escaped-exception repro → 5|4, exit 0 (branch: exit 139); stream_context_create → resource(2) of type (stream) (branch, pre-fix: int(1)). So all three are regressions from 33fa7a377, not pre-existing.

One genuine pre-existing bug found on the way, present on the green head too and untouched: a typed function grow(array $arr): array called in a loop segfaults. Happy to file it.

The call

If you want #606 green tomorrow, the cheapest honest route is reverting to 16e4caa0e — it was green and it fixes #594 — and letting the fixed-point generalization come back as its own PR with its own CI. It absorbs #607/#608, which is valuable, but its write-side lowering is incomplete and cause 2 is a silent miscompile, which is worse than the bugs being fixed.

If you'd rather land it anyway, the smallest subset I'd trust is the pushed fix plus rolling back the provisional signature on the error path. That still won't make CI green — cause 3 remains — but it removes the silent-miscompile class.

Unverified, stated as such: everything above is macOS aarch64 only, I have no local evidence for the two Linux targets. And I confirmed causes 2 and 3 explain the three named failures, not that they're the only remaining causes — my regression run covered four modules (834 passed, 3 failed, the 3 known) rather than the full suite.

One cross-PR note for whatever order you merge in: 5bb580ee5 from #596 is not an ancestor of this branch, so merge_ir_indexed_element_type and merge_ir_assoc_value_type here still call codegen_repr() directly. Not one of tonight's failures, and I haven't touched it.

[mirchaemanuel]

CI has finished on 4690681fa and confirms the picture from my previous comment, on all three architectures rather than just my local macOS aarch64 — so that caveat is now closed.

Red checks went from 20 to 10, and the remaining failures are exactly the three known ones, with nothing new introduced:

codegen::regressions::arrays::test_array_reassign_after_function_growth
codegen::runtime_gc::regressions::test_regression_408_string_key_promotion_does_not_leak
codegen::runtime_gc::regressions::test_regression_escaped_caught_exception_retained_once

So the pushed fix cleanly resolves the codegen_repr() collapse (resources, unions, the exception crash) with no fallout, and what's left is precisely the two design-level causes: the fixed point feeding itself through global signature resolution, and the storage contract not being honoured by the element-write paths.

Nothing further from me here — the decision on whether to revert to 16e4caa0e or complete the contract is yours.

[mirchaemanuel]

Correction to my earlier comment, plus three more fixes pushed (07e98b9d2). Please read the correction before acting on anything I wrote before — I got a mechanism wrong about your code.

Retraction: the element-write paths are fine

I wrote that the storage contract "isn't honoured by the write side", that $keep[] = $e pushes a raw object pointer into a boxed-payload array, and that coerce_container_to_mixed_payload was the right tool never being called. That was wrong.

Codegen already boxes on push: array_push_value_needs_mixed_box (src/codegen/lower_inst/arrays.rs:1237) is true exactly when the element type is Mixed and the value isn't, and routes to lower_mixed_array_push_*. The raw Heap(Object) operand I read off the EIR gets boxed later, during lowering to assembly. And coerce_container_to_mixed_payload is a container-level conversion (ArrayToMixed/HashToMixed), not an element-boxing tool — it was never the relevant instrument.

To be precise about the scope of the retraction: I verified the array_push path directly. I have not audited ref-assign, list-unpack or foreach-by-ref, so I'm not claiming those are fine — I'm withdrawing the claim that a gap exists, because the evidence I based it on doesn't say that. There was no "audit every write path" job, and the "missing half of the contract" framing was mine, not something the code supports.

What the two failures actually were

408 leak — fixed, ae282d40a. Your rewrite dropped a guard. apply_loop_storage_contracts ends in _ => ctx.set_local_type(&name, target_ty), which re-declares a local's type without converting its value. For $g the contract is Array(Mixed) while the slot actually holds a hash (the checker's real type is AssocArray, from $g["a"]="x"), so the fall-through left a hash in the slot with every later read typed array<mixed>. That pushed the promotion from the assignment into the element-write site, where lower_string_key_array_promotion releases the old slot value before array_to_hash reads it and stores the result without a retain — the 5005/5004. The pre-rewrite code had exactly this guard: if array_value.ir_type != IrType::Heap(IrHeapKind::Array) { continue; }. The fix makes the fall-through a no-op, so a contract the representation can't hold leaves the local alone.

The escaped-exception segfault was NOT a regression from your commit — fixed, 38418f87c. I attributed it to 33fa7a377; that was wrong too. lower_mixed_method_candidate_call emitted a class-vtable dispatch for every candidate, but builtin Throwables implement getMessage as a compact intrinsic and leave their vtable slots empty, so the blr hits a null slot. The direct-receiver path guards this with is_throwable_standard_method_call (lower_inst.rs:3081); the Mixed path never did. It now routes to lower_throwable_standard_method_from_reg, which already existed for precisely this case.

It reproduces on the green 16e4caa0e, no loop involved:

$keep = []; $keep[] = 1; $keep[] = new LogicException("boom");
$m = $keep[1]; echo $m->getMessage();   // SIGSEGV on both heads

Your commit only made it reachable from that test, by widening $keep to array<mixed>.

That test is still red, for a different and legitimate reason

It no longer crashes — correct output 5|4, exit 0, heap completely clean (allocs=25 frees=25 live_blocks=0). It fails because it asserts live_blocks=10, pinning the documented baseline where main's epilogue leaves escaped throwables live rather than deep-freeing array object elements.

The two shapes now disagree: with try/catch ($keep is array<mixed>, boxed cells) the epilogue deep-frees and we get 0; without try ($keep is array<LogicException>, raw pointers) we still get 10. The invariant that test pins is genuinely broken by the Array(Mixed) contract. Making them agree means either not widening $keep — the catch binding is modelled Opaque -> Mixed, which is lattice semantics — or teaching the epilogue to deep-free raw object elements, which is new behaviour well beyond this PR. I left the assertion untouched. Relaxing it would be making CI green by weakening a test, and whether that divergence is acceptable is your design call.

Fix B, and a trade-off you should weigh — 07e98b9d2

This restores the previous signature on the failure paths, so a failed body check stops leaving params=[Mixed] -> Int in checker.functions. It does not fix test_array_reassign_after_function_growth. It changes how it fails, and not unambiguously for the better: the silent miscompile becomes a spurious error[4:9]: array_push() first argument must be array on a valid PHP program.

I kept it because a poisoned global signature isn't confined to this test — any later resolution can read it — and a loud error is diagnosable where a wrong count() isn't. But "my valid program no longer compiles" is a harder user-visible regression than "wrong output", and you may weigh it differently. It's the last commit, so dropping it is one clean revert.

Cause 2 itself is untouched: grow is still resolved with Mixed, and I did not go near the widening lattice.

Numbers, on the exact pushed head

six target tests	4 passed, 2 failed (was 3/3, originally 0/6)
runtime_gc + regressions + arrays + exceptions	810 passed, 2 failed, 1 ignored
error_tests	1109 passed, 0 failed

The only failures anywhere are the two discussed above. Unverified and stated as such: macOS aarch64 only, and I did not run the full 6771-test binary — CI covers both better than I can.

The recommendation from my previous comment stands unchanged, and so does the fact that it's yours to make: revert to the green 16e4caa0e and let the fixed-point generalization return as its own PR, or complete the contract. What's changed is that two of the three causes are now closed, one of them being a pre-existing crash that had nothing to do with this PR.

[mirchaemanuel]

One addendum, because my correction above was too generous in one place and I'd rather over-correct than leave you with a wrong impression twice.

I said I was "withdrawing the claim that a gap exists". That's too broad. A narrower defect does exist, and it is not repaired by anything I pushed:

lower_string_key_array_promotion (src/ir_lower/stmt/mod.rs:990) releases the old slot value via prepare_mutated_local_owner before the Op::ArrayToHash that reads that same storage, and stores the resulting hash without a retain. That is the actual +1 in the 5005/5004. It's a refcount-ordering defect, not the payload-representation one I originally described — different in kind, and much narrower.

ae282d40a does not fix it. It stops the contract from misdeclaring the local's representation, so execution no longer reaches that path in this scenario. The defect stays latent in the code. I have not established whether it's reachable by any other route — on the green head, local string-key promotion always happens at the assignment instead — so "latent, reachability not established" is the honest status, and it's probably worth its own issue rather than being considered closed by this PR.

Also, to be exact about coverage rather than implying more than I checked: array_push_value_needs_mixed_box is the append opcode's own guard, called from four sites inside indexed-append lowering and nothing else. So my disproof applies to indexed append specifically. The string-key promotion path (ArrayToHash + HashSet), ref-assign, list-unpack and foreach-by-ref are unverified — not inspected and found correct.

[mirchaemanuel]

I dropped the provisional-signature commit — it broke a test that was passing. Branch is now 38418f87c (force-pushed to remove 07e98b9d2; nothing of yours was touched).

CI on 07e98b9d2 failed a fourth test that was green before I pushed:

codegen::callables::expr_calls::test_closure_fetched_from_object_property_through_method_runs

Bisected: green at 38418f87c, ae282d40a and 4690681fa, red only with the signature commit on top. So it wasn't just the known case producing a spurious error — it stopped a previously-compiling program from compiling. That's the objection I flagged as "a legitimate reading" in my last comment, and it turned out to be the correct one, so I've acted on it rather than leaving you to.

My mistake in process, worth stating: I verified those commits on four surfaces I'd picked because that's where the failing tests were, when the surface I should have measured was everything the change could touch — and a change to signature resolution can touch any call site. codegen::callables was the obvious miss. I said in my previous comment that I hadn't run the full binary; declaring a limit isn't the same as covering it.

State on 38418f87c, all macOS aarch64:

six target tests	4 passed, 2 failed
runtime_gc + regressions + arrays + exceptions	810 passed, 2 failed, 1 ignored
codegen::callables	400 passed, 0 failed
error_tests	1109 passed, 0 failed

Dropping the commit costs nothing: the six target tests are 4/2 either way, so it was pure cost with no benefit. What remains is exactly the two failures I described before, both design calls for you, and test_array_reassign_after_function_growth goes back to failing as the silent miscompile rather than as a compile error — which is worse in kind, and is one more reason the underlying cause 2 needs a real answer rather than a patch at the error path.

The two commits still on the branch are the ones that stand on their own: ae282d40a (the dropped heap-kind guard, fixes the #408 leak) and 38418f87c (the pre-existing builtin-Throwable vtable segfault, which reproduces on the green head and is worth keeping whatever you decide here).