Skip to content

fix(checker): a dynamic-only function returns what it was given - #1033

Merged
nahime0 merged 10 commits into
mainfrom
fix/576-dynamic-only-untyped-params
Sep 18, 2026
Merged

nahime0 merged 10 commits into
mainfrom
fix/576-dynamic-only-untyped-params

Conversation

@Guikingone

Copy link
Copy Markdown
Collaborator

Closes #576.

function h($b, $p) { return $b; }
$fn = 'h';
var_dump(call_user_func($fn, "probe", 9));   // int(0); PHP: string(5) "probe"

Silent: no cast in the source, no diagnostic, and the value is gone.

Why

An untyped parameter starts as the checker's Int placeholder, which a direct call site replaces with the real argument type. A function reached only through a dynamic callable has no such site, so the placeholder survives — and the return type, inferred from return $b;, records Int for a value that is really whatever the caller passed. The runtime-callable invoker coerces through that recorded type.

The value was never lost on the way in: gettype($b) inside the callee answers string, exactly as the issue reports. Only the return was misdescribed.

The rule

An un-hinted body that hands one of its untyped by-value parameters straight back records mixed. Narrow on purpose:

  • only a return that yields the parameter itself counts, through the pass-through shapes (?:, ??, match, @, assignment);
  • a body that computes its own result keeps its inferred type — function add($a, $b) { return $a + $b; } still returns int;
  • a declared return type is authoritative and is never overridden.

Where, and why there

In the checker, because the declaration and its call sites have to reach the same answer.

EIR already boxes such a parameter, and normalize_method_map_for_eir already applies this exact rule to methods. Widening only the callee makes things worse rather than better — measured, call_user_func then read the boxed cell back as a raw integer and printed int(4309365472) instead of int(0). The checker's signature is what both sides start from.

That the rule was already written for methods and missing for functions is the defect, so it now lives in one place — src/types/dynamic_params.rs — and ir_lower calls it instead of keeping its own copy. That is most of the diff: 147 lines move out of ir_lower/program/metadata.rs.

Measured

Every row of the issue's scope table, byte-identical to the host PHP 8.5.10, including the ones that must not change:

call_user_func($fn, "probe", 9) string(5) "probe" the defect
call_user_func($fnp, "x", 12345) int(12345) was accidentally right — an int survives an int cast
call_user_func($fnt, "typed", 1) string(5) "typed" declared types, never affected
m("direct") then call_user_func($fm, "probe") string(6) "direct", string(5) "probe" the masking variant, both orders
strlen(call_user_func($fs, "abcde")) int(5) refused to compile before
call_user_func($fadd, 2, 3) int(5) computing body keeps int
array_map, call_user_func_array the other entry points into the same invoker

Verified against the defect, not only against PHP: with the rule disabled the regression fails at the static consumer with

strlen cannot lower checked operand type Int

which is the recorded return type read back directly.

Suites: --bin elephc 1915, error_tests 1534, codegen::callables 455, codegen::types 313, codegen::oop 615, codegen::arrays 568, codegen::spl 258, codegen::runtime_gc 298 — all clean.

Docs

docs/internals/the-type-checker.md gains a "When no direct call site exists" subsection under call-site inference: why the Int fallback is only sound when something replaces it, why the rule is recorded in the checker rather than during lowering, and what keeps it narrow. docs/php/functions.md states the user-visible guarantee next to the dynamic-callback paragraph.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr

@github-actions github-actions Bot added area:eir Touches EIR definitions, lowering, validation, or passes. area:types Touches type checking, inference, or compatibility. size:s Small pull request. type:fix Corrects broken or incompatible behavior. labels Sep 15, 2026
@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no new changes were introduced since the previous review, all previous threads are resolved, and no repository-rule violations remain.

Summary

This PR preserves values returned through dynamic callables by widening eligible untyped pass-through function returns to mixed, while retaining inferred types for directly specialized or computed returns.

  • Centralizes dynamic-parameter return detection for checker and EIR lowering.
  • Tracks genuine direct function calls while excluding signature probes and first-class callable resolution.
  • Handles generated include-function variants through resolver metadata rather than symbol-name parsing.
  • Adds PHP-compatible regression coverage, documentation, and an example.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Resolve function declarations] --> B[Infer provisional signatures]
    B --> C[Track genuine direct calls]
    C --> D{Direct arguments specialized function?}
    D -->|Yes| E[Keep inferred return type]
    D -->|No| F{Returns untyped parameter directly?}
    F -->|Yes| G[Record mixed return type]
    F -->|No| E
    G --> H[Checker and EIR share dynamic-parameter predicate]
    E --> I[Generate callable descriptor]
    H --> I
    I --> J[Dynamic invocation preserves returned value]
