Skip to content

fix(examples): ys-lower-report calls the public compile-form - #797

Open
mparrett wants to merge 1 commit into
mainfrom
fix/ys-lower-report-public-api
Open

fix(examples): ys-lower-report calls the public compile-form#797
mparrett wants to merge 1 commit into
mainfrom
fix/ys-lower-report-public-api

Conversation

@mparrett

@mparrett mparrett commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

examples/aot/yamlstar/ys-lower-report.lg has been unable to run since
compile-form* became private. Every invocation dies at load:

error: compiling let body
caused by: Can't resolve ir.passes.pipeline/compile-form* in this context
  --> examples/aot/yamlstar/ys-lower-report.lg:45:32

It never reports on a single form, so the per-fn lower/fallback report described
in examples/aot/README.md is currently unavailable.

Why the public wrapper, beyond it resolving

compile-form normalises a :multi-fn-template result to :status :lowered.
Its own comment gives the reason: public callers key off :status, and without
the normalisation "a form that lowered via lambda-lift reads as unlowered".
ys-lower-report is one of those callers — it branches on :status and prints
anything other than :lowered as a fallback. So the wrapper is the correct
entry point here, not just the reachable one.

Verification

Ran the tool over five in-repo namespaces, which exercises the whole of its
contract:

File Result
examples/aot/native-entry/fib.lg ok=2 fallback=0 threw=0
examples/aot/cross-package/src/lib.lg ok=2 fallback=0 threw=0
test/native-entry/closure_capture.lg ok=2 fallback=0 threw=0
test/native-entry/loop_carried_closure.lg ok=3 fallback=0 threw=0
test/native-entry/conditional_closure.lg ok=4 fallback=0 threw=0

fib.lg's ok=2 agrees with the 2 fns lowered that lg-compile reports for
the same file.

The three closure fixtures are the lambda-lift shapes, and they only show that
the fixed tool reports them lowered. There is no before-and-after to compare,
because nothing ran before.

Scope

Two lines: the call site and the header comment that describes it. Other
references to compile-form* in the tree describe the private function itself
and are left alone.

Found while using the tool to survey a demo's lowerability against tip, which
also produced #796 and the comments on #551, #562, #660 and #358.

@nooga

nooga commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Review summary

Verified end to end:

  • Reverted the fix locally and reran the tool — reproduces the exact reported error (Can't resolve ir.passes.pipeline/compile-form* in this context at the same line).
  • Restored the fix and ran it against all five files listed in the PR body (fib.lg, lib.lg, and the three closure fixtures) — output matches the PR's table exactly, including ok=2/ok=3/ok=4 and fallback=0 threw=0 across the board.
  • Confirmed the compile-form vs compile-form* distinction actually matters here, not just resolves: read compile-form's normalization in pkg/rt/core/ir/passes/pipeline.lg (a :multi-fn-template result gets :status :lowered attached, since the private path keys off :kind instead) and confirmed the three closure fixtures are exactly the lambda-lift shape that would read as unlowered without it.
  • Checked scope: grepped for remaining compile-form* references in the tree — all are the definition itself, doc/comment mentions, or internal test files that intentionally exercise the private function directly, not other broken external callers.
  • go build ./..., go vet ./... clean.

Small, precise, well-verified fix. LGTM — approving.

@nooga nooga left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Verified: reproduced the original error, confirmed the fix resolves it and matches the PR's reported output across all five test files, and confirmed compile-form's :multi-fn-template normalization is the correct (not just reachable) entry point here.

@nnunley

nnunley commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Approved and green at cb90b439a05e2d3ee04ac1ae3709babe8d31747f, but GitHub currently reports this branch as behind main (now 3aa9f4ea86d41dd55ced66fb3ca800c4bc66c355 after #800).

@mparrett, please update/rebase the contributor-owned branch and let CI rerun. I will not push to it. Once it is current-base and fully green, it is next eligible to land.

@mparrett
mparrett force-pushed the fix/ys-lower-report-public-api branch from cb90b43 to 1710dac Compare September 7, 2026 17:22
@mparrett

mparrett commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto 3aa9f4ea and force-pushed; head is now 1710dacc. Clean rebase, diff unchanged — the same two lines in examples/aot/yamlstar/ys-lower-report.lg.

Re-verified on the new base. The pre-fix script still fails at the same line, and the fixed version reproduces the PR body's table: fib.lg ok=2, lib.lg ok=2, closure_capture.lg ok=2, loop_carried_closure.lg ok=3, conditional_closure.lg ok=4, fallback=0 threw=0 throughout.

CI is green on the new head: all 15 checks pass.

@nnunley

nnunley commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Thanks for updating this to 1710daccaea872d062c8f722538bba2e8d479ca2; its fresh checks are all green. Main advanced once more when the previously blocked #764 cleared and merged as 638b4a6aa1ab0aaadcfa42d29458e02bcbcd7bdd, so the PR is again one base step behind our exact-current-base merge rule.

@mparrett, please update the contributor-owned branch once more and let CI rerun. I will not push it. I’ll merge after confirming the new head/base, approval, mergeability, and every required check.

@mparrett
mparrett force-pushed the fix/ys-lower-report-public-api branch from acfe213 to 9ee86d1 Compare September 7, 2026 19:58
@mparrett

mparrett commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Updated again: head is now 9ee86d1f, a single commit on 638b4a6a.

The branch had picked up a merge commit from the Update-branch button (acfe213b, "Merge branch 'main' into fix/ys-lower-report-public-api"). I replaced it with a rebase, so the branch is one commit on current main rather than a merge. main requires linear history here, and it keeps the head/base check unambiguous.

Re-verified on the new base, since #764 changes small-map ordering and the IR pipeline reads those maps: the five files still report ok=2, ok=2, ok=2, ok=3, ok=4, with fallback=0 threw=0.

All 15 checks green.

The tool has been unable to run since compile-form* became private: every
invocation dies at load with "Can't resolve ir.passes.pipeline/compile-form* in
this context" before reporting on a single form.

compile-form is the public wrapper over it, and is the right entry point for
this caller for a second reason. It normalises a :multi-fn-template result to
:status :lowered, which its own comment explains is there for callers that key
off :status — otherwise a form that lowered via lambda-lift reads as unlowered.
ys-lower-report is exactly such a caller: it branches on :status and prints
anything that is not :lowered as a fallback.

Verified by running the tool over five in-repo namespaces, which is the whole
of its contract:

  examples/aot/native-entry/fib.lg            ok=2 fallback=0 threw=0
  examples/aot/cross-package/src/lib.lg       ok=2 fallback=0 threw=0
  test/native-entry/closure_capture.lg        ok=2 fallback=0 threw=0
  test/native-entry/loop_carried_closure.lg   ok=3 fallback=0 threw=0
  test/native-entry/conditional_closure.lg    ok=4 fallback=0 threw=0

fib.lg's ok=2 agrees with the "2 fns lowered" that lg-compile reports for the
same file. The three closure fixtures are the lambda-lift shapes, though they
only demonstrate that the fixed tool reports them as lowered — there is no
before-and-after to compare, since nothing ran before.

The header comment is updated to match. Other references to compile-form* in
the tree describe the private function itself and stay as they are.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@mparrett
mparrett force-pushed the fix/ys-lower-report-public-api branch from 9ee86d1 to 782b7f8 Compare September 7, 2026 20:57
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.

3 participants