Skip to content

refactor(ir): calc lowering moves to lower.ToCalc, the runtime holds a program.Calc - #2146

Open
devin-ai-integration[bot] wants to merge 5 commits into
feat/ir-program-idsfrom
feat/ir-calc-lowering
Open

devin-ai-integration[bot] wants to merge 5 commits into
feat/ir-program-idsfrom
feat/ir-calc-lowering

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Step 5 of the execution-owned IR migration (docs/internals/execution-ir-design.md, "Migration as a stack of pull requests", row 5), stacked on #2137 (feat/ir-program-ids). Merge order: #2137 first; this PR is then retargeted to develop.

Calc lowering leaves the runtime. runtime.calcShape — the flattened invocation interface the runtime built from symbol.Decl in invoke_calc.go — becomes program.Calc, built by lower.ToCalc. The runtime wraps the record in loadedCalc (the state only the runtime has: the compiled tier and its eligibility, the tool binding, member-declaration metadata, per-run statement orders) and keeps memoizing by symbol in Model.calcShapes / Model.predicateShapes. compileBatch.callees is keyed by *program.Calc; the batch keeps a *program.Calc → *loadedCalc map (held) for the runtime state it settles.

What program.Calc holds and what it defers

Per the design's "Expressions" section, program imports nothing from internal/syntax/ast (the layering test pins it) and expressions are not lowered until steps 6a/6b. So:

  • Holds (internal/ir/program/calc.go): the calc symbol, its Origin, qualified Name, Kind and Label; the specialization Chain; the positional Params (name, owner, declaring symbol, the CalcMember declarations along the chain, subject/actor/calc flags); ParamNames and the redefinition Aliases; the Outputs (name, owner, declarations, result/in-out/initial flags); BodyOwner and BodyOutputs; Returns, StepsReturn, StepCount.
  • Defers behind program.ExprID: every expression — a parameter's Default, an output's Value, the Result expression — is an opaque id (dense from 1, 0 = none) into a per-calc table.
  • Stays on the lower side (lower.CalcTrees): the body statements and their StatementOrder, the steps, the body Nodes and Bindings, and the ExprID → ast.Node table (CalcTrees.Expr). The runtime reads an expression through loadedCalc.Expr(id) exactly where it read the ast.Node field before, so evaluation, error timing and error text are unchanged.

Public surface added

internal/ir/program: Calc, CalcParam, CalcOutput, CalcMember, ResultOutputName, Canonical, AliasRedefined; methods Calc.ResultOutputIndex/ResultOutput/HasInitialOutput/IsCase/Performs/BodyScope/Parameter/HasParameter/SubjectParameter/PositionalIndexes/PositionalArity/OutputNames/UsageSpelling.

internal/ir/lower: CalcEnv, CalcTrees (+ Expr), ToCalc, ToPredicate, ToCalcSignature, CalcChain, CalcKind, CalcStatesComputation, UnboundResultHint, DefaultValueOf, IsCalcDecl, IsPredicateDecl, PredicateKind, IsCalcUsageSymbol, UnwrappedDeclMembers, EachUnwrappedDeclMember, SubjectDecl, SubjectDeclaration.

Nothing from #2137's API changed.

Where the design met the code

  • Predicates (invoke_predicate.go) shared calcShape; they lower through lower.ToPredicate, so the runtime keeps one record type and no second AST-reading constructor.
  • The library-performance signature (resolveLibraryPerformance) built a bodiless calcShape; that is lower.ToCalcSignature.
  • The row names "tests/perf calc benchmarks", but no benchmark in tests/perf matches -bench Calc; the calc-invoking benchmarks are BenchmarkREPLEvalExpr, BenchmarkGRPCEvaluate and BenchmarkConnectEvaluateHTTP (each evaluates Perf::Calc1(2.0, 3.0)), reported below.
  • The spec-compliance sources cite file:function pairs (a hygiene test checks them), so the calc rows now cite lower/calc.go, program/calc.go and loadedCalc.

Baselines

None moved: no conformance .expected.json, golden trace, testdata/compiled file, corpus ratchet or pilot-differential record changed.

How it was verified

