fix: read possibly undefined locals as null with the "Undefined variable" warning by JGamboa · Pull Request #1938 · illegalstudio/elephc

JGamboa · 2026-10-20T02:35:06Z

Summary

A local that a join point may leave undefined (assigned in one if arm, in a loop body that may
not run, in one switch case or match arm, in a try body, on one lazy &&/||/?:/??
edge, or unset() in a loop or branch) used to be read straight from its slot. Main therefore
returned the value a previous call left there, an empty string, or uninitialized memory at the
top level, and isset()/??/empty() answered from that stale value too.

This PR makes those reads behave like php:

Checker. The checker tracks maybe-undefined locals per construct
(Checker::maybe_undefined_locals, joined at if/else, loops, switch, try/catch/
finally, match, ternaries and lazy operands) and records every read made while a local is
in that state (CheckResult::maybe_undefined). The join logic is in the new module
types::checker::condition_edges.
- Binders define their names: foreach key/value, catch variables, list(), global,
  static and builtin out-parameters.
- A local passed to a call that may bind it by reference is not tracked, so that call keeps
  receiving the local's real storage.
Lowering. EIR lowering gives only those locals a hidden defined-flag. The flag is cleared
at frame entry, set by every store and cleared by unset. Each recorded read lowers to
flag ? value : null.
- A plain read warns Undefined variable $x through trigger_error, so @ applies.
- isset, ?? and empty probe silently.
- Locals that are always defined keep their native storage and plain reads, so they cost
  nothing extra.
Closures.
- use (&$r) on a possibly undefined local creates it as null.
- use ($r) warns and captures null.
- An arrow function carries the flag as a hidden capture.
Condition edges. A local assigned in a lazy operand of a condition is defined in the
region that edge reaches, and only there:
- the if/while/for body on the true edge of $a && ($r = f());
- the else branch, or the code after an if whose clauses all diverge, on the false edge
  of !$x || !($y = g());
- the matching ternary arm.
Outside that region the local is maybe-undefined, so later reads go through the flag.
Type guards. A guard that is false for null ($x instanceof C, is_array($x),
isset($x)) defines the local in the region it guards, in if bodies and ternary arms.
Inside the guard the local is read directly with its narrowed type, so
if ($o instanceof O) { g($o); } with function g(O $x) compiles.
throw $e. When $e is a possibly undefined local, the operand is checked against the
local's own type and not against the mixed read. Without this check, this PR would start
rejecting code that main accepts:
- a Throwable local assigned in one if arm;
- a catch variable rethrown from finally.
When the local is undefined, the runtime raises Error("Can only throw objects") after the
warning, as php does. A maybe-undefined scalar is still rejected at compile time.
Removed error test. error_tests::type_system::test_error_short_circuit_assignment_effect_is_not_definite
is gone. echo false && ($x = 1); echo $x; used to be a compile error and now runs as in php.

Closes #1936

Differences from php

Output stream. The warning has php's text, Warning: Undefined variable $x in <file> on line N.
It goes to stderr, like every elephc runtime warning. The php CLI prints it on stdout, with a
leading blank line.
Error level. The warning is raised as E_USER_WARNING, not E_WARNING. An error handler
installed with set_error_handler sees errno 512 where php passes 2.
New compile errors. A possibly undefined read is typed mixed. Positions that need a
static array or object type now reject it at compile time where main accepted it, because
main read the slot directly. Examples:
- if ($c) { $o = new O; } if ($c) { g($o); } with function g(O $x);
- an array return type fed by such a local;
- $a + [..] where $a is such a local.
Scalar parameters (int, string), foreach, method calls, concatenation and appends are
not affected. Guarding the use with instanceof, is_array() or isset() avoids the
error (see above). This is documented as a new [[limitations]] entry in
scripts/docs/comparison_catalog.toml, next to the existing note on Mixed-sourced objects.
docs/php/compatibility.md was regenerated from it.

