feat(compiler): add specialization-aware CFG analysis - #89
Conversation
Record the growth-guard prerequisite, settled-specialization CFG lifecycle, cache publication rules, replay semantics, migration hazards, and acceptance tests for PIR Step 2B.
Cap each script solve at 256 unique unsettled specializations before shared-cache allocation, while settled warm hits remain free. Report a bounded active specialization-key chain and preserve finite polymorphic recursion by making the budget the sole termination guarantee. Add direct, mutual, retry, fixed-closure, scalar-companion, and lifecycle regressions; align the PIR and Step 2B plans with the sound budget-only policy.
Split template validation into structural checks and run effect-sensitive dataflow for each settled specialization. Cache immutable CFG diagnostics and complete direct-call edges, then replay the reachable closure deterministically for each typed script. Reuse specialization graph topology for effect settlement and CFG publication, preserve print and ReadsSeed events, and remove the duplicated syntactic effect classifiers.
Record the implemented Step 2B lifecycle, cache invariants, event ordering, and validation status in the durable PIR plan. Remove the temporary execution plan now that its completion criteria pass; it remains available in Git history.
ReviewReviewed at Verdict. The central idea is right and the pass split is genuinely necessary — template-time syntactic effect classification was an approximation, and this PR retires the whole mirrored classifier family ( Blocking1. The 256-specialization budget rejects ordinary valid programs
Repro — 300 distinct one-line functions, no recursion, no type growth: 257 ordinary functions is not a large program, so this is a hard false positive on a real limit. Two secondary problems visible in that message: the "active signature chain" degenerates to a single frame here, so the diagnostic points at nothing actionable; and because settled specializations are free ( The budget should count what it claims to bound. Either charge only specializations reachable through a growing signature chain, or key the bound to nesting depth of the discovery stack rather than the flat unique count. 2. A single
|
Bound only the active recursive inference region before cache allocation, so broad and acyclic specialization sets remain unrestricted. Keep the guard an honest cold-discovery resource fuse and add coverage for wide cycles, retries, warm tails, and flat breadth.
Preserve immutable diagnostics on every specialization while presenting each source-identical defect once per script replay. Keep specialization-key traversal separate so unique scalar or typed diagnostics are never hidden.
Keep read and formatting validation pure until typed statement events have been constructed, then publish destinations at the caller. Add reachable-specialization and proven MustWrite regressions and remove an unnecessary ABI test seed.
Document the recursive-region resource fuse, structural-only unreachable templates, proven MustWrite strictness, and source-stable diagnostic set union across reachable specializations.
Follow-up review —
|
| Repro | 2a40e28 |
97d0f53d |
|---|---|---|
| 300 flat non-recursive functions | budget exhausted | ✅ builds |
Foo(1) + Foo(2.0) shared dead store |
printed 2× | printed 1× |
| scalar companion, single call site | printed 2× | printed 1× |
| self-expanding recursion | caught | caught, controlled |
mutual expanding recursion (Ping/Pong) |
— | caught |
valid finite recursion (Fact(10)) |
— | ✅ builds |
| trace message size | ~16 KB | 1358 B |
go test -race ./lexer ./parser ./compiler and python3 test.py (74/74) both pass.
Scoping the limit to a recursive inference region is the right call, and I think it's sound rather than merely stricter-in-practice: with finitely many templates, any chain deeper than the template count must repeat a template (pigeonhole), so an expanding recursion always establishes a region and cannot outrun the bound — while flat breadth and deep acyclic chains cost nothing. TestRecursiveSpecializationLimitAllowsFlatBreadthColdAndWarm and TestWideMutualGrowthHitsOneRecursiveRegionLimit pin both halves of that.
Two things you did better than what I suggested: dropping the h = 0.0 seed in compiler_test.go outright rather than adding the interleaved read I proposed; and turning TestUnreachableFunctionDataflowDiagnosticsAreDeferred into TestFunctionDataflowDiagnosticsWaitForReachableSpecialization plus an assertion that the diagnostics really do appear once the specialization is reachable — that makes the name true instead of just honest.
New: guard trip emits a spurious "is not converging" error per extra call in the statement
a178449 removed both the else if !f.Settled && !recordSpecializationDiscovery(...) { return f } short-circuit from InferFuncTypes and the guard call from TypeFunc, leaving allowSpecializationAllocation (compiler/solver.go:2637) as the only check — and it fires only on the uncached (!ok) path.
Once specializationGuardFailed is set, every cached-but-unsettled specialization reached later in the same statement is re-walked in full. New allocations at the bottom are denied silently (solver.go:204), so no new error is appended, the len(ts.Errors) > errsAtEntry early return doesn't fire, and TypeScriptFunc ends up with unresolved outputs and Converging == false — hitting the !ts.Converging branch at solver.go:2694 and appending a second diagnostic that contradicts the first and blames the template.
lib.pt: res = Grow(x)
res = Grow([x])
main.spt: v = Grow(1) + Grow([1])
v
97d0f53d: /lib.pt:2:11:recursive specialization resource limit exceeded (limit 256 …)
/lib.pt:1:7:Function Grow is not converging. Check for cyclic recursion and
that each function has a base case ← spurious
2a40e28: /lib.pt:2:11:specialization budget exhausted (limit 256 per Solve); …
Isolation: one call → 1 error; two calls in separate statements → 1 error (Solve aborts between statements); four calls in one statement → 3 spurious errors. So the count is calls-in-statement − 1, and each one costs a full ~256-frame re-walk.
It only degrades an already-failing compile, hence minor — but the second message actively misdirects, pointing at "check for a base case" when the real cause is the resource limit already reported one line above.
Fix: restore the short-circuit on the cached path — in InferFuncTypes, after the if !ok block, if ts.specializationGuardFailed && !f.Settled { return f }. Alternatively suppress the non-convergence diagnostic in TypeScriptFunc (solver.go:2694-2701) when the guard has tripped. Worth a regression test with two script-level calls to the growing function in one statement asserting exactly one error — TestUnboundedSpecializationGrowthHitsRecursiveLimitWithActiveChain misses it because it uses a single call with require.Len(ts.Errors, 1).
Still open (all minor, all fine to decline)
- A shared
.ptdefect is still reported once per calling script, each trailed byerror compiling scriptFile .../a.spt— inherent to per-script error lists now that dedup is per-script. Only worth revisiting if the duplicate output annoys in practice. - Replayed diagnostics still aren't sorted by
(file, line, column), so output readsmain.spterror →lib.pterror rather than grouped by file. collectDirectCallees(effects.go:556) is still a wrapper that exists to drop a return value."has no CFG result"still appears 4× for a one-producer invariant, including incallBodyOutputEffects(effects.go:293), which returnsBodyOutputEffectsand never readsCFG.effects.go:290still derefsFuncCache[mangled]unguarded, six lines above three carefully worded ICE panics.validateStatementEffectstill carries 9 inline panics restating whatderiveLetestablishes by construction.
Nothing above blocks. With the specializationGuardFailed short-circuit restored, I'd call this ready to come out of draft.
(One candidate finding — that the local key := in the new dedup loop shadows the package-level key() — I checked and dismissed: that shadow already exists three times on master (compiler.go, codecompiler.go, solver.go), so the new code follows the existing convention.)
Return cached unsettled functions immediately after the recursive discovery fuse reports an error. This prevents sibling calls in the same statement from appending contradictory non-convergence diagnostics.
Sign-off —
|
97d0f53d |
5ff5074 |
|
|---|---|---|
spurious not converging errors |
3 | 0 |
| total diagnostics (2-call form) | 2 | 1 |
TestRecursiveSpecializationFailureStopsSiblingRewalks pins exactly this shape with require.NotContains(..., "not converging"). go test -race ./lexer ./parser ./compiler and python3 test.py (74/74) pass locally, matching CI.
On the limits — agreed, and I checked each claim
Every one holds up empirically on 5ff5074:
- 300 unrelated specializations compile — cold and warm;
TestRecursiveSpecializationLimitAllowsFlatBreadthColdAndWarmreally does usespecializationCount = 300. - Acyclic call depth is not limited — a 300-deep chain
F0 → F1 → … → F299builds fine. - Array rank is not limited — a rank-200 literal builds fine.
- Frame 257 is the rejection point — the emitted chain reports
249 specializations omittedout of 257 frames, consistent with a limit of 256.
So 256 is not too small, and the fuse's scope is exactly as described.
Fixes #79 is accurate. I confirmed the original repro still hangs on master (CPU-bound, killed at 60s, no output) and now terminates on this branch with a single controlled diagnostic.
The layered plan is right, and I have evidence the remaining layers are load-bearing rather than theoretical: a rank-2000 nested literal (Id([[[…1…]]])) does not complete within 90s on 5ff5074. That is a genuinely different vector from #79 — no recursive template, so the frame fuse correctly never engages — which is precisely the gap #90 describes. Agreed too that C ABI / linker symbol limits are the wrong safeguard: they fail late, after the compiler has already done the expensive work, and the diagnostic would point at the linker rather than the source.
One thing worth writing into #90 explicitly: a rank cap of 32 or 64 narrows currently-accepted behavior — rank 200 compiles today. Almost certainly nobody cares, but it's a deliberate narrowing rather than pure hardening, so it deserves a line in the issue and a note in the release notes rather than being discovered by whoever first trips it.
Tracking issues
#90, #91, and #92 between them cover every open item from my earlier reviews — #91 takes the diagnostic ordering and the per-script attribution trailer, #92 takes the repeated has no CFG result panics, the unguarded FuncCache deref at effects.go:290, and the assertion density in validateStatementEffect. Splitting them out rather than growing this PR is the right call.
Nothing blocking left.
Return read events directly and report format-specifier diagnostics through the CFG error sink. Preserve the read-before-destination publication order without a parallel result wrapper.
Split function binding classification and body validation into focused helpers, reducing validateFuncTemplate complexity. Mark impossible CFG block and global-scope underflows as internal compiler errors.
Treat every parsed function parameter as an input, matching the parser invariant that parameter and output names cannot overlap. Remove the unreachable membership guard and use parameter-oriented naming throughout structural CFG validation.
Build parameter and output membership sets at their only call site now that no additional binding policy is involved.
Remove the redundant nil guard from AnalyzeSpecialization; the solver validates and dereferences every walked specialization before CFG settlement.
Treat scripts as zero-input, zero-output templates before typed dataflow so the forward pass no longer needs a structural-validation mode. Validate sparse statement-effect shape at solver publication, leaving CFG to enforce only missing facts, invalid write states, and seed scope.
Continue typed CFG dataflow after structural script diagnostics by reusing the structural pass's explicit reads in a fresh scope. This reports independent dead-store and write-after-write findings without duplicating format errors, and reports every missing dynamic specifier binding in one compile.
Summary
FuncInfo.Settled, then replay and deduplicate diagnostics from each script's reachable specialization closureReadsSeed, and sparseMustWrite/MayWritebehaviorFixes #79.
Why
Step 2A made write and yield effects solver-owned, but the existing CFG still performed a syntax-approximate pass over every template and only analyzed the script after type solving. Function specializations never received effect-aware CFG analysis, cached specializations could not replay diagnostics, and recursive specialization discovery could grow without reaching SCC settlement.
This change makes CFG diagnostics specialization-aware and extends the invariant that a settled specialization has published effects, complete direct-call edges, and a non-nil immutable CFG result.
The recursive guard is an operational fuse for cold, unsettled inference work. It is not a limit on total functions, total specializations, array rank, or flat/deep acyclic code. Settled cache hits do not consume it. Unreachable
.pttemplates intentionally receive structural checks only; dead-store and write-after-write diagnostics begin when a specialization becomes reachable.Review guide
compiler/solver.goand the specialization guard tests incompiler/solver_test.go. The guard starts at the earliest repeated template in the active inference path, caps that recursive region at 256 frames, rejects beforeFuncCacheallocation, and emits one bounded diagnostic even when later sibling calls reuse an unsettled key.compiler/codecompiler.goand the structural portion ofcompiler/cfg.go. Templates are checked once for type-independent errors; reads are validated before let destinations are published.compiler/cfg.go. Event order is explicit reads,ReadsSeed, then sparse writes;MustWriteandMayWriteretain their distinct kill behavior.compiler/effects.go,compiler/solver.go, andcompiler/types.go. Batch-local primary edges feed SCC settlement; complete persistent direct edges feed replay. CFG results are staged and published before anySettledbit is flipped.compiler/scriptcompiler.goandcompiler/cfg_replay_test.go. Replay is deterministic, specialization-key visited, source-diagnostic deduplicated, and limited to the script's reachable closure.The large deletion in
compiler/cfg.goremoves the syntax-only effect classifiers whose responsibility is now owned by solver-publishedStatementEffectsandYieldEffects.Validation
eval "$(python3 scripts/llvm_env.py --shell)" && go test -race ./lexer ./parser ./compiler -count=1python3 test.py --leak-check— 74 passed, zero detected leaksgit diff --check 92fd5ac..HEAD