fix(infer): judge forward references; gate the tail-call false accept it revealed - #21
Merged
Merged
Conversation
…pre-pass
`inferModule'` folded declarations strictly in order, so any forward
reference — every mutually recursive `fn` pair, and every plain
`fn a` calling a `fn b` declared below it — hit `SKIP: unbound variable`
and took the whole file to exit 2. Safe, but it hid real defects,
including a mutually recursive pair with a wrong RETURN annotation that
march rejects.
What march does (`typecheck.ml`): pass 1 binds every top-level `DFn` to
one shared MONOMORPHIC placeholder (:11210-11235, conditional on the name
not already being bound); `reorder_decls` (:8988) then permutes each
maximal contiguous `DFn` run into callee-before-caller DFS post-order
(:8533), tolerating cycles by falling back to the placeholder on the
cyclic edges (:8531); `check_fn` uses a fresh monomorphic self-ref
(:6860) — which is why march REJECTS polymorphic recursion, verified —
and reconciles the placeholder only when the result is `Mono` (:7189).
The reordering is load-bearing and observable, so it is NOT reconstructed
here: `fn main` calling a later `fn pick(x) do x end` at two types is
ACCEPTED, but inserting a top-level `let` between them splits the run,
blocks the reordering, and march then REJECTS the same program (both
verified). Reconstructing that needs march's shadowing-aware
`free_vars_expr` and run-contiguity rules, and getting it wrong makes
false rejects.
Instead this pre-binds only the class for which the reordering is
provably UNOBSERVABLE: a `dfn` whose signature is complete and ground
(every param annotated, ground return annotation) has a scheme fixed by
the signature alone — provably `Mono`, provably `p1 -> ... -> pn -> ret`
— so caller-first and callee-first converge on the same constraint set
and the same verdict. Its declared arrow type is published as a
zero-variable scheme before any body is inferred.
Everything else keeps the old skip, and a pending pre-pass binding is
hidden from `dlet` right-hand sides and non-ground `dfn` bodies. That
second guard is not caution for its own sake: both looser designs were
built and BOTH produced live false rejects (a bare-metavar pre-pass on
`fn main` calling a later `fn mk(u : Int) do fn(y) -> y end`; and a
visible-everywhere pre-pass on `let h = b` before `fn b(y : Int) : Int`,
where march's level-1 placeholder is generalized into `h : forall p. p`
and march accepts `h(2)` and `h("s")` alike).
Probes: mutual recursion well-typed 2->0, with a type error 2->1, with a
wrong return annotation 2->1, three-way cycle 2->0, plain forward ref
2->0, wrong-arity forward call 2->1. Self-recursion unchanged (0/0, 1/1).
Polymorphic recursion, forward polymorphic callees, and the
generalization-boundary family all stay at skip.
Corpus: (0,0)=34 (0,2)=213 (1,2)=28 over 275 march-accepted files, and
(1,0)=2 (1,1)=30 (1,2)=88 over the 120-file reject corpus — both
byte-identical before and after. No file changed verdict either way.
…heck is unmodeled march runs an ERROR-level check this oracle does not model at all: `enforce_tail_calls_in_decls` (typecheck.ml:10902, invoked as Pass 3 at :11368), analysis `check_tail_position` (:10715-10898). It rejects a recursive call that is BOTH outside tail position AND not provably structurally decreasing. The discriminator is easy to miss: fn pong(n : Int) : Int do pong(n - 1) + 1 end -> WARNING, march exit 0 fn a(n : Int) : Int do b(n + 1) + 1 end / fn b -> ERROR, march exit 1 Telling them apart needs march's structural-decrease analysis, which is deliberately NOT reconstructed here — a partial reconstruction of a march decision is how false rejects get built. So the honest answer to either is a non-verdict. Without this gate the oracle answered ACCEPT for both: fn loopy(n : Int) : Int do loopy(n + 1) + 1 end march=1 lean=0 fn a/fn b as above march=1 lean=0 The first was ALREADY a false accept before the pre-pass (self-recursion never needed it); the second became reachable when the ground-signature pre-pass made mutual recursion inferable. Both are confident wrong verdicts on grounds we never examined, which the branch's governing rule forbids: a march decision we neither model nor reconstruct must fail toward skip. The gate is purely syntactic and declines to answer rather than judging tail-call legality. Scoping mirrors march exactly (:10917-10966): graph over top-level DFn names only, recursive iff SCC size > 1 or a direct self-call, checked names = the whole SCC. That scoping is load-bearing — `fn main() do println(int_to_string(helper(x))) end` is not itself recursive and is never gated. Every approximation points at MORE skipping: edges from all mentions (ignoring shadowing) rather than direct calls; no model of the `no_warn_recursion` attribute exemption (:10955); bare non-callee mentions and lambda/local-fn bodies treated as non-tail despite march giving them their own scope (:10856-10863); post-flatten single graph vs march's per-module graphs (:10968). All can only merge groups or add disqualifying positions, never the reverse. REJECT stays reachable: the gate runs only after `inferModule'` has already succeeded, so a wrong argument type, wrong return annotation or wrong arity still rejects regardless of tail position. Probes: `loopy` 0->2, mutual `a`/`b` 0->2, ADT `walk`/`helper` 0->2. Every previously-rejecting row still rejects (p02, p03, p05, p20, r5, u1, u3). Tail-safe mutual/forward rows still accept (p01, p06, p07, r3). Cost is one over-skip class: non-tail but structurally decreasing self-recursion (`fact(n - 1) * n`), which march only warns about, now skips. Corpus: (0,0)=34 (0,2)=213 (1,2)=28 over 275 march-accepted files and (1,0)=2 (1,1)=30 (1,2)=88 over the 120-file reject corpus — unchanged from the pre-branch baseline. Zero files moved; no accepting corpus file contains the over-skipped structural-recursion shape.
…se whitelist The blanket gate cost real coverage: march ERRORs on a non-tail recursive call only when it is ALSO not provably structurally decreasing (typecheck.ml:10738-10751). Gating every non-tail recursive call skipped an entirely ordinary class march merely WARNS about and accepts — `fn fib(n) do fib(n - 1) + fib(n - 2) end`, `n * fact(n - 1)`, `pong(n - 1) + 1`. The real-corpus casualty was specs/lang/grammar/parse/p15_multi_head_fn_merge (MATCH -> SKIP), which my 275-file sweep could not see because grammar/parse is not under a directory named `accept`. The gate now consults a whitelist of decreasing-argument shapes, built as a deliberate STRICT SUBSET of march's `is_structurally_smaller` (typecheck.ml:10693-10706) rather than a reproduction of it. Recognised: an argument `p - k` where `p` is a bare variable in the decrease base and `k` is a POSITIVE INTEGER LITERAL. A call is allowed when at least one argument qualifies, mirroring march's `List.exists` (:10743). The decrease base starts as the dfn's own directly-bound parameters (march's `fn_params`, :10957-10963) and grows exactly as march's `smaller` does: at a `match` whose scrutinee is a bare base variable, every arm-bound pattern variable joins the base inside that arm (:10812-10824). That growth is load-bearing, not incidental — desugaring a multi-head `fn` leaves ONE synthetic `__arg0` parameter and binds `n` in a `PatVar` arm, so without it `fib` has no parameter to decrement and p15 stays skipped. Every difference from march is in the over-gating direction. march treats `v - <anything>` and `v / <anything>` as smaller and never examines the right operand, so it accepts `n - 0` and `n - m`; both are gated here. march also recognises pattern-bound sub-components, list accessors and nullary constructors (rules 1, 3, 4), none of which are implemented. And march never removes a shadowed name from `fn_params`, while the base here is shadowing-aware across `let`/`lam`/`letfn`/patterns. Under-gating is the false accept this gate exists to remove; over-gating only costs coverage. Probes — must accept again: fib multi-head 2->0, `n * fact(n-1)` 2->0, `pong(n-1)+1` 2->0, mutual `a(n-1)`/`b(n-1)` 2->0. Must stay skipped, all march=1 with the tail-call diagnostic confirmed: `loopy(n+1)+1`, mutual `b(n+1)+1`, `dbl(n*2)`, `walk(q, q)` on an unrelated variable, `h(idf(n))` on a call result. Edges: `n - 0`, `n - m` and a `let`-shadowed parameter all over-gate (march accepts, we skip); `f(b - 1, a)` accepts, matching march's any-argument rule. Every reject row still rejects. Full conformance harness: MATCH 87, MISMATCH 0, ERROR 0, p15 matching again, zero new skips in the ledger diff (the sole entry is the expected stale reject/t57, fixed on a sibling branch).
…otations The pre-pass docstring asserted that every `dfn` reaching inference has ground parameter types, on the grounds that an unannotated parameter is emitted as FPPat and takes the file out of fragment. That is false for a DESUGARED MULTI-HEAD fn: merging `fn fib(0)`/`fn fib(1)`/`fn fib(n)` emits a synthetic `__arg0` as FPNamed with `ty: null`, which decodes in fragment with `annot = none` (verified against specs/lang/grammar/parse/p15_multi_head_fn_merge.march). No behaviour depended on it — `dfnGroundArrow` independently requires every parameter annotation to be present and ground, so `__arg0` fails the test and `fib` is correctly not pre-bound — but the assertion was an unchecked one of exactly the kind that hides defects, so it is retracted and the real counterexample recorded in its place.
Brings in the sibling branch so this one has the t57 ledger update (Check 6 moved it skip -> reject) and the decoder-coverage gate. Without it the harness fails on a stale ledger entry that is already fixed downstream.
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.
Forward references were never judged — and closing that revealed a live false accept
Compare.inferModule'folds declarations in order, so any forward reference hitSKIP: unbound variableand the whole file exited 2. The gap was not just mutual recursion: a plainfn acalling a laterfn bskipped identically. Self-recursion already worked.What march actually does (not textbook HM)
All in
lib/typecheck/typecheck.ml::11210-11235): every top-levelDFn→Mono (fresh_var 1), one shared monomorphic var per name, conditional on the name not already being bound (:11232).reorder_decls:8988,dependency_order_dfn_run:8533): each maximal contiguousDFnrun is permuted into callee-before-caller DFS post-order. Cycles are tolerated, not analysed.check_fn(:6842): fresh monomorphic self-ref, generalize at the outer level (:7139).:7189-7192): unify placeholder with inferred type only ifMono.No recursive group, no group generalization. A non-cyclic forward reference is fully let-polymorphic; only cyclic edges see the monomorphic placeholder. The ordering is observable — inserting one top-level
letbetween two functions splits the run, blocks reordering, and march then rejects a program it otherwise accepts.Polymorphic recursion: probed, not assumed — march rejects it (
sz((x,x))→ exit 1, annotated and unannotated alike).What this implements
A ground-signature pass-1 pre-pass: a
dfnwhose parameters and return are all ground-annotated has a scheme provably fixed by its signature, so it is published before any body is inferred. For that class march's reordering is provably unobservable.reorder_declsis not reconstructed; everything else keeps the old skip.Two looser designs were built and discarded because both produced live false rejects — the bare-metavar pre-pass, and the ground pre-pass made visible everywhere (march's placeholder is unsound at
fresh_var 1, yieldingh : ∀p. p, and accepts what we then rejected).The part that matters more: a false accept this revealed
Making mutual recursion inferable immediately started ACCEPTing programs march REJECTS on tail-call grounds —
enforce_tail_calls_in_decls(:10902), an ERROR-level check with no counterpart here.The blind spot was not created by this change. Self-recursion had been false-accepting the same check all along; the exposing shapes just skipped for unrelated reasons. Removing a skip converts a hidden non-verdict into a visible wrong verdict.
Discriminator that makes this easy to miss: march warns (exit 0) on non-tail recursion it can prove structurally decreasing (
pong(n - 1) + 1), but errors when it cannot (b(n + 1) + 1). Probing only the warning case makes the whole issue look like agreement.The gate
nonTailRecursionGate(Compare.lean) declines to answer — it renders no opinion on tail-call legality. Scoping mirrors march (:10917-10966): call graph over top-levelDFnnames, recursive iff SCC size > 1 or a direct self-call, checked set = the whole SCC. REJECT stays reachable: a genuine modeled type error still rejects regardless of tail position.The whitelist (so
fibsurvives)The gate alone cost
grammar/parse/p15_multi_head_fn_merge.march— i.e.fib— becausefib(n-1) + fib(n-2)is non-tail but decreasing, so march only warns. A deliberately tiny structural-decrease recogniser restores it: an argumentp - kwithpa bare variable in the decrease base andka positive integer literal, mirroring march'sList.exists(:10743).Conservative on the ungate side by construction — anything unrecognised stays gated. Every divergence from march over-gates: march also accepts
v - <anything>,v / <anything>, pattern sub-components, list accessors and nullary constructors; none implemented.A discovery that turned out to be load-bearing: a multi-head
fn fib(0)/fib(1)/fib(n)desugars into one clause with a synthetic__arg0parameter and amatch, so infib(n - 1)nis not a parameter at all — it is an arm-bound pattern variable. The base therefore has to grow atmatcharms exactly as march'ssmallerdoes (:10812-10824). Without that,fibstays skipped and the reason is invisible.Probe table (abridged)
fib(n-1) + fib(n-2)(p15)n * fact(n-1);pong(n-1) + 1loopy(n+1) + 1b(n+1) + 1;dbl(n*2);walk(q,q);h(idf(n))Every reject row still rejects. No
march=1row is accepted anywhere. Over-gated edges (all safe):z(n - 0), a shadowed parameter, non-literalg(n - m, m).Verification
395-file sweep: accept
34/213/28, reject2/30/88— identical to pre-branch, zero files changed.Also included
ecb56f3retracts a false claim the author had written into the pre-pass docstring — that everydfnreaching inference has ground parameter types. Desugared multi-headfns falsify it (FPNamedwith"ty": null). No behaviour depended on it, but it was an unchecked assertion propping up a soundness argument.Open, not closed
march's tail-call enforcement is gated, not modeled. The over-gate class (non-tail recursion whose decrease we cannot recognise syntactically) skips rather than being judged. Separating it properly needs march's real decrease analysis, which was deliberately not reconstructed.