fix: match with heterogeneous arm types no longer coerces every arm to one unified type#492
Open
mirchaemanuel wants to merge 5 commits into
Open
Conversation
…tagged-scalar path
This was referenced Jul 7, 2026
The checker's arm merge typed both null literals and throw arms as Void and made them defer to the other arm's type, so a match returned through an inferred return type coerced the null arm's value (NULL|string became string|string). Throw arms are now normalized to Never at the merge call site and genuine null arms widen the merge to a nullable union, mirroring the lowered temp's nullable-aware merge. Also adds review coverage: heterogeneous arm values consumed (not just gettype-probed), float/int arms, and a nested heterogeneous match arm.
…ties int/bool arms and indexed-array arms with different element types merge to one runtime representation, diverging from PHP; moved that claim out of the control-structures match section's "matching PHP" sentence and into the known-incompatibilities list. The int/bool divergence is pinned by test_match_int_and_bool_arms_merge_documented_divergence.
Contributor
Author
|
Pushed three follow-up commits after a multi-agent review of this branch:
Two confirmed pre-existing items intentionally not addressed here (both reproduce at merge-base too, via the shared boxing/merge paths):
Happy to open follow-up issues for either if useful. |
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.
Fixes #488.
Root cause
lower_matchtyped its hidden merge temp with the syntactic expression-type join (fallback_expr_type→infer_expr_type_syntactic), whosewider_type_syntacticfold is scalar-biased: anyStroperand collapses the whole join toStr(and it can never produceMixed). With arms{object, array, string}the temp becameStr, sostore_expr_into_temp→coerce_value_for_tempemitted anOp::Castto string for the object arm — the runtime fatal from the issue — and a silent string coercion for the array arm. The full pairwise matrix from the issue reproduced exactly (Str absorbs everything; otherwise the last-folded arm's concrete type wins).The type checker's
ExprKind::Matchinference had an independent bug in the same construct: it returned the first arm's type and ignored every other arm, which is why the heterogeneous match with an inferred return type failed to compile (runtime_call from PHP type Str to PHP type Object) — two different wrong unifications colliding.The ternary lowering already solves this correctly via
branch_merge_result_type→wider_type_for_merge, which merges heterogeneous heap types toMixedso each branch value is boxed as-is (Op::MixedBox) instead of cast. This PR bringsmatchto that exact model.Changes
src/ir_lower/expr/mod.rs— newmatch_merge_result_type()sitting next tobranch_merge_result_type(): foldsmaterialized_expr_type_for_mergeover all arms + default withnullable_aware_branch_merge_type, keeps nullable merges, and reconciles non-nullable merges with the syntactic fallback viawider_type_for_merge(which can only keep or widen — the Str-absorbing fallback can no longer narrow the temp).lower_matchnow uses it for the hidden temp.materialized_expr_type_for_mergealso gains an explicitExprKind::Matcharm (mirroring the existingTernaryarm) so a match nested inside a ternary/coalesce merge uses the same rule instead of falling into the syntactic catch-all.src/types/checker/inference/expr/mod.rs—ExprKind::Matchinference now folds all arm types withmerge_match_arm_result_type(): identical arms keep their type,Never/Voidarms (throw/null) defer to the other arms, any other heterogeneous pair widens toMixed. This keeps the checker and EIR lowering in agreement for inferred return types and assignments.src/codegen/lower_inst/builtins.rs—lower_gettypenow dispatches onty.codegen_repr()instead of the raw PHP type. A nullable-int union (null|int) stores an inline tagged scalar, but the raw-type dispatch matchedUnion(_)and routed it to the boxed-Mixed path, calling__rt_mixed_unboxon a non-pointer payload → segfault. This was a pre-existing hole (the ternary twin$v = ($argc == 1) ? 1 : null; echo gettype($v);segfaults on currentmain), but the match fix newly routed top-levelmatchint/null results into it, so it is fixed here with regression tests for both constructs. Verified under both--null-repr=sentineland--null-repr=tagged.tests/codegen/control_flow/match_expressions.rs(new) — 13 tests: the issue repro, every pairwise arm mix from the characterization matrix (object/string both orders, object/array, object/int, array/int, string/int, object/null), top-level and inferred-return-type shapes, homogeneous-arm and throw-default no-regression guards, a heap-debug balance test, and the gettype tagged-scalar regression. Plustest_ternary_int_null_gettypeinternary.rsfor the pre-existing ternary crash.docs/php/control-structures.md— documents heterogeneous arm behavior (boxedmixedresult, per-arm runtime types, same-representation merges).Behavior after the fix
object|stringobject|stringarray|array(silent)object|arrayobject|arrayobject|integerobject|integerinteger|integer(silent)array|integerarray|integerstring|string(silent)string|integerstring|integerobject|object(silent)object|NULLobject|NULLgettypeinteger/ segfault (ternary)integer|NULLinteger|NULLValidation
control_flowsuite).match(114),ternary(36),enum(46),coalesce(52),gettype(16),error_tests match(21).--ir-opt=on|off(byte-identical outputs), declared?stdClassreturns, throw arms inside try/catch, and heap balance — no new leaks and no checker/lowering disagreement found.Adjacent pre-existing findings (out of scope, kept out of this PR)
mixed_boxproducer reference), pending in fix(ir): release the producer's reference when boxing an owned value as Mixed (#484) #489; the behavior here is byte-for-byte identical to the ternary twin.Mixed | Union(_)dispatch pattern that caused the gettype crash appears in a dozen other builtin emitters insrc/codegen/lower_inst/builtins.rs; only the gettype instance (the one this fix routes values into) is fixed here. Filed separately with the site list.null-typed arms at inferred-return boundaries still erase nullability at the checker level (object|NULLprintsobject|objectwith no declared return type) — identical for ternary and unchanged by this PR; filed separately.