From c7831bd8b60a0bfbf4bfed805d082420fe0cc9ce Mon Sep 17 00:00:00 2001 From: mirchaemanuel Date: Mon, 6 Jul 2026 12:26:37 +0200 Subject: [PATCH 1/2] fix(types,ir): loop-grown arrays widen to mixed before the body is processed Fixes #452. Both the type checker and EIR lowering process loop bodies in a single pass, so with $vals[] = 1; $vals[] = 2.0; in a loop the earlier push site was typed/lowered against the pre-promotion element type (array/ array) and emitted a raw scalar push. On iterations >= 2 the back edge brings the promoted array around, so the raw push wrote an unboxed scalar into boxed-cell storage; reading that element dereferenced the scalar as a Mixed cell pointer (SIGSEGV), or silently corrupted values and leaked in the string/float flavour. A shared prescan (types::checker::loop_widening) collects the body's $name[] = value pushes recursively and reports locals whose element type joins to mixed. The checker widens those locals in the TypeEnv before checking each loop body; EIR lowering widens its local_types and materializes the promotion once before the loop (in-place ArrayToMixed + storeback), so every push site boxes its value. Same-type pushes and never->T growth keep their current lowering (no action unless the join is mixed). The foreach prescan binds the loop value/key variables to their real element types so a push of the loop variable does not over-widen. Regression tests in tests/codegen/array_basics.rs (element read, foreach sum, int+string flavour, float-first ordering). (cherry picked from commit c3b449bae8a7e4eb10852b6d997dc56e65c35f0c) --- CHANGELOG.md | 1 + src/ir_lower/stmt/mod.rs | 75 ++++++++ src/types/checker/loop_widening.rs | 172 +++++++++++++++++++ src/types/checker/mod.rs | 2 + src/types/checker/stmt_check/control_flow.rs | 21 +++ tests/codegen/array_basics.rs | 111 ++++++++++++ 6 files changed, 382 insertions(+) create mode 100644 src/types/checker/loop_widening.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index b31f875751..57dcff7029 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to elephc, a PHP-to-native compiler written in Rust. Releases are listed newest first. ## [Unreleased] +- Fixed a heap corruption when an array grown inside a loop is promoted to mixed-element storage (issue #452): with `$vals[] = 1; $vals[] = 2.0;` in a loop body, the earlier push site was typed and lowered against the pre-promotion element type, so from the second iteration it wrote an unboxed scalar into boxed-cell storage — reading that element produced garbage or a segfault. Loop bodies now pre-scan their push sites and fix the array's element type to `mixed` up front (checker and EIR lowering), materializing the promotion once before the loop so every push boxes correctly. Also fixes the silent value corruption and per-conversion leak of the string/float flavour of the same pattern. - Fixed a constant-propagation miscompile with by-reference calls in a `match` subject (issue #384): `echo match(bump($i)) { ... } . "|" . $i` kept the pre-call constant for `$i` and printed the stale value, because a call's unknown write set was treated as "no writes" instead of forcing conservative invalidation. A read sequenced after any by-reference-mutating call in the same expression now observes the post-call value, matching PHP; pure calls (`gettype`, `strlen`, …) keep their operands foldable. - Fixed PHP parser/codegen parity for parenthesis-free object instantiation (issue #371): `new Foo`, `new self`, `new static`, `new parent`, and `new $class` now compile with empty constructor arguments, while immediate postfix forms such as `new Foo->bar`, `new Foo::bar()`, `new Foo?->bar`, and `new Foo[0]` are rejected instead of being misparsed. - Fixed three PHP parity regressions in the EIR backend: indexed array elements such as `$a[0]` can now be passed to by-reference parameters with copy-on-write storage split before mutation (issue #360); `IteratorAggregate::getIterator()` may declare the marker `Traversable` return type while `foreach` still dispatches through the returned object's concrete `Iterator` methods (issue #385); and catchable private/protected method access plus readonly-property write errors now preserve PHP's receiver/RHS evaluation order before throwing `Error` (issue #383). The merge also removes a duplicate `_spl_error_class_id` data symbol that could make post-merge user assembly fail to assemble. diff --git a/src/ir_lower/stmt/mod.rs b/src/ir_lower/stmt/mod.rs index bef17b5bba..3899631f93 100644 --- a/src/ir_lower/stmt/mod.rs +++ b/src/ir_lower/stmt/mod.rs @@ -446,7 +446,68 @@ fn lower_ifdef( } /// Lowers a `while` loop. +/// Widens locals whose indexed-array element type joins to `mixed` across the loop body's +/// push sites (issue #452) and materializes the promotion once before the loop. Loop bodies +/// are lowered in a single pass, so without this an early `$a[] = ` site is emitted +/// as a raw push against the pre-promotion element type even though the back edge brings the +/// promoted `array` around, writing an unboxed scalar into boxed-cell storage on +/// iterations >= 2. Converting the array up front (in place, same pointer) and fixing its +/// type to `array` makes every push site box its value. `overrides` supplies types +/// for names bound by the loop itself (the `foreach` value/key variables) so a push of the +/// loop variable joins with its real element type. Locals without a materialized slot yet +/// (first assigned inside the body, hence fresh every iteration) are left untouched. +fn widen_loop_grown_arrays( + ctx: &mut LoweringContext<'_, '_>, + body: &[Stmt], + update: Option<&Stmt>, + overrides: &[(&str, PhpType)], + span: Option, +) { + let names = { + let lookup = |name: &str| -> Option { + if let Some((_, ty)) = overrides.iter().find(|(n, _)| *n == name) { + return Some(ty.clone()); + } + Some(ctx.local_type(name)) + }; + crate::types::checker::loop_grown_mixed_array_pushes(body, update, &lookup) + }; + for name in names { + if !ctx.local_slots.contains_key(&name) { + continue; + } + let array_value = ctx.load_local(&name, span); + if array_value.ir_type != IrType::Heap(crate::ir::IrHeapKind::Array) { + continue; + } + let mixed_array_ty = PhpType::Array(Box::new(PhpType::Mixed)); + let converted = ctx.emit_value( + Op::ArrayToMixed, + vec![array_value.value], + None, + mixed_array_ty.clone(), + Op::ArrayToMixed.default_effects(), + span, + ); + ctx.store_mutated_local(&name, converted, mixed_array_ty, span); + } +} + +/// Mirrors the checker's `foreach` value binding for the loop-widening prescan: the element +/// type for an indexed/associative array source variable, `mixed` otherwise. +fn foreach_prescan_value_type(ctx: &LoweringContext<'_, '_>, array: &Expr) -> PhpType { + let ExprKind::Variable(name) = &array.kind else { + return PhpType::Mixed; + }; + match ctx.local_type(name) { + PhpType::Array(elem) => *elem, + PhpType::AssocArray { value, .. } => *value, + _ => PhpType::Mixed, + } +} + fn lower_while(ctx: &mut LoweringContext<'_, '_>, condition: &Expr, body: &[Stmt]) { + widen_loop_grown_arrays(ctx, body, None, &[], Some(condition.span)); let header = ctx.builder.create_named_block("while.cond", Vec::new()); let body_block = ctx.builder.create_named_block("while.body", Vec::new()); let exit = ctx.builder.create_named_block("while.exit", Vec::new()); @@ -479,6 +540,7 @@ fn lower_while(ctx: &mut LoweringContext<'_, '_>, condition: &Expr, body: &[Stmt /// Lowers a `do while` loop. fn lower_do_while(ctx: &mut LoweringContext<'_, '_>, body: &[Stmt], condition: &Expr) { + widen_loop_grown_arrays(ctx, body, None, &[], Some(condition.span)); let body_block = ctx.builder.create_named_block("do.body", Vec::new()); let cond_block = ctx.builder.create_named_block("do.cond", Vec::new()); let exit = ctx.builder.create_named_block("do.exit", Vec::new()); @@ -523,6 +585,10 @@ fn lower_for( if ctx.builder.insertion_block_is_terminated() { return; } + let widen_span = condition + .map(|c| c.span) + .or_else(|| body.first().map(|s| s.span)); + widen_loop_grown_arrays(ctx, body, update, &[], widen_span); let header = ctx.builder.create_named_block("for.cond", Vec::new()); let body_block = ctx.builder.create_named_block("for.body", Vec::new()); @@ -1037,6 +1103,15 @@ fn lower_foreach( value_by_ref: bool, body: &[Stmt], ) { + // Widen before the source is lowered so an iterated-and-pushed array is loaded with + // its fixed-point element type, and bind the loop variables' prescan types so a push + // of the foreach value joins with its real element type. + let prescan_value_ty = foreach_prescan_value_type(ctx, array); + let mut overrides: Vec<(&str, PhpType)> = vec![(value_var, prescan_value_ty)]; + if let Some(key_var) = key_var { + overrides.push((key_var, PhpType::Mixed)); + } + widen_loop_grown_arrays(ctx, body, None, &overrides, Some(array.span)); let source = lower_expr(ctx, array); let source_php_ty = ctx.builder.value_php_type(source.value); let source_ty = source_php_ty.codegen_repr(); diff --git a/src/types/checker/loop_widening.rs b/src/types/checker/loop_widening.rs new file mode 100644 index 0000000000..95df6901a6 --- /dev/null +++ b/src/types/checker/loop_widening.rs @@ -0,0 +1,172 @@ +//! Purpose: +//! Pre-scans a loop body for `$array[] = value` pushes whose element types widen a +//! local indexed array to `mixed` across the loop back-edge (issue #452), so both the +//! type checker and EIR lowering can fix the array's element type to `mixed` *before* +//! processing the body once. +//! +//! Called from: +//! - `crate::types::checker::stmt_check::control_flow` (loop arms widen the `TypeEnv`). +//! - `crate::ir_lower::stmt` (loop lowering widens `local_types` and materializes the +//! promotion before emitting the body). +//! +//! Key details: +//! - Both passes are single-pass over loop bodies; without this scan an early push site +//! is typed/lowered against the pre-promotion element type and writes an unboxed +//! scalar into mixed-element storage on iterations >= 2, corrupting the heap. +//! - Only the widening-to-`mixed` transition is reported: it is the one that changes the +//! element representation (raw scalar slots vs boxed cells). Same-type pushes and +//! `never -> T` growth keep their current lowering. +//! - Value types resolve syntactically (`infer_expr_type_syntactic`); variables resolve +//! through the caller-supplied lookup so loop-bound names (e.g. a `foreach` value +//! variable) can be given their real type. Unknown values conservatively join as +//! `mixed`, which matches what a single push site of that value would do itself. + +use crate::parser::ast::{Expr, ExprKind, Stmt, StmtKind}; +use crate::types::checker::infer_expr_type_syntactic; +use crate::types::PhpType; + +/// Returns the names of locals that currently hold a non-`mixed` indexed array and whose +/// element type joins to `mixed` across every `$name[] = value` push found in the loop +/// body (and the optional `for` update statement). `lookup` supplies the current type of +/// a local at loop entry; names it does not know are skipped as push targets and resolve +/// to `mixed` as pushed values. +pub fn loop_grown_mixed_array_pushes( + body: &[Stmt], + update: Option<&Stmt>, + lookup: &dyn Fn(&str) -> Option, +) -> Vec { + let mut pushes: Vec<(&str, &Expr)> = Vec::new(); + collect_array_pushes(body, &mut pushes); + if let Some(stmt) = update { + collect_array_push_stmt(stmt, &mut pushes); + } + let mut names: Vec = Vec::new(); + for (name, _) in &pushes { + if names.iter().any(|n| n == name) { + continue; + } + let Some(PhpType::Array(elem)) = lookup(name) else { + continue; + }; + if *elem == PhpType::Mixed { + continue; + } + let joined = pushes + .iter() + .filter(|(n, _)| n == name) + .fold(*elem, |acc, (_, value)| { + join_pushed_element_type(acc, resolve_pushed_value_type(value, lookup)) + }); + if joined == PhpType::Mixed { + names.push(name.to_string()); + } + } + names +} + +/// Resolves the static type a pushed value contributes to the element join: variables go +/// through the caller's lookup (defaulting to `mixed` when unknown), everything else uses +/// the shared syntactic inference. +fn resolve_pushed_value_type(value: &Expr, lookup: &dyn Fn(&str) -> Option) -> PhpType { + match &value.kind { + ExprKind::Variable(name) => lookup(name).unwrap_or(PhpType::Mixed), + _ => infer_expr_type_syntactic(value), + } +} + +/// Joins two indexed-array element types on the widening lattice used by push sites: +/// equal types stay, `never` adopts the other side, and any other combination widens to +/// `mixed` (the representation-changing transition this scan exists to detect). +fn join_pushed_element_type(a: PhpType, b: PhpType) -> PhpType { + if a == b { + a + } else if a == PhpType::Never { + b + } else if b == PhpType::Never { + a + } else { + PhpType::Mixed + } +} + +/// Collects `$name[] = value` pushes from every statement in `stmts`, recursively. +fn collect_array_pushes<'a>(stmts: &'a [Stmt], out: &mut Vec<(&'a str, &'a Expr)>) { + for stmt in stmts { + collect_array_push_stmt(stmt, out); + } +} + +/// Collects pushes from one statement, recursing into every nested statement body that +/// executes as part of the enclosing loop iteration. Declaration bodies (functions, +/// classes) do not execute in the loop and closures capture by value by default, so +/// neither is descended into. +fn collect_array_push_stmt<'a>(stmt: &'a Stmt, out: &mut Vec<(&'a str, &'a Expr)>) { + match &stmt.kind { + StmtKind::ArrayPush { array, value } => out.push((array.as_str(), value)), + StmtKind::If { + then_body, + elseif_clauses, + else_body, + .. + } => { + collect_array_pushes(then_body, out); + for (_, clause_body) in elseif_clauses { + collect_array_pushes(clause_body, out); + } + if let Some(else_body) = else_body { + collect_array_pushes(else_body, out); + } + } + StmtKind::IfDef { + then_body, + else_body, + .. + } => { + collect_array_pushes(then_body, out); + if let Some(else_body) = else_body { + collect_array_pushes(else_body, out); + } + } + StmtKind::While { body, .. } + | StmtKind::DoWhile { body, .. } + | StmtKind::Foreach { body, .. } + | StmtKind::IncludeOnceGuard { body, .. } => collect_array_pushes(body, out), + StmtKind::For { + init, + update, + body, + .. + } => { + if let Some(init) = init { + collect_array_push_stmt(init, out); + } + if let Some(update) = update { + collect_array_push_stmt(update, out); + } + collect_array_pushes(body, out); + } + StmtKind::Switch { cases, default, .. } => { + for (_, case_body) in cases { + collect_array_pushes(case_body, out); + } + if let Some(default) = default { + collect_array_pushes(default, out); + } + } + StmtKind::Try { + try_body, + catches, + finally_body, + } => { + collect_array_pushes(try_body, out); + for catch in catches { + collect_array_pushes(&catch.body, out); + } + if let Some(finally_body) = finally_body { + collect_array_pushes(finally_body, out); + } + } + StmtKind::Synthetic(stmts) => collect_array_pushes(stmts, out), + _ => {} + } +} diff --git a/src/types/checker/mod.rs b/src/types/checker/mod.rs index 081d95ced8..4faadddb4f 100644 --- a/src/types/checker/mod.rs +++ b/src/types/checker/mod.rs @@ -26,6 +26,7 @@ mod driver; mod extern_decl; mod functions; mod inference; +mod loop_widening; mod method_pass; mod schema; mod stmt_check; @@ -45,6 +46,7 @@ use crate::types::{ }; pub use inference::{infer_expr_type_syntactic, infer_return_type_syntactic}; +pub(crate) use loop_widening::loop_grown_mixed_array_pushes; pub(crate) use inference::closure_body_uses_this; pub(crate) use builtin_types::InterfaceDeclInfo; use builtin_types::validate_magic_method_contracts; diff --git a/src/types/checker/stmt_check/control_flow.rs b/src/types/checker/stmt_check/control_flow.rs index 187bcc9182..70b602c7ff 100644 --- a/src/types/checker/stmt_check/control_flow.rs +++ b/src/types/checker/stmt_check/control_flow.rs @@ -19,6 +19,21 @@ const FS_CURRENT_AS_PATHNAME: i64 = 32; const FS_CURRENT_MODE_MASK: i64 = 240; const FS_SKIP_DOTS: i64 = 4096; +/// Widens locals whose indexed-array element type joins to `mixed` across the loop body's +/// push sites (issue #452). Loop bodies are checked in a single pass, so without this the +/// entry environment types an early push site against the pre-promotion element type even +/// though the back edge brings the promoted array around; fixing the element type to +/// `mixed` up front makes every push site see the fixed-point type. +fn widen_loop_grown_array_pushes(body: &[Stmt], update: Option<&Stmt>, env: &mut TypeEnv) { + let names = + crate::types::checker::loop_grown_mixed_array_pushes(body, update, &|name| { + env.get(name).cloned() + }); + for name in names { + env.insert(name, PhpType::Array(Box::new(PhpType::Mixed))); + } +} + /// Restores a narrowed variable in the environment to its previously saved type after a guarded /// branch, removing it when it had no prior type. Used to keep `if`/`else` type narrowing scoped /// to its branch. @@ -131,6 +146,9 @@ impl Checker { "by-reference foreach over Iterator/IteratorAggregate objects or iterable-typed values is not supported; use an array source or remove &", )); } + // Widen after the key/value bindings are in the environment so a push of + // the foreach value variable joins with its real element type. + widen_loop_grown_array_pushes(body, None, env); let errors = self.check_break_continue_target_body(body, env); if errors.is_empty() { Ok(()) @@ -257,6 +275,7 @@ impl Checker { } } StmtKind::DoWhile { body, condition } => { + widen_loop_grown_array_pushes(body, None, env); let errors = self.check_break_continue_target_body(body, env); self.infer_type_with_assignment_effects(condition, env)?; if errors.is_empty() { @@ -266,6 +285,7 @@ impl Checker { } } StmtKind::While { condition, body } => { + widen_loop_grown_array_pushes(body, None, env); self.infer_type_with_assignment_effects(condition, env)?; let errors = self.check_break_continue_target_body(body, env); if errors.is_empty() { @@ -283,6 +303,7 @@ impl Checker { if let Some(s) = init { self.check_stmt(s, env)?; } + widen_loop_grown_array_pushes(body, update.as_deref(), env); if let Some(c) = condition { self.infer_type_with_assignment_effects(c, env)?; } diff --git a/tests/codegen/array_basics.rs b/tests/codegen/array_basics.rs index a8dbb78d67..6c943146fd 100644 --- a/tests/codegen/array_basics.rs +++ b/tests/codegen/array_basics.rs @@ -240,6 +240,117 @@ echo $a[0]->n . "|" . $a[5]->n . "|" . count($a); assert_eq!(out, "p0|p5|6"); } +/// Regression for #452: pushing heterogeneous scalars (int then float) into an empty array +/// inside a loop promotes the array to mixed-element storage on iteration 1, but the earlier +/// push site was lowered against the stale pre-promotion type and wrote an unboxed scalar +/// into the mixed array on iteration 2. Reading that element then dereferenced the raw +/// scalar as a boxed cell pointer and crashed. The loop body must see the fixed-point +/// (back-edge) element type so every push site boxes correctly. +#[test] +fn test_loop_grown_mixed_array_element_read() { + let out = compile_and_run( + r#" Date: Mon, 6 Jul 2026 21:24:37 +0200 Subject: [PATCH 2/2] fix(types,ir): loop-widening prescan requires type evidence, not Mixed defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught a false positive: the prescan treated any pushed variable unknown at loop entry as Mixed evidence, spuriously widening same-typed rebuild loops. The compiler-synthesized MultipleIterator::detachIterator body rebuilds an array through a loop-defined variable ($candidate = $this->iterators[$i]; $newIterators[] = $candidate;), and the widened array then failed the typed-property storeback — every program calling detachIterator stopped compiling. Two layers: - loop_widening: a pushed variable now joins its loop-entry type with self-evident literal assignments found inside the body ($x = 1; $a[] = $x still widens); a variable whose only in-loop sources are non-literal yields no evidence and is excluded from the join (poison entries guard partial evidence). Unknown never defaults to Mixed. - ir_lower lookup closure: returned Some(Mixed) for undeclared names via the local_type fallback, so the checker skipped but ir_lower still widened; it now returns None for names without a declared slot, mirroring the checker's env.get. Regression tests: the MultipleIterator detach shape compiles and runs, and the literal-through-variable heterogeneous push still widens (array_basics.rs). Full sweep green including the spl suite (1803 codegen + 993 error tests). --- src/ir_lower/stmt/mod.rs | 7 ++ src/types/checker/loop_widening.rs | 192 +++++++++++++++++++++++++++-- tests/codegen/array_basics.rs | 40 ++++++ 3 files changed, 226 insertions(+), 13 deletions(-) diff --git a/src/ir_lower/stmt/mod.rs b/src/ir_lower/stmt/mod.rs index 3899631f93..c02582152d 100644 --- a/src/ir_lower/stmt/mod.rs +++ b/src/ir_lower/stmt/mod.rs @@ -468,6 +468,13 @@ fn widen_loop_grown_arrays( if let Some((_, ty)) = overrides.iter().find(|(n, _)| *n == name) { return Some(ty.clone()); } + // A name with no declared slot yet (first assigned inside the loop body) is + // genuinely unknown at loop entry: report it as such instead of the Mixed + // fallback `local_type` would return, so the prescan does not take Mixed as + // widening evidence for it (mirrors the checker's `env.get` lookup). + if !ctx.local_slots.contains_key(name) { + return None; + } Some(ctx.local_type(name)) }; crate::types::checker::loop_grown_mixed_array_pushes(body, update, &lookup) diff --git a/src/types/checker/loop_widening.rs b/src/types/checker/loop_widening.rs index 95df6901a6..ce82eade36 100644 --- a/src/types/checker/loop_widening.rs +++ b/src/types/checker/loop_widening.rs @@ -17,19 +17,28 @@ //! element representation (raw scalar slots vs boxed cells). Same-type pushes and //! `never -> T` growth keep their current lowering. //! - Value types resolve syntactically (`infer_expr_type_syntactic`); variables resolve -//! through the caller-supplied lookup so loop-bound names (e.g. a `foreach` value -//! variable) can be given their real type. Unknown values conservatively join as -//! `mixed`, which matches what a single push site of that value would do itself. +//! through the caller-supplied lookup (loop-entry types, e.g. a `foreach` value +//! variable's real element type) joined with self-evident literal assignments found +//! inside the loop body (`$x = 1; $a[] = $x;`). +//! - A pushed value with no usable evidence — a variable defined only inside the loop +//! from non-literal sources — contributes nothing to the join. Treating it as `mixed` +//! would spuriously widen same-typed rebuild loops (e.g. the synthesized +//! `MultipleIterator::detachIterator` body, whose `array` rebuild must stay +//! assignable to its typed property); genuinely-widening pushes of such values are +//! still promoted per-site by the existing lowering. use crate::parser::ast::{Expr, ExprKind, Stmt, StmtKind}; use crate::types::checker::infer_expr_type_syntactic; use crate::types::PhpType; /// Returns the names of locals that currently hold a non-`mixed` indexed array and whose -/// element type joins to `mixed` across every `$name[] = value` push found in the loop +/// element type joins to `mixed` across the `$name[] = value` pushes found in the loop /// body (and the optional `for` update statement). `lookup` supplies the current type of -/// a local at loop entry; names it does not know are skipped as push targets and resolve -/// to `mixed` as pushed values. +/// a local at loop entry; names it does not know are skipped as push targets. A pushed +/// value with no usable type evidence — a variable defined only inside the loop from a +/// non-literal source, such as `$candidate = $this->iterators[$i]` — contributes nothing +/// to the join instead of forcing `mixed`, so same-typed rebuild loops (common in +/// compiler-synthesized SPL method bodies) are not spuriously widened. pub fn loop_grown_mixed_array_pushes( body: &[Stmt], update: Option<&Stmt>, @@ -40,6 +49,11 @@ pub fn loop_grown_mixed_array_pushes( if let Some(stmt) = update { collect_array_push_stmt(stmt, &mut pushes); } + let mut literal_assignments: Vec<(&str, Option)> = Vec::new(); + collect_literal_assignments(body, &mut literal_assignments); + if let Some(stmt) = update { + collect_literal_assignment_stmt(stmt, &mut literal_assignments); + } let mut names: Vec = Vec::new(); for (name, _) in &pushes { if names.iter().any(|n| n == name) { @@ -55,7 +69,10 @@ pub fn loop_grown_mixed_array_pushes( .iter() .filter(|(n, _)| n == name) .fold(*elem, |acc, (_, value)| { - join_pushed_element_type(acc, resolve_pushed_value_type(value, lookup)) + match resolve_pushed_value_type(value, lookup, &literal_assignments) { + Some(pushed) => join_pushed_element_type(acc, pushed), + None => acc, + } }); if joined == PhpType::Mixed { names.push(name.to_string()); @@ -64,13 +81,162 @@ pub fn loop_grown_mixed_array_pushes( names } -/// Resolves the static type a pushed value contributes to the element join: variables go -/// through the caller's lookup (defaulting to `mixed` when unknown), everything else uses -/// the shared syntactic inference. -fn resolve_pushed_value_type(value: &Expr, lookup: &dyn Fn(&str) -> Option) -> PhpType { +/// Resolves the static type a pushed value contributes to the element join, or `None` +/// when there is no usable evidence. A pushed variable joins its loop-entry type with any +/// self-evident literal assignments to it inside the loop body (`$x = 1; $a[] = $x;`); +/// a variable that is only ever assigned from non-literal sources inside the loop yields +/// no evidence — the loop-entry lookup is authoritative and unknown stays unknown, never +/// `mixed`. Non-variable values use the shared syntactic inference as before. +fn resolve_pushed_value_type( + value: &Expr, + lookup: &dyn Fn(&str) -> Option, + literal_assignments: &[(&str, Option)], +) -> Option { + match &value.kind { + ExprKind::Variable(name) => { + let entry = lookup(name); + let body = joined_literal_assignment_type(name, literal_assignments); + match (entry, body) { + (Some(a), Some(b)) => Some(join_pushed_element_type(a, b)), + (Some(a), None) => Some(a), + (None, Some(b)) => Some(b), + (None, None) => None, + } + } + _ => Some(infer_expr_type_syntactic(value)), + } +} + +/// Joins the recorded in-loop literal assignments for `name`. Returns `None` when the +/// variable has no literal assignment or any of its assignments is non-literal (a poison +/// entry) — partial literal evidence must not overrule the unknown non-literal sources. +fn joined_literal_assignment_type( + name: &str, + literal_assignments: &[(&str, Option)], +) -> Option { + let mut joined: Option = None; + for (candidate, ty) in literal_assignments { + if *candidate != name { + continue; + } + let Some(ty) = ty else { + return None; + }; + joined = Some(match joined { + Some(acc) => join_pushed_element_type(acc, ty.clone()), + None => ty.clone(), + }); + } + joined +} + +/// Returns the self-evident scalar type of a literal expression, or `None` for anything +/// that needs real inference (the shared syntactic helper defaults unknown constructs to +/// `Int`, which is not usable as widening evidence). +fn literal_expr_type(value: &Expr) -> Option { match &value.kind { - ExprKind::Variable(name) => lookup(name).unwrap_or(PhpType::Mixed), - _ => infer_expr_type_syntactic(value), + ExprKind::IntLiteral(_) => Some(PhpType::Int), + ExprKind::FloatLiteral(_) => Some(PhpType::Float), + ExprKind::StringLiteral(_) => Some(PhpType::Str), + ExprKind::BoolLiteral(_) => Some(PhpType::Bool), + _ => None, + } +} + +/// Collects `$name = ` assignments from every statement in `stmts`, recursively, +/// recording a poison entry (`None` type) for non-literal assignments so partial evidence +/// never overrules an unknown source. +fn collect_literal_assignments<'a>(stmts: &'a [Stmt], out: &mut Vec<(&'a str, Option)>) { + for stmt in stmts { + collect_literal_assignment_stmt(stmt, out); + } +} + +/// Collects literal assignments from one statement, mirroring the nested-statement +/// recursion of `collect_array_push_stmt` (only bodies that execute within the loop +/// iteration are descended into). +fn collect_literal_assignment_stmt<'a>(stmt: &'a Stmt, out: &mut Vec<(&'a str, Option)>) { + match &stmt.kind { + StmtKind::Assign { name, value } | StmtKind::TypedAssign { name, value, .. } => { + out.push((name.as_str(), literal_expr_type(value))); + } + StmtKind::If { + then_body, + elseif_clauses, + else_body, + .. + } => { + collect_literal_assignments(then_body, out); + for (_, clause_body) in elseif_clauses { + collect_literal_assignments(clause_body, out); + } + if let Some(else_body) = else_body { + collect_literal_assignments(else_body, out); + } + } + StmtKind::IfDef { + then_body, + else_body, + .. + } => { + collect_literal_assignments(then_body, out); + if let Some(else_body) = else_body { + collect_literal_assignments(else_body, out); + } + } + StmtKind::While { body, .. } + | StmtKind::DoWhile { body, .. } + | StmtKind::IncludeOnceGuard { body, .. } => collect_literal_assignments(body, out), + StmtKind::Foreach { + key_var, + value_var, + body, + .. + } => { + // The foreach bindings assign non-literal values each iteration: poison them. + if let Some(key_var) = key_var { + out.push((key_var.as_str(), None)); + } + out.push((value_var.as_str(), None)); + collect_literal_assignments(body, out); + } + StmtKind::For { + init, + update, + body, + .. + } => { + if let Some(init) = init { + collect_literal_assignment_stmt(init, out); + } + if let Some(update) = update { + collect_literal_assignment_stmt(update, out); + } + collect_literal_assignments(body, out); + } + StmtKind::Switch { cases, default, .. } => { + for (_, case_body) in cases { + collect_literal_assignments(case_body, out); + } + if let Some(default) = default { + collect_literal_assignments(default, out); + } + } + StmtKind::Try { + try_body, + catches, + finally_body, + } => { + collect_literal_assignments(try_body, out); + for catch in catches { + collect_literal_assignments(&catch.body, out); + } + if let Some(finally_body) = finally_body { + collect_literal_assignments(finally_body, out); + } + } + StmtKind::Synthetic(stmts) => collect_literal_assignments(stmts, out), + _ => {} } } diff --git a/tests/codegen/array_basics.rs b/tests/codegen/array_basics.rs index 6c943146fd..d5b3532ce2 100644 --- a/tests/codegen/array_basics.rs +++ b/tests/codegen/array_basics.rs @@ -1075,3 +1075,43 @@ echo (in_array("hello", $a) ? "y" : "n"), ); assert_eq!(out, "yyn"); } + +/// Regression: the loop-widening prescan must not treat a variable defined only inside +/// the loop from a non-literal source as `mixed` evidence. The compiler-synthesized +/// `MultipleIterator::detachIterator` body rebuilds an `array` through such a +/// variable; a spurious widen made its typed-property storeback fail to compile. +#[test] +fn test_loop_rebuild_of_typed_array_not_spuriously_widened() { + let out = compile_and_run( + r#"attachIterator($it); +$multi->detachIterator($it); +echo $multi->countIterators(); +"#, + ); + assert_eq!(out, "0"); +} + +/// Regression companion for #452: heterogeneous scalars carried through loop-defined +/// variables (literal assignments inside the body) must still widen the pushed array — +/// the literal-assignment scan supplies the evidence the loop-entry lookup lacks. +#[test] +fn test_loop_grown_mixed_array_via_local_literals() { + let out = compile_and_run( + r#"