From d4927b2297f515be2aa504da14b7672090ca20eb Mon Sep 17 00:00:00 2001 From: Brummel Date: Mon, 11 May 2026 09:29:21 +0200 Subject: [PATCH] plan: ct.3 codegen + mono cleanup --- .../2026-05-11-ct.3-codegen-mono-cleanup.md | 478 ++++++++++++++++++ 1 file changed, 478 insertions(+) create mode 100644 docs/plans/2026-05-11-ct.3-codegen-mono-cleanup.md diff --git a/docs/plans/2026-05-11-ct.3-codegen-mono-cleanup.md b/docs/plans/2026-05-11-ct.3-codegen-mono-cleanup.md new file mode 100644 index 0000000..1610530 --- /dev/null +++ b/docs/plans/2026-05-11-ct.3-codegen-mono-cleanup.md @@ -0,0 +1,478 @@ +# Iteration ct.3: Codegen + Mono Cleanup — Implementation Plan + +> **Parent spec:** `docs/specs/2026-05-10-canonical-type-names.md` +> +> **For agentic workers:** REQUIRED SUB-SKILL: use `skills/implement` +> to run this plan. Steps use `- [ ]` checkboxes for tracking. + +**Goal:** Delete the codegen-side `lookup_ctor_by_type` imports- +fallback (iter 23.1.4) and narrow `apply_per_module_ctor_index_overlay` +in mono.rs to types-only — its `env.ctor_index` half became +decorative once ct.2.2's Pattern::Ctor lookup stopped consulting +the index. Also refresh two stale doc-comments left behind by ct.2. + +**Architecture:** Three tasks across two crates. Task 1 simplifies +`Codegen::lookup_ctor_by_type` to two-branch direct lookup +(qualified or local; bare-non-local fails) — symmetric to ct.2.3's +Term::Ctor synth fix. Task 2 narrows the mono overlay so it scopes +`env.types` per-module (still needed for Term::Ctor bare lookup +when re-running synth in the mono pass) but drops the +`env.ctor_index` clear/rebuild (no consumer left). Task 3 is a +small doc-comment refresh in `crates/ail/src/main.rs` retiring the +"parity with typechecker `ambiguous-type` diagnostic" paragraph +that became obsolete in ct.2.3. + +**Tech Stack:** `crates/ailang-codegen/src/lib.rs` (lookup fn), +`crates/ailang-check/src/mono.rs` (overlay fn + two call sites), +`crates/ail/src/main.rs` (doc-comment only). + +--- + +## Files this plan creates or modifies + +- Modify: `crates/ailang-codegen/src/lib.rs:1690-1766` — replace + `lookup_ctor_by_type`'s three-branch lookup with two-branch + (qualified or local). Refresh the docstring at line 1690 and + the lockstep-rationale comment at line 380. +- Modify: `crates/ailang-check/src/mono.rs:386-434` — rename + `apply_per_module_ctor_index_overlay` to + `apply_per_module_types_overlay`; drop the + `env.ctor_index.clear()` + insertion lines; rewrite the + module-level rationale comment. +- Modify: `crates/ailang-check/src/mono.rs:160-167, 228-237` — + the two call sites (in `monomorphise_workspace` and + `collect_residuals_ordered`); rename the calls. +- Modify: `crates/ail/src/main.rs:228-250` — drop the obsolete + paragraph mentioning `ambiguous-type` parity. + +No files are created. No files are deleted. + +--- + +## Task 1: Delete codegen `lookup_ctor_by_type` imports-fallback + +The codegen-side helper at `crates/ailang-codegen/src/lib.rs:1698` +currently has three branches: + +1. Qualified `module.Type`: resolve via `import_map.get(prefix)` → + `module_ctor_index[target_module][ctor_name]`, validate + `cref.type_name == suffix`. (Clean, stays.) +2. Local hit (bare `type_name`): look up + `module_ctor_index[self.module_name][ctor_name]`; accept if + `cref.type_name == type_name`. (Stays — bare-canonical means + local.) +3. Imports-walk fallback (iter 23.1): scan every other module's + ctor_index for `(ctor_name)` with `cref.type_name == type_name`, + first hit wins. **Delete.** Post-ct.1+ct.2, every Term::Ctor + reaching codegen carries a canonical `type_name` (bare = local, + qualified = explicit cross-module); the imports-walk is + structurally unreachable. + +The fail-closed path: when the local hit's `type_name` does not +match the requested `type_name`, return +`CodegenError::Internal("unknown ctor `{ctor_name}` for type +`{type_name}` in module `{self.module_name}`")`. The lookup +function returns `Err` for any non-canonical input. + +**Files:** +- Modify: `crates/ailang-codegen/src/lib.rs:1690-1766` — + `lookup_ctor_by_type` body + its docstring. +- Modify: `crates/ailang-codegen/src/lib.rs:378-385` — the + comment block at the top of `build_check_env`-related code that + cites "lockstep with Term::Ctor typecheck / lookup_ctor_by_type + codegen" needs a tiny touch-up to remove the "imports-fallback" + framing. + +- [ ] **Step 1: Run the current test suite to confirm baseline** + +Run: `cargo test --workspace` + +Expected: all pass (this is the post-ct.2 baseline; the test +count is 451). + +- [ ] **Step 2: Apply the production fix — collapse to two branches** + +Edit `crates/ailang-codegen/src/lib.rs:1698-1766`. Replace the +function body with: + +```rust + pub(crate) fn lookup_ctor_by_type( + &self, + type_name: &str, + ctor_name: &str, + ) -> Result { + if type_name.matches('.').count() == 1 { + let (prefix, suffix) = type_name.split_once('.').expect("checked"); + let target_module = self.import_map.get(prefix).cloned().ok_or_else(|| { + CodegenError::Internal(format!( + "qualified ctor `{type_name}/{ctor_name}`: prefix `{prefix}` not in import map" + )) + })?; + let cref = self + .module_ctor_index + .get(&target_module) + .and_then(|m| m.get(ctor_name)) + .cloned() + .ok_or_else(|| { + CodegenError::Internal(format!( + "qualified ctor `{type_name}/{ctor_name}` not in module `{target_module}`" + )) + })?; + if cref.type_name != suffix { + return Err(CodegenError::Internal(format!( + "ctor `{ctor_name}` belongs to `{}`, not `{type_name}`", + cref.type_name + ))); + } + Ok(cref) + } else { + // ct.3 Task 1: bare type_name is canonical (post-ct.1 + // validator) ⇒ local. Hit the current module's ctor + // table directly; non-match is a hard error. No + // imports-walk — the typechecker has already pinned + // type_name to the owning module. + let cref = self + .module_ctor_index + .get(self.module_name) + .and_then(|m| m.get(ctor_name)) + .cloned() + .ok_or_else(|| { + CodegenError::Internal(format!( + "unknown ctor `{ctor_name}` for type `{type_name}` in module `{}`", + self.module_name + )) + })?; + if cref.type_name != type_name { + return Err(CodegenError::Internal(format!( + "ctor `{ctor_name}` belongs to local type `{}`, not `{type_name}`; \ + cross-module ctor refs require qualified type_name", + cref.type_name + ))); + } + Ok(cref) + } + } +``` + +- [ ] **Step 3: Refresh the lookup_ctor_by_type docstring at lib.rs:1690** + +Edit `crates/ailang-codegen/src/lib.rs:1690-1697`. Replace the +existing docstring (which references the imports-fallback + +`AmbiguousType` that no longer exist) with: + +```rust + /// Resolves a `Term::Ctor.type_name` (canonical post-ct.1: bare + /// = local TypeDef, qualified `.` = explicit + /// cross-module) to the codegen-side `CtorRef`. Qualified names + /// route through `import_map`; bare names hit the current + /// module's `module_ctor_index` directly. No imports-walk + /// fallback — the typechecker (post-ct.2) and the workspace + /// validator (post-ct.1) have already pinned canonical form. +``` + +- [ ] **Step 4: Refresh the lockstep-rationale comment at lib.rs:378-385** + +Edit `crates/ailang-codegen/src/lib.rs` at the comment block that +cites "lockstep with Term::Ctor typecheck / lookup_ctor_by_type +codegen". Find the block (search for `Term::Ctor typecheck / +lookup_ctor_by_type` near line 380) and replace it with one that +names the post-ct.3 reality: every Type::Con name reaching +codegen is canonical; the lockstep is now structural, not +fallback-based. The exact rewrite depends on the surrounding +context — preserve the function-level comment's intent, drop only +the bits that cite the now-removed mechanism. Keep the comment +≤ 6 lines. + +- [ ] **Step 5: Build and run the full workspace test suite** + +Run: `cargo test --workspace` + +Expected: all pass. + +Rationale for skipping a dedicated RED test: this task is a pure +deletion of unreachable code. The existing E2E suite — in +particular `crates/ail/tests/mono_xmod_ctor_pattern.rs` (the +iter-23.2 cross-module ctor regression) and the +prelude-using examples in `examples/` — exercises every code path +the deletion touches. If the deletion accidentally removes a live +path, those tests would fail. A new RED test would only re-pin +what the existing tests already pin. + +- [ ] **Step 6: Commit** + +```bash +git add crates/ailang-codegen/src/lib.rs +git commit -m "iter ct.3.1: codegen lookup_ctor_by_type direct lookup; delete imports-fallback" +``` + +--- + +## Task 2: Narrow mono overlay to types-only + +The function `apply_per_module_ctor_index_overlay` at +`crates/ailang-check/src/mono.rs:418-434` exists to scope +`env.types` AND `env.ctor_index` to the current module's local +TypeDefs before the mono pass re-runs `synth`. The rationale, +spelled out at mono.rs:386-413, has two halves: + +- Half A: scope `env.types` per-module so a bare-name `Term::Ctor` + lookup in mono's re-run synth resolves to the current module's + local TypeDef (NOT some workspace-wide bare-name collision). + **Still load-bearing.** Term::Ctor synth at lib.rs:1982 still + consults `env.types.get(type_name)` for bare canonical names. +- Half B: scope `env.ctor_index` per-module so the Pattern::Ctor + `env.ctor_index.get(ctor)` lookup found local hits first and the + imports-walk fallback could produce qualified type names. + **Obsolete post-ct.2.2.** Pattern::Ctor no longer consults + `env.ctor_index`; it walks `env.module_types` directly from the + scrutinee's qualified type name. + +This task drops Half B and renames the function to reflect Half A +only. + +**Files:** +- Modify: `crates/ailang-check/src/mono.rs:386-434` — the + function + its docstring. +- Modify: `crates/ailang-check/src/mono.rs:160-167` — the call + site inside `monomorphise_workspace`. +- Modify: `crates/ailang-check/src/mono.rs:228-237` — the call + site inside `collect_residuals_ordered`. + +- [ ] **Step 1: Run the current test suite to confirm baseline** + +Run: `cargo test --workspace` + +Expected: all pass. + +- [ ] **Step 2: Replace the overlay function body and rename it** + +Edit `crates/ailang-check/src/mono.rs:418-434`. Replace the +function with the narrowed types-only form: + +```rust +fn apply_per_module_types_overlay(env: &mut crate::Env, ws: &Workspace, module_name: &str) { + env.types.clear(); + if let Some(m) = ws.modules.get(module_name) { + for d in &m.defs { + if let Def::Type(td) = d { + env.types.insert(td.name.clone(), td.clone()); + } + } + } +} +``` + +The body loses the inner `for c in &td.ctors { env.ctor_index.insert(...) }` +loop and the `env.ctor_index.clear()` line. + +- [ ] **Step 3: Replace the function docstring at mono.rs:386-417** + +Edit the docstring block above the function (lines ~386-417). +Replace it with a shorter rationale that names only the +still-live purpose: + +```rust +/// Rebuild `env.types` to contain only the TypeDefs declared in +/// `module_name`'s `Def::Type`s, mirroring `check_in_workspace`'s +/// per-module overlay at `crates/ailang-check/src/lib.rs:1234`. +/// +/// The mono pass re-runs `crate::synth` on every fn body to +/// recover residual class constraints. `synth`'s `Term::Ctor` arm +/// looks up bare canonical `type_name`s via `env.types.get(name)` +/// (lib.rs:1982); without this overlay, `env.types` would be the +/// workspace-flat map built by `build_workspace_env`, and a bare +/// `type_name` in module A could resolve to a same-named TypeDef +/// from module B. The overlay restores per-module bare-name +/// scoping. +/// +/// Iter ct.3 (this iter): the `env.ctor_index` half of the +/// original 22c overlay is gone. Pattern::Ctor post-ct.2.2 walks +/// `env.module_types` from the scrutinee's qualified `Type::Con` +/// name directly; it no longer consults `env.ctor_index`. +/// +/// Caller-contract assumption: the workspace has already +/// typechecked, so duplicate type names within a single module +/// cannot occur. +``` + +- [ ] **Step 4: Update the two call sites** + +Edit `crates/ailang-check/src/mono.rs:160-167`. Replace: + +```rust + // Per-module ctor_index overlay — see + // [`apply_per_module_ctor_index_overlay`] for rationale. +``` + +(and however the surrounding code reads — locate the call by +searching for `apply_per_module_ctor_index_overlay(&mut env_mod`) +with: + +```rust + // Per-module env.types overlay — see + // [`apply_per_module_types_overlay`] for rationale. +``` + +Then update the function call itself: + +```rust + apply_per_module_types_overlay(&mut env_mod, &ws_owned, mname); +``` + +Do the same at the second call site (mono.rs:228-237), again +matching the comment header and the function name. + +- [ ] **Step 5: Build and run the full workspace test suite** + +Run: `cargo test --workspace` + +Expected: all pass. In particular, the +`mono_pass_handles_xmod_ctor_pattern` regression test (which +exercises cross-module ctor patterns through mono) must continue +to pass — its scrutinee carries a qualified type and the +Pattern::Ctor lookup walks `env.module_types` directly, +independently of `env.ctor_index`. + +- [ ] **Step 6: Commit** + +```bash +git add crates/ailang-check/src/mono.rs +git commit -m "iter ct.3.2: mono overlay narrowed to env.types only; rename to apply_per_module_types_overlay" +``` + +--- + +## Task 3: Retire the obsolete `migrate-canonical-types` paragraph in ail/main.rs + +The doc-comment at `crates/ail/src/main.rs:228-250` (the +`MigrateCanonicalTypes` variant) has a "Note: this is one-shot +dev tooling and intentionally weaker than the typecheck-time +`ambiguous-type` diagnostic shipped in iter 23.1.3" paragraph. + +The `ambiguous-type` diagnostic was removed in ct.2.3. The +paragraph now references a defunct diagnostic and frames a +parity argument with a comparand that no longer exists. + +**Files:** +- Modify: `crates/ail/src/main.rs:228-250` — drop the paragraph. + +- [ ] **Step 1: Confirm the location** + +Run: `grep -n "ambiguous-type" crates/ail/src/main.rs` + +Expected: a single hit around line 239. + +- [ ] **Step 2: Drop the obsolete paragraph** + +Edit `crates/ail/src/main.rs`. Find the doc-comment block on +`MigrateCanonicalTypes` (around lines 228-250). Delete the +paragraph that starts with "Note: this is one-shot dev tooling +and intentionally weaker..." and ends with "...enforce the +stricter rule going forward." Keep the rest of the docstring +(the schema-level description, the "first import wins" +disambiguation note). + +The resulting docstring should be: + +```rust + /// ct.1 (dev-only): rewrite every `*.ail.json` under `` so + /// that bare cross-module `Type::Con` and `Term::Ctor.type_name` + /// references gain their owning module's qualifier. Idempotent. + /// + /// Disambiguation: the first import (in declaration order) that + /// owns a matching type wins. Ties are NOT diagnosed — the + /// migration silently picks the first match. Prelude is consulted + /// as an implicit last-resort fallback when no listed import owns + /// the name. + MigrateCanonicalTypes { + /// Directory containing `*.ail.json` to migrate (typically + /// `examples/`). + dir: PathBuf, + }, +``` + +- [ ] **Step 3: Build to confirm the doc-comment still parses** + +Run: `cargo build --workspace` + +Expected: clean build, no warnings related to the doc-comment. + +- [ ] **Step 4: Commit** + +```bash +git add crates/ail/src/main.rs +git commit -m "iter ct.3.3: retire obsolete ambiguous-type paragraph in migrate-canonical-types docstring" +``` + +--- + +## Closing checks + +After all three tasks land: + +- [ ] **Step C1: Run the full test suite once more** + +Run: `cargo test --workspace` + +Expected: 451+ tests, 0 failed. + +- [ ] **Step C2: Confirm the four obsolete mechanisms catalogue is exhausted** + +The ct.1 JOURNAL entry named four mechanisms slated for retirement: +- ✅ `Pattern::Ctor` imports-fallback in ailang-check (ct.2.2). +- ✅ `Term::Ctor` synth imports-fallback in ailang-check (ct.2.3). +- ✅ `lookup_ctor_by_type` imports-fallback in ailang-codegen (ct.3.1). +- ✅ `apply_per_module_ctor_index_overlay`'s ctor_index half in + ailang-check/mono.rs (ct.3.2). + +All four are now gone. Iteration ct.3 closes the typechecker + +codegen + mono side of the milestone. Only ct.4 (DESIGN.md +amendments + hash re-baseline + prose round-trip extension + +optional fixture recreation for iter 23.3 Task 4+5) remains +before the canonical-type-names milestone closes. + +- [ ] **Step C3: JOURNAL entry** + +Append to `docs/JOURNAL.md`: + +```markdown +## 2026-05-11 — Iteration ct.3: codegen + mono cleanup + +Three tasks landed: + +- **ct.3.1**: deleted the `lookup_ctor_by_type` imports-fallback + in `crates/ailang-codegen/src/lib.rs`. Replaced with two-branch + direct lookup (qualified via `import_map` → + `module_ctor_index[target_module]`; bare via + `module_ctor_index[self.module_name]` with a hard type-name + match). Symmetric to ct.2.3's `Term::Ctor` synth fix on the + typecheck side. Docstring and the lockstep-rationale comment + refreshed. +- **ct.3.2**: narrowed `apply_per_module_ctor_index_overlay` to + `env.types` only, dropping the `env.ctor_index` half that became + decorative after ct.2.2 stopped consulting the index from + Pattern::Ctor. Renamed to `apply_per_module_types_overlay`. + Updated both call sites and the module-level rationale comment. +- **ct.3.3**: retired the obsolete "parity with `ambiguous-type` + diagnostic" paragraph in the `MigrateCanonicalTypes` doc-comment + (`crates/ail/src/main.rs`). The diagnostic it referenced was + removed in ct.2.3. + +All four obsolete mechanisms named in the ct.1 JOURNAL entry are +now gone. Only ct.4 remains in the canonical-type-names milestone: +DESIGN.md amendments, hash-pinned regression test re-baseline, +prose round-trip extension, and the optional +`examples/compare_primitives_smoke.ail.json` recreation that +demonstrates the end-to-end closure of the iter-23.3 Task 4 bug. + +Workspace at iter-ct.3 close: full `cargo build --workspace` and +`cargo test --workspace` green (test count unchanged from ct.2's +451; this iter is pure code deletion + doc tidy). +``` + +- [ ] **Step C4: Commit the JOURNAL entry** + +```bash +git add docs/JOURNAL.md +git commit -m "journal: ct.3 codegen + mono cleanup" +```