Loading

Reviews (10) · Last reviewed commit: "ci: empty commit to retrigger checks"

Comment thread src/types/dynamic_params.rs
Comment thread src/types/checker/driver/functions.rs Outdated
@github-actions github-actions Bot added size:m Medium-sized pull request. and removed size:s Small pull request. labels Sep 15, 2026
Comment thread src/types/checker/functions/resolution/call.rs Outdated
@github-actions github-actions Bot added the area:resolver Touches include, namespace, name, or autoload resolution. label Sep 16, 2026
Comment thread src/resolver/function_variants.rs Outdated
Comment thread src/types/checker/driver/functions.rs Outdated
Comment thread src/types/dynamic_params.rs
@Guikingone
Guikingone force-pushed the fix/576-dynamic-only-untyped-params branch from 05dc9da to 4dae531 Compare September 16, 2026 07:33
@Guikingone Guikingone self-assigned this Sep 16, 2026
Closes #576.

    function h($b, $p) { return $b; }
    $fn = 'h';
    var_dump(call_user_func($fn, "probe", 9));   // int(0); PHP: string(5) "probe"

Silent. No cast in the source, no diagnostic, and the value is gone.

## Why

An untyped parameter starts as the checker's `Int` PLACEHOLDER, which a direct
call site replaces with the real argument type. A function reached ONLY through a
dynamic callable has no such site, so the placeholder survives -- and the return
type, inferred from `return $b;`, records `Int` for a value that is really
whatever the caller passed. The runtime-callable invoker then coerces through
that recorded type.

The value itself was never lost on the way in: `gettype($b)` inside the callee
answers `string`. Only the RETURN was misdescribed.

## The rule

An un-hinted body that hands one of its untyped by-value parameters straight back
records `mixed`. It is narrow on purpose:

- only a `return` that yields the parameter itself counts, through the
  pass-through shapes (`?:`, `??`, `match`, `@`, assignment);
- a body that computes its own result keeps its inferred type, so
  `function add($a, $b) { return $a + $b; }` still returns `int`;
- a declared return type is authoritative and is never overridden.

## Where, and why there

In the CHECKER, because the declaration and its call sites must reach the same
answer. EIR already boxes such a parameter, and `normalize_method_map_for_eir`
already applies this exact rule to methods -- but widening only the callee makes
it worse, not better: measured, `call_user_func` then read the boxed cell back as
a raw integer and printed `int(4309365472)` instead of `int(0)`. The checker's
signature is what both sides start from.

That the rule was already written for methods and missing for functions is the
whole defect, so it now lives in one place -- `src/types/dynamic_params.rs` --
and `ir_lower` calls it rather than keeping its own copy.

## Measured

Every row of the issue's scope table, byte-identical to the host PHP 8.5.10,
including the ones that must NOT change:

    call_user_func($fn, "probe", 9)      string(5) "probe"   the defect
    call_user_func($fnp, "x", 12345)     int(12345)          was accidentally right
    call_user_func($fnt, "typed", 1)     string(5) "typed"   declared types
    m("direct") then call_user_func      string(6)/string(5) the masking variant
    strlen(call_user_func($fs, "abcde")) int(5)              refused to COMPILE before
    call_user_func($fadd, 2, 3)          int(5)              computing body keeps int
    array_map, call_user_func_array                          the other entry points

Verified against the defect: with the rule disabled the regression fails at the
static consumer with `strlen cannot lower checked operand type Int` -- the
recorded return type read back directly.

Suites: `--bin elephc` 1915, `error_tests` 1534, `codegen::callables` 455,
`codegen::types` 313, `codegen::oop` 615, `codegen::arrays` 568, `codegen::spl`
258, `codegen::runtime_gc` 298 -- all clean.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
… site

CI caught a feedback loop the local suites had not reached. Recording `mixed`
for EVERY un-hinted body that returns one of its own untyped parameters is too
broad, because the recorded type flows back into the caller:

    function grow($arr) { for (…) { array_push($arr, $i); } return $arr; }
    $arr = grow($arr);

`mixed` on the return makes the caller's LOCAL mixed, the next call
re-specializes the parameter to mixed, and `array_push($arr, …)` inside the body
stops type-checking:

    array_push() first argument must be array

`test_array_reassign_after_function_growth`, on shards 6/10/11 of all three
architectures.

The widening now happens in `resolve_unchecked_functions`, which is exactly the
set the defect is about: `fn_decls` minus `functions` is the functions no direct
call ever resolved, so the `Int` placeholder is all their parameters ever had.
A function with a direct call site already learns its real types from it and
needs nothing here -- `grow` keeps `array` and the loop is gone.