Left out

The throw operand changes from topic 31 (D5): accepting mixed, and accepting unions of
Throwables and null such as throw $c ? new A : new B. Only the maybe-undefined lookup
above is included, because this PR needs it.
The re-check of ternary arms that retype an existing local, and the lazy-operand retype path
($b = null; $ok || $b = f();). Both build on the assignment-expression lowering in PR 5.
Here a lazy operand is still checked on a copy of the environment, as on main, plus the
maybe-undefined join.
The array + mixed refusal in numeric_binary.rs, which was part of the original commit.
The checker already rejects that case before lowering.

Tests

The new tests are in tests/codegen/possibly_undefined_locals.rs. Expected outputs come from
php 8.5.10. Each case was run with the main binary at 0842fca, the current main (debug build).

Fail on main:

test_maybe_undefined_plain_read_warns_and_yields_null: main prints the previous call's
int(5)/int(2) and "", with no warning.
test_top_level_maybe_undefined_reads_answer_php_null: main prints uninitialized memory
(int(8595952200), bool(true)).
test_maybe_undefined_local_reads_answer_php_null: covers isset, ??, @, empty,
lazy edges, a loop that did not run, and unset.
test_maybe_undefined_closure_captures
test_maybe_undefined_locals_across_unset_try_switch_match
test_condition_edge_definitions_reach_the_guarded_region: fails to compile on main
(Undefined variable: $y).
test_condition_edge_definitions_nested_and_negated: fails to compile on main
(Undefined variable: $reflector).
test_throw_maybe_undefined_throwable_local: main's stdout matches. The test fails on main
because main prints no Undefined variable $e warning on stderr.
test_type_guard_defines_possibly_undefined_local: main's stdout matches. The test fails on
main because main prints neither of the two warnings.

Controls (pass on main too):

test_condition_edge_local_can_be_retyped_in_the_region
test_maybe_undefined_array_access_receiver_probes
error_tests::exceptions_enums_magic::test_error_throw_maybe_undefined_scalar_local

Local runs, all green:

cargo build;
error_tests, parser_tests, lexer_tests;
codegen_tests -- possibly_undefined variable scope if try finally loop closure global static unset isset exception.

Refs #1886

AI assistance

This change was written with substantial help from Claude Code (Anthropic). I reviewed it,
checked the expected outputs against php 8.5, and remain responsible for it.

🤖 Generated with Claude Code

…ble" warning A local a join point may leave undefined (assigned in one `if` arm, in a loop body that may not run, in one `switch` case or `match` arm, in a `try` body, on one lazy `&&`/`||`/`?:`/`??` edge, or `unset()` in a loop or branch) was read straight from its slot: the value a previous call left there, an empty string, or uninitialized memory at the top level, and `isset()`/`??`/`empty()` answered from that stale value. The checker now tracks such locals per construct (`Checker::maybe_undefined_locals`, joined at if/else, loops, switch, try/catch/finally, match, ternaries and lazy operands; see the new `types::checker::condition_edges`) and records every read made in that state. EIR lowering gives only those locals a hidden defined-flag (cleared at frame entry, set by every store, cleared by `unset`) and lowers each recorded read as `flag ? value : null`. A plain read warns "Undefined variable $x" through trigger_error, so `@` applies; isset/??/empty probe silently. Always-defined locals keep native storage. - Binders (foreach key/value, catch variables, list(), global, static, builtin out-parameters) define their names; a local passed to a call that may bind it by reference is not tracked. - `use (&$r)` creates a possibly undefined local as null, `use ($r)` warns and captures null, and an arrow function carries the flag as a hidden capture. - A local assigned in a lazy operand of a condition is defined in the region that edge reaches (the if/while/for body on the true edge of `$a && ($r = f())`, the code after a diverging `if` on the false edge of `!$x || !($y = g())`, the matching ternary arm) and only there. - A guard that is false for null (`instanceof`, `is_array()`, `isset()`) defines the local in the region it guards, so a typed parameter accepts the narrowed read. - `throw $e` on such a local is checked against the local's own type, so a Throwable local stays accepted and a scalar one is still rejected. `echo false && ($x = 1); echo $x;` is no longer a compile error.

