fix(ir_lower): widen mismatched array/array branch merges to boxed mixed - #583
Merged
nahime0 merged 4 commits intoJul 22, 2026
Merged
Conversation
Match, ternary, `??`, and `??=` merges of indexed arrays (or hashes) with different element types let the last arm's type win wholesale, so the other arm's runtime slots were relabeled and read through the wrong element repr — SIGSEGV when array<int> was read as array<string>. `wider_type_for_merge` now merges container types elementwise, reusing the heterogeneous-literal rules, and the merge-temp store path boxes typed container slots via ArrayToMixed / HashToMixed. Borrowed sources retain the payload first so the conversion's copy-on-write split leaves live caller arrays untouched; whole-boxed `?array` cells flowing through `??` route through the owned-payload unbox coercion for the same reason. Null-container sentinels forwarded by missed reads pass through both conversions unconverted (issue illegalstudio#533 convention). Closes illegalstudio#549
This was referenced Jul 21, 2026
Closed
Contributor
Author
|
Follow-ups from the adversarial review of this fix, all verified pre-existing on |
Member
|
Maintainer follow-up after the current-main refresh:
Local validation on the refreshed branch:
GitHub now sees head |
…erge-widen # Conflicts: # CHANGELOG.md
nahime0
pushed a commit
that referenced
this pull request
Jul 26, 2026
… instead of bare mixed The type checker collapsed a heterogeneous-element match/ternary/`??` array merge (e.g. `[1, 2]` vs `["a", "b"]`) to bare `Mixed`, so PHP-valid array uses of the result were rejected at compile time: passing it to a by-ref `array` parameter, `in_array()`/`array_sum()`, and spread (`[...$r]`). The spread rejection additionally emitted a misleading follow-on "Undefined variable" diagnostic naming the assignment target. `merge_match_arm_result_type` now mirrors the lowering-side `wider_type_for_merge` array/array and assoc/assoc rules through a new pure `merge_array_branch_types` helper: differing element (or key/value) types widen elementwise to `Mixed`, so the merged value keeps an `array<mixed>` type and array operations type-check just as they do for homogeneous merges. Non-array merges are untouched — scalar unions stay `Mixed` and object|null merges stay nullable unions — and an indexed-vs-associative mix still falls through to `Mixed`, matching lowering. Runtime correctness of heterogeneous merges is tracked by #549 / PR #583; this change only aligns the static type. The residual "Undefined variable" cascade for genuinely-failing spreads (repro: `$x = 5; $s = [...$x]; count($s);`) is a separate latent defect left for a follow-up. Claude-Session: https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #549 —
matcharms (and ternary /??/??=merges) returning indexed arrays with different element types merged to a single element type; reading the other arm's array through the merged type misinterpreted the slot bytes (SIGSEGV on currentmainwhen the int arm is read as strings).Root cause
Two cooperating gaps:
wider_type_for_mergeresolved(Array, Array)and(AssocArray, AssocArray)asright.clone()— the last arm's element type won wholesale.coerce_value_for_tempperformed no container conversion, so the other arm's runtime array was stored raw and merely relabeled; element reads then decoded the wrong slot width/content (int slots read as string{ptr,len}pairs → fault in__rt_str_persist).Fix
wider_type_for_mergenow merges array/array and assoc/assoc elementwise, reusing the heterogeneous-literal rules (merge_ir_indexed_element_type/merge_ir_assoc_value_type): equal reprs keep the type,Never/Voidyields the other side, any mismatch widens to boxedmixed.widen_container_value_for_temphelper: typed containers stored into Mixed-payload temps box their slots via the existingArrayToMixed/HashToMixedops. Borrowed and provisionally-owned sources are retained first so the conversion's copy-on-write split rewrites a private copy (per the PR fix(ir): release the producer's reference when boxing an owned value as Mixed (#484) #489 ownership rules); whole-boxed?arraycells flowing through??unbox with the owned-payload coercion before converting when the cell does not own its payload, so a caller's live array is never rewritten in place.lower_array_to_mixed/lower_hash_to_mixedpass null and in-band null-container sentinels through unconverted on both targets (call-site guard, issue fix(codegen): guard container consumers against the null-container sentinel (chained-read miss segfault) #533 convention), so a missed read forwarded by an arm is never dereferenced.This fixes match, full ternary, short ternary,
??, and??=in one place — they share the same merge/store helpers. Target-neutral: AArch64 and x86_64 emissions for both guards.Tests
18 regression tests in
tests/codegen/control_flow/{match_expressions,ternary,nulls}.rs: both arm orders for match and ternary, assoc value-type mismatch, empty-array arm, nested arrays, variable (borrowed) arms, missed-read sentinel forwarding, and??with a?arrayvalue side — owning and borrowed cells, including a live caller array passed twice and a 20-iteration loop — with heap-clean assertions under--heap-debug. Focused neighbor suites are green locally on macOS ARM64 (match 151, ternary 45, coalesce 63, nulls 32, plus array/hash/assoc/spread/optimizer filters, ~1,600 tests). The issue's repro prints PHP-parity output on both arms.Notes
$r === nullwhere PHP seesnull) is pre-existing (Checker narrows ?T after instanceof but not after an === null guard with early exit #508 family) and unchanged here — this PR guarantees crash-freedom and??-fallback parity for that shape.