The #576 fixture is unchanged and still passes, including the masking variant
(`m("direct")` followed by `call_user_func`) that has a direct call site: the
direct call teaches the parameter `string`, which is what the dynamic path then
returns.

Verified against the defect again after the change: disabling the widening fails
the regression at the static consumer with `strlen cannot lower checked operand
type Int`.

Suites: `--bin elephc` 1915, `error_tests` 1534, `codegen::regressions` 342,
`codegen::callables` 456, `codegen::arrays` 568, `codegen::types` 313 -- clean.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Review follow-up, and the reviewer is right: membership in `functions` was the
wrong proxy for "a call site taught this function its parameter types".

A FIRST-CLASS CALLABLE reference resolves a function without calling it.
`array_map(h(...), ["probe"])` inserts `h`'s placeholder-based signature while
checking the callable expression, so the previous gate skipped `h` and the defect
survived on exactly the path #576 is about. Measured, all three spellings:

    array_map(h(...), ["probe"])        [int(0)]      PHP: ["probe"]
    $fn = k(...); $fn("direct-fcc")     int(0)        PHP: "direct-fcc"
    call_user_func(m(...), "via-cuf")   int(0)        PHP: "via-cuf"

`functions_called_directly` now records the name at `check_function_call`, which
is the one place a real argument list reaches a function, and the widening runs
as a whole-program pass after every signature exists and every direct call has
been seen. The `grow($arr)` feedback loop the previous round fixed stays fixed:
it HAS a call site, so it is still skipped.

Also from review: `stmt_returns_dynamic_param` skipped `StmtKind::IfDef`. That
arm is unreachable today -- `check_stmt` rejects a surviving `ifdef` with
"Unresolved ifdef statement", so neither caller can see one -- but the sibling
body walkers (`mixed_storage_scan`, `binding_decision_ambiguity`) recurse into
it, and "no returns here" is the wrong default for a construct a walker does not
know. It now recurses too.

Regression extended with the three first-class callable forms.

Suites: `--bin elephc` 1915, `error_tests` 1534, `codegen::regressions` 342,
`codegen::callables` 456, `codegen::arrays` 568 -- clean.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Review follow-up. Two things reach `check_function_call` without teaching a
function anything, and both were suppressing the pass-through return widening:

  function_exists('fe')   resolves the signature by calling it with FABRICATED
                          zeros -- `check_function_exists` builds one
                          `IntLiteral(0)` per parameter purely to force
                          resolution
  opt()                   a genuine call that passes nothing, so a returned
                          optional parameter keeps the type its default implies

Measured through a callable variable, before and after:

    function_exists('fe'); $fn='fe'; call_user_func($fn, "probe")
      was  int(0)          now  "probe"
    opt(); $fo='opt'; call_user_func($fo, "probe")
      was  NULL            now  "probe"

The probe restores the PREVIOUS membership rather than clearing it, so a probe
that follows a genuine call cannot erase that call's mark. The empty-argument
case is a plain `!args.is_empty()` guard.

This stays coarse for a call that fills SOME untyped parameters and not others:
it marks on any argument rather than per parameter. The comment says so.

## Not fixed here, and not caused here

The same two shapes with a LITERAL callable string still report a compile error:

    call_user_func('fe', "probe")   parameter $b expects Int, got Str

That is an ordering problem, not a widening one -- a literal callable string is
validated against the signature during the top-level walk, which runs before the
widening pass. Verified identical on unmodified `origin/main`, so it predates
this PR and is out of its scope. The regression drives both shapes through a
callable variable and says why.

Suites: `--bin elephc` 1915, `error_tests` 1534, `codegen::callables` 456,
`codegen::regressions` 342, `codegen::arrays` 568 -- clean.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
…s generated

CI caught the last gap in the call-site gate. A function declared in an INCLUDED
file is registered under a generated symbol --
`__elephc_include_variant_{hash}_{local}` -- while the call site names it as the
source wrote it. `widen_dynamic_only_passthrough_returns` compared those two
directly, so every include variant looked like a function no caller had ever
touched.

`test_include_function_variant_specializes_untyped_array_param_from_call` on
shard 13/16 of all three architectures:

    function append_item($items, $value) { …; return $items; }
    $items = append_item($items, "c");
    echo count($items) . ':' . $items[2];

    left:  "4335090960:"      <- the widened return read back as an integer
    right: "3:c"

`function_was_called_directly` now reads the local name back out of the generated
symbol, through `resolver::include_variant_local_name` -- a reader that sits
beside the writer and shares its prefix constant, so the two cannot drift.