greptile-apps · 2026-10-20T02:41:54Z

Confidence Score: 1/5

[Medium impact] This changes how compiled programs read locals across control-flow paths.

Fix the loop compile failures and missed undefined-read paths before merging.

Findings

Defined loop locals are rejected ▶
Later conditions read undefined locals ▶
Catch reads miss undefined locals ▶
Discarded reads lose their warnings ▶
New helpers lack required docs ▶

Fix with agent prompt

### Issue 1
src/types/checker/stmt_check/control_flow.rs:936-940
Loop bodies keep the maybe-undefined mark after their condition assigns the local. For example, `function take(array $a) {} function t($c) { while ($c && ($a = [1])) { take($a); break; } } t(true);` reads `$a` as `mixed` and rejects the call, although every path into the body assigns an array.

`condition_edge_definitions` cannot fix this because the lazy-operand join already added `$a` to the environment. Clear the true-edge marks before checking the body and update, as the `if` path does. The same gap exists in `check_for_tail`.

### Issue 2
src/types/checker/stmt_check/control_flow.rs:691-694
An `elseif` condition is checked before names introduced by earlier bodies are marked maybe-undefined. In `if ($c) { $x = 5; } elseif ($x > 3) { echo 'wrong'; }`, the path reaching `elseif` never assigned `$x`, but its read is not recorded. It still loads the raw slot without a warning and can choose the wrong branch.

Apply `body_introduced` to the false-path marks before checking the next condition.

### Issue 3
src/types/checker/condition_edges.rs:555-561
`end_try` saves only the marks left at the end of the body, not states where an exception can leave it. For example, `$x = 1; try { unset($x); if ($c) throw new Exception(); $x = 2; } catch (Exception $e) { var_dump($x); }` leaves `$x` undefined when `$c` is true. The later assignment clears its mark during checking, so the catch read loads the cleared slot without the required warning.

Keep the intermediate undefined states for catch and finally reads. The scan after the whole `try` runs too late.

### Issue 4
src/ir_lower/expr/maybe_undefined_read.rs:60-74
The new warning exists only when a variable read reaches EIR lowering, but the AST optimizer still treats every variable read as pure. In `function t($c) { if ($c) { $x = 1; } $x; } t(false);`, dead-code removal deletes `$x;` before this warning can be emitted. This also skips any installed error handler.

Teach the optimizer about recorded warning-producing reads and add a regression that discards the read's value.

### Issue 5
src/types/checker/condition_edges.rs:408-410
Several new functions lack the required Rustdoc comments. `AGENTS.md` requires a concise docblock for every explicit Rust function, including private helpers and methods. Add one to `env_keys` and the other new undocumented functions, including `SwitchMaybeUndefined::enter` and `TryMaybeUndefined::enter`. This repository requirement must be satisfied before merging.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds checker state and hidden defined-flags so possibly undefined locals yield null and warn instead of reading old slot contents.

Locals left undefined on the current path now read as null.
Assignments in lazy conditions are available on the edges that guarantee them.
Closures keep PHP's null and warning behavior for undefined locals.

Supplied acknowledgments from JGamboa: stderr output and E_USER_WARNING are intentional differences; static mixed restrictions are documented; by-reference call exclusions are intentional; broader throw support and lazy/ternary retyping are deferred. These were not reported as findings.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Checker tracks undefined paths] --> B[Read decisions by span]
  B --> C[AST optimization]
  C --> D[EIR read lowering]
  D --> E{Defined flag}
  E -->|Yes| F[Load value]
  E -->|No| G[Warn for plain reads]
  G --> H[Return null]

