Skip to content

fix(optimize): conservative invalidation for by-ref calls in match subject (#384)#444

Merged
nahime0 merged 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/384-match-subject-byref-writeback
Jul 6, 2026
Merged

fix(optimize): conservative invalidation for by-ref calls in match subject (#384)#444
nahime0 merged 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/384-match-subject-byref-writeback

Conversation

@mirchaemanuel

Copy link
Copy Markdown
Contributor

Summary

Fixes #384. A by-reference call used as a match subject lost its writeback for constant-propagation purposes: echo match(bump($i)) { 1 => "one", default => "other" } . "|" . $i; printed one|0 instead of PHP's one|1.

Root cause

This is an AST constant-propagation bug, not a lowering/codegen one. With --emit-ir --no-ir-opt the trailing $i is already emitted as const_i64 0, so the stale value is folded before EIR.

In src/optimize/propagate/expr.rs, propagate_expr cleared the constant environment only when expr_local_writes(&expr) returned Some(non-empty). A call expression returns None (unknown write set), and None was treated as "no writes" — so the pre-call constant $i = 0 was propagated into the read sequenced after the match. The same call in an arithmetic expression or if condition works only because statement-level side-effect clearing (env_after_expr_side_effects) invalidates across statement boundaries; within a single expression there was no invalidation.

This also contradicts the module's own documented contract: "unknown calls force conservative invalidation."

Fix

propagate_expr now clears the environment when the write set is unknown (None) as well as when it is a known non-empty set. A read sequenced after any by-reference-mutating call in the same expression therefore observes the post-call value, matching PHP.

Tests

  • New regression test test_constant_propagation_match_subject_byref_writeback in tests/codegen/optimizer/constant_propagation/straight_line.rs (fails one|0 before, passes one|1 after).
  • No regressions: 32 propagate unit tests, 242 optimizer codegen tests, 1093 regressions/types/callables/control codegen tests — all green.
  • cargo build warning-free; git diff --check clean.

…bject

Fixes illegalstudio#384. Constant propagation treated a call's unknown local-write set (None)
as 'no writes', so a variable mutated by a by-reference call used as a match
subject kept its pre-call constant for a later read in the same expression:
echo match(bump($i)) {..} . '|' . $i printed the stale value.

propagate_expr now clears the environment when the write set is unknown AND the
expression has side effects (a by-reference-mutating call). Pure calls (gettype,
strlen, ...) cannot mutate a local, so their operands stay foldable — this keeps
existing folds intact (e.g. gettype($a * $b) still constant-folds), avoiding a
regression in the int-overflow-promotion tests.

Rebased onto current main. Regression test in constant_propagation/straight_line.rs.
@mirchaemanuel
mirchaemanuel force-pushed the fix/384-match-subject-byref-writeback branch from 7c85448 to 5b9f1b6 Compare July 6, 2026 08:46
@nahime0
nahime0 merged commit 234f0b5 into illegalstudio:main Jul 6, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

match subject call loses by-reference write-back side effects

2 participants