A call to `append_item` reaches whichever variant is live, so ANY variant of a
called name counts as called. That is the conservative direction: its only effect
is to leave a return type alone, so a variant reachable solely through a dynamic
callable keeps the placeholder exactly as it did before this pass existed.

Regression added with the CI fixture's shape, plus `load_items` beside it as the
control -- no parameters at all, so the rule must miss it for a different reason.

Suites: `--bin elephc` 1915, `error_tests` 1534, `codegen::include_paths` 16,
`codegen::callables` 457, `codegen::regressions` 342 -- clean.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
…ts spelling

Review follow-up on three threads.

## P1 -- a name is not a proof

`function_was_called_directly` recovered an include variant's public name by SPLITTING
the generated symbol: strip `__elephc_include_variant_`, drop the hash, keep the rest.
A user function may legitimately be NAMED that way -- a leading `__` is legal PHP -- and
it then inherited the call sites of whatever the tail spells.

Reproduced. `foo(7)` is a direct call; the imitator is dynamic-only and returns its
untyped parameter:

    function foo($x) { return $x; }
    function __elephc_include_variant_0123456789abcdef_foo($x) { return $x; }
    echo foo(7), "\n";
    var_dump(array_map(__elephc_include_variant_0123456789abcdef_foo(...), ["fcc"]));

    PHP:                          array(1) { [0]=> string(3) "fcc" }
    elephc before this commit:    array(1) { [0]=> int(0) }

`foo`'s call site suppressed the imitator's widening and the returned string came back
through the `Int` placeholder. Exactly the #576 failure the PR exists to fix, reachable
by naming a function unluckily.

The fix is to stop parsing names. `function_variant_groups` already holds the real
mapping -- the key IS the public PHP name and the values are exactly the symbols the
resolver generated for it -- so the lookup is a membership test against data, and a name
nobody generated is in no group. `include_variant_local_name` and the `pub(crate)` on
`INCLUDE_VARIANT_PREFIX` existed only to serve the parse and are gone with it; the
constant's docblock now says why nothing reads a symbol back apart.

Note this is narrower than the reviewer's framing: a literal `call_user_func('name', …)`
or `array_map('name', …)` resolves and specializes the parameter before the widening
pass, so the wrong answer only appears through a first-class callable. The test uses
that shape for exactly that reason, and says so.

`test_a_function_named_like_an_include_variant_keeps_its_own_call_sites` pins it, and
all seven existing include-variant tests still pass -- the group lookup covers the real
variant case that motivated the reader.

## P2 -- the docblock described the wrong function

The block explaining the widening was attached to `function_was_called_directly`, and
`widen_dynamic_only_passthrough_returns` had none. Split: the predicate documents how it
sees past the decoration and why any variant of a called name counts; the pass documents
what it records, why the gate is `functions_called_directly` rather than membership in
`functions`, and why it is gated at all.

## P2 -- missing example

`examples/advanced-functions/main.php` gains a `callable-only passthrough` section: an
untyped parameter reached only through a first-class callable, a callable string and
`array_map`, returning a string as a string. It lives there rather than in
`examples/callbacks/` because that example does not run under host PHP at all
(`ARRAY_FILTER_USE_VALUE` is not a PHP constant), so it cannot back a parity claim.
`examples/advanced-functions/main.php` is byte-identical to host PHP 8.5.10, before and
after.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
@Guikingone
Guikingone force-pushed the fix/576-dynamic-only-untyped-params branch from 4dae531 to 08bc387 Compare September 16, 2026 21:16
cursoragent and others added 4 commits September 18, 2026 11:33
…untyped-params

# Conflicts:
#	docs/internals/the-type-checker.md

Co-authored-by: Vincenzo Petrucci <nahime0@users.noreply.github.com>
The shared predicate is applied by widen_dynamic_only_passthrough_returns, not signature resolution.

Co-authored-by: Vincenzo Petrucci <nahime0@users.noreply.github.com>
A method reached only through a callable array has no function-shaped call site, so this pins the shared dynamic-params rule for methods.

Co-authored-by: Vincenzo Petrucci <nahime0@users.noreply.github.com>
Co-authored-by: Vincenzo Petrucci <nahime0@users.noreply.github.com>
@nahime0
nahime0 merged commit ba4bdce into main Sep 18, 2026
149 checks passed
@nahime0
nahime0 deleted the fix/576-dynamic-only-untyped-params branch September 18, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:eir Touches EIR definitions, lowering, validation, or passes. area:resolver Touches include, namespace, name, or autoload resolution. area:types Touches type checking, inference, or compatibility. size:m Medium-sized pull request. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Untyped functions reachable only via dynamic callables get Int-defaulted params: returned strings silently become int(0)

3 participants