_{Reviews (1) · Last reviewed commit: "fix: read possibly undefined locals as n..." · Reviewed by Greptile}

greptile-apps · 2026-10-20T02:41:57Z

+                // `while ($ok && ($row = next()))`: the body runs on the condition's true edge.
+                let true_defs = self.condition_edge_definitions(condition, env, true);
+                let true_inserted = Self::insert_edge_definitions(env, &true_defs);
                let errors = self.check_break_continue_target_body(body, env);
+                Self::remove_edge_definitions(env, &true_inserted);


Defined loop locals are rejected

Loop bodies keep the maybe-undefined mark after their condition assigns the local. For example, function take(array $a) {} function t($c) { while ($c && ($a = [1])) { take($a); break; } } t(true); reads $a as mixed and rejects the call, although every path into the body assigns an array.

condition_edge_definitions cannot fix this because the lazy-operand join already added $a to the environment. Clear the true-edge marks before checking the body and update, as the if path does. The same gap exists in check_for_tail.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/types/checker/stmt_check/control_flow.rs Line: 936-940 Comment: **Defined loop locals are rejected** Loop bodies keep the maybe-undefined mark after their condition assigns the local. For example, `function take(array $a) {} function t($c) { while ($c && ($a = [1])) { take($a); break; } } t(true);` reads `$a` as `mixed` and rejects the call, although every path into the body assigns an array. `condition_edge_definitions` cannot fix this because the lazy-operand join already added `$a` to the environment. Clear the true-edge marks before checking the body and update, as the `if` path does. The same gap exists in `check_for_tail`. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-10-20T02:41:57Z

                for (cond, body) in &clauses {
+                    self.maybe_undefined_locals = path_marks.clone();
                    self.infer_type_with_assignment_effects(cond, env)?;
+                    let marks_after_cond = self.maybe_undefined_locals.clone();


Later conditions read undefined locals

An elseif condition is checked before names introduced by earlier bodies are marked maybe-undefined. In if ($c) { $x = 5; } elseif ($x > 3) { echo 'wrong'; }, the path reaching elseif never assigned $x, but its read is not recorded. It still loads the raw slot without a warning and can choose the wrong branch.

Apply body_introduced to the false-path marks before checking the next condition.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/types/checker/stmt_check/control_flow.rs Line: 691-694 Comment: **Later conditions read undefined locals** An `elseif` condition is checked before names introduced by earlier bodies are marked maybe-undefined. In `if ($c) { $x = 5; } elseif ($x > 3) { echo 'wrong'; }`, the path reaching `elseif` never assigned `$x`, but its read is not recorded. It still loads the raw slot without a warning and can choose the wrong branch. Apply `body_introduced` to the false-path marks before checking the next condition. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-10-20T02:41:57Z

+        let marks = checker.maybe_undefined_locals.clone();
+        let keys = env_keys(env);
+        if !checker.body_cannot_fall_through(body) {
+            self.exits.push((marks.clone(), keys.clone()));
+        }
+        self.try_marks = marks;
+        self.introduced = keys.difference(&self.keys_entry).cloned().collect();


Catch reads miss undefined locals

end_try saves only the marks left at the end of the body, not states where an exception can leave it. For example, $x = 1; try { unset($x); if ($c) throw new Exception(); $x = 2; } catch (Exception $e) { var_dump($x); } leaves $x undefined when $c is true. The later assignment clears its mark during checking, so the catch read loads the cleared slot without the required warning.

Keep the intermediate undefined states for catch and finally reads. The scan after the whole try runs too late.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/types/checker/condition_edges.rs Line: 555-561 Comment: **Catch reads miss undefined locals** `end_try` saves only the marks left at the end of the body, not states where an exception can leave it. For example, `$x = 1; try { unset($x); if ($c) throw new Exception(); $x = 2; } catch (Exception $e) { var_dump($x); }` leaves `$x` undefined when `$c` is true. The later assignment clears its mark during checking, so the catch read loads the cleared slot without the required warning. Keep the intermediate undefined states for catch and finally reads. The scan after the whole `try` runs too late. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-10-20T02:41:57Z

+    if warns {
+        let warning = Expr::new(
+            ExprKind::FunctionCall {
+                name: crate::names::Name::unqualified("trigger_error"),
+                args: vec![
+                    Expr::new(ExprKind::StringLiteral(format!("Undefined variable ${}", name)), expr.span),
+                    Expr::new(
+                        ExprKind::ConstRef(crate::names::Name::unqualified("E_USER_WARNING")),
+                        expr.span,
+                    ),
+                ],
+            },
+            expr.span,
+        );
+        let _ = lower_expr(ctx, &warning);


Discarded reads lose their warnings

The new warning exists only when a variable read reaches EIR lowering, but the AST optimizer still treats every variable read as pure. In function t($c) { if ($c) { $x = 1; } $x; } t(false);, dead-code removal deletes $x; before this warning can be emitted. This also skips any installed error handler.

Teach the optimizer about recorded warning-producing reads and add a regression that discards the read's value.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/ir_lower/expr/maybe_undefined_read.rs Line: 60-74 Comment: **Discarded reads lose their warnings** The new warning exists only when a variable read reaches EIR lowering, but the AST optimizer still treats every variable read as pure. In `function t($c) { if ($c) { $x = 1; } $x; } t(false);`, dead-code removal deletes `$x;` before this warning can be emitted. This also skips any installed error handler. Teach the optimizer about recorded warning-producing reads and add a regression that discards the read's value. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-10-20T02:41:57Z

+fn env_keys(env: &crate::types::TypeEnv) -> NameSet {
+    env.keys().cloned().collect()
+}


New helpers lack required docs

Several new functions lack the required Rustdoc comments. AGENTS.md requires a concise docblock for every explicit Rust function, including private helpers and methods. Add one to env_keys and the other new undocumented functions, including SwitchMaybeUndefined::enter and TryMaybeUndefined::enter. This repository requirement must be satisfied before merging.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: src/types/checker/condition_edges.rs Line: 408-410 Comment: **New helpers lack required docs** Several new functions lack the required Rustdoc comments. `AGENTS.md` requires a concise docblock for every explicit Rust function, including private helpers and methods. Add one to `env_keys` and the other new undocumented functions, including `SwitchMaybeUndefined::enter` and `TryMaybeUndefined::enter`. This repository requirement must be satisfied before merging. **Context Used:** AGENTS.md ([source](https://github.com/illegalstudio/elephc/blob/main/AGENTS.md)) --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

JGamboa mentioned this pull request Oct 20, 2026

feat: PHP compatibility batch (parser forms, checker convergence, references, dynamic includes, builtins) #1886

Draft

greptile-apps Bot reviewed Oct 20, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix: read possibly undefined locals as null with the "Undefined variable" warning - #1938

fix: read possibly undefined locals as null with the "Undefined variable" warning#1938
JGamboa wants to merge 1 commit into
illegalstudio:mainfrom
JGamboa:fix/possibly-undefined-locals

JGamboa commented Oct 20, 2026

Uh oh!

greptile-apps Bot commented Oct 20, 2026 •

edited

Loading

Diagram

Uh oh!

greptile-apps Bot Oct 20, 2026

Uh oh!

greptile-apps Bot Oct 20, 2026

Uh oh!

greptile-apps Bot Oct 20, 2026

Uh oh!

greptile-apps Bot Oct 20, 2026

Uh oh!

greptile-apps Bot Oct 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

JGamboa commented Oct 20, 2026

Summary

Differences from php

Left out

Tests

AI assistance

Uh oh!

greptile-apps Bot commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 1/5

Findings

Summary

Diagram

Uh oh!

greptile-apps Bot Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Oct 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Oct 20, 2026 •

edited

Loading