Skip to content

checker: stop eating record-literal fn bodies as effect annotations#85

Merged
ecto merged 2 commits into
mainfrom
claude/upbeat-maxwell-12e668
Jul 14, 2026
Merged

checker: stop eating record-literal fn bodies as effect annotations#85
ecto merged 2 commits into
mainfrom
claude/upbeat-maxwell-12e668

Conversation

@ecto

@ecto ecto commented Jul 14, 2026

Copy link
Copy Markdown
Owner

What

Fixes two type-checker bugs that made a valid program (one the EIR VM runs correctly) fail with [E0200] cannot unify Record{...} with ().

Repro that used to fail loon run but pass loon test:

[fn log-monoid [] {:unit "" :tensor [fn [m n] [str m n]]}]
[fn act [mon m w]
  [let t [get mon :tensor]]
  [let {log log val val} w]
  {:log [t m log] :val val}]
[fn pure [mon a] {:log [get mon :unit] :val a}]
[fn main []
  [let x [act [log-monoid] "step1;" [act [log-monoid] "step2;" [pure [log-monoid] "done"]]]]
  [let {log log val val} x]
  [IO.println [str "log: " log " value: " val]]]

Why

1. Record-literal fn bodies were swallowed as effect annotations. infer_named_fn consumed any Map right after the params as a {IO Fail}-style effect annotation (a "parser quirk" fallback). So [fn log-monoid [] {:unit "" :tensor ...}] had its entire body eaten, leaving nothing — the checker inferred the return type as (). That's the Record{tensor: ...} vs () mismatch: the record the function actually returns at runtime against the () the checker believed. All three runtimes (interp, machine, EIR lower) only skip Set(_), never Map, so the checker had diverged from execution.

Fix: a Map only counts as an annotation in the quirk form — all keys and values are bare symbols, and a body expression follows. Keyword-keyed maps (record literals) are the body.

2. Map destructuring collapsed record fields to one shared type. bind_map_destructure unified any non-Map subject with Map Keyword v and bound every key to the same v. Since keyword-keyed literals type as Records, destructuring a param like w both forced it away from record types and forced log and val to share one type.

Fix: keep the shared-element path for genuine Map Keyword v subjects; otherwise constrain per key with infer_record_get_lenient (open row, lenient like the total get builtin — a missing key binds () at runtime, so it's not a hard error), giving each field its own type.

Tests

Two regression tests in check::tests:

  • record_body_is_not_effect_annotation — record body plus a check that the {IO Fail} quirk form still parses as a declared effect set
  • map_destructure_record_fields_keep_own_types — the monoid program above, plus a heterogeneous {:log "x" :val 41} destructure

The repro now checks and prints log: step1;step2; value: done. Full workspace suite passes.

🤖 Generated with Claude Code

Two checker bugs made a program that runs fine on the EIR VM fail to
type-check with E0200 (cannot unify Record{...} with ()).

1. infer_named_fn treated ANY map right after the params as a
   `{IO Fail}`-style effect annotation (the parser-quirk fallback). So
   `[fn mk [] {:a 1}]` had its whole body swallowed and was inferred as
   returning (), breaking every caller. None of the three runtimes
   (interp, machine, EIR lower) skip a Map there — only Set — so the
   checker diverged from execution. A Map now only counts as an
   annotation in the quirk form: all keys and values bare symbols, with
   a body expression following.

2. bind_map_destructure unified any non-Map subject with
   `Map Keyword v` and bound every key to the same shared value type.
   Keyword-keyed literals are Records, so destructuring a param forced
   it away from record types AND forced all fields to share one type.
   Non-Map subjects are now constrained per key with an open record row
   (lenient, like the total `get` builtin: a missing key binds ()).

Adds regression tests for both.
@vercel

vercel Bot commented Jul 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
loon Ready Ready Preview, Comment Jul 14, 2026 2:05pm

Request Review

@chojiai

chojiai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Choji live review — Nothing blocking — a few notes — 1 minor

Choji review — Nothing blocking — a few notes

Both fixes are correct and well-targeted: the annotation-vs-body disambiguation is sound (all-symbol-keys + trailing body = annotation, anything else = body), and the per-key record-row path in bind_map_destructure correctly replaces the single shared element type. The prior open NITs are either resolved by the new body-requirement guard or remain minor style points that don't affect correctness.

1 minor finding
  • Minor · Design churn — Recently changed by someone else crates/loon-lang/src/check/mod.rs
    1 file in this PR was changed by other people in the last 14 days. If those were deliberate choices, make sure you're not undoing them — loop the recent authors in before merging:
  • crates/loon-lang/src/check/mod.rs — last changed by @claude 7 days ago

Verified by Choji

Choji ran your change live — checks passed.

1 check.

Claim Result Evidence
cargo check Verified exit 0 · 275s

The ticket asked for the type-checker to stop treating record-literal fn bodies as effect annotations and to fix map destructuring collapsing record fields to one type; the verification checks confirm the fix works correctly. No live preview was run since this is a compiler/type-checker change with no web UI surface. cargo check passed (exit 0), and per the PR description the full workspace test suite passes including the two new regression tests (record_body_is_not_effect_annotation and map_destructure_record_fields_keep_own_types). There is nothing observable in a running UI to verify, but the check results are the appropriate signal for this kind of change and they are clean.

No issues found in the running app.


Reviewed 45aa9f3 · Rate findings · Choji updates this comment as you push

chojiai[bot]
chojiai Bot previously approved these changes Jul 14, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@chojiai
chojiai Bot dismissed their stale review July 14, 2026 14:07

Dismissing prior approval to re-evaluate 45aa9f3.

@ecto
ecto merged commit 19ebade into main Jul 14, 2026
7 checks passed
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