Full gate on the head (base feat/ir-program-ids merged in with --no-ff): go build ./..., go vet ./..., gofmt -l . (empty), make lint, go test ./..., go test -count=1 ./tests/corpus -run 'TestTrainingExamples|TestPilotCorpora' with both corpora fetched, go test -count=1 ./tests/hygiene, go test -C tools ./..., and the two-line-comment check over the diff (prints nothing). New unit tests in internal/ir/lower/calc_test.go pin the chain/interface lowering, the chained-assignment rule and the dense, deterministic expression ids.

Benchmarks (go test ./tests/perf -run '^$' -bench 'REPLEvalExpr|GRPCEvaluate|ConnectEvaluateHTTP' -benchmem, benchstat, base feat/ir-program-ids vs this head):

Three interleaved comparisons on the same box (n=6, n=8, n=4); benchstat finds no difference the noise does not explain, and the allocation profile is byte- and count-identical:

                      │ feat/ir-program-ids │     this head      │
                      │       sec/op        │  sec/op   vs base  │
REPLEvalExpr-8               9.107m ± 40%     9.621m ± 24%   ~ (p=0.798 n=8)
GRPCEvaluate-8               23.77µ ± 25%     25.34µ ± 44%   ~ (p=0.959 n=8)
ConnectEvaluateHTTP-8        139.1µ           143.9µ         ~ (p=0.886 n=4)
                      │        B/op         │   B/op    vs base  │
REPLEvalExpr-8              6.811Mi ± 0%     6.811Mi ± 0%   ~ (p=0.234 n=8)
GRPCEvaluate-8              10.39Ki ± 1%     10.39Ki ± 1%   ~ (p=0.721 n=8)
ConnectEvaluateHTTP-8       22.85Ki          23.07Ki        ~ (p=0.486 n=4)
                      │      allocs/op      │ allocs/op vs base  │
REPLEvalExpr-8                 110.0 ± 0%      110.0 ± 0%   ~ (all samples equal)
GRPCEvaluate-8                 129.0 ± 0%      129.0 ± 0%   ~ (all samples equal)
ConnectEvaluateHTTP-8          214.0           214.0        ~ (all samples equal)

The wall-clock spread (±25–44%) is the shared runner, not the change: the earlier n=6 comparison put this head at −13% geomean, this one at +6%, both inside the interval.

Pilot execution differential (go run -C tools ./cmd/pilot-exec-diff, evaluator provisioned): agree 229 · kind-only 1 · order-only 0 · disagree 38 · differs-by-design 5 · pilot-unevaluated 167 · pilot-silent 24 · pilot-error 7 · ours-error 10 · ours-undetermined 17 · both-error 16 · nondeterministic 0 (514 cases) — identical to the committed record in docs/project/pilot-execution-referee.md.

Checklist

  • make test and make lint pass locally
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (see CONTRIBUTING.md)
  • Changelog entry added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md — internal refactor, no user-visible change
  • baselines regenerated and make docs-counts run if a gate count moved (compliance rows need nothing: the census is counted at docs build) — nothing moved
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/d00fc0ddec284f26b9cb865143821490
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/d00fc0ddec284f26b9cb865143821490?variant=devin
Requested by: @HuiJun


🟢 Devin Review found nothing to fix · View findings

Devin Review

devin-ai-integration Bot and others added 5 commits November 11, 2026 17:12
…a program.Calc

calcShape becomes program.Calc built by lower.ToCalc; its body, bindings and expressions stay on the lower side as CalcTrees indexed by ExprID until expressions are lowered. The runtime wraps the record in loadedCalc with what it builds per calc (write targets, tool, compiled tier) and memoizes by symbol as before; compileBatch.callees is keyed by *program.Calc.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…nd loadedCalc

The spec-compliance sources name the functions that implement each rule; the calc interface now lowers in lower/calc.go and is held by the runtime's loadedCalc.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
The compliance maps name file:function pairs that a hygiene test checks against the tree; the calc chain, parameters, result binding and predicate classification now live in lower/calc.go and program/calc.go, and the runtime's record is loadedCalc. Drops Context.calcMemberDeclOf, left unused by the move.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

This branch has not been deployed

No deployments
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.

1 participant