From ff5dc7f4e737536fee8911279a92a34acb42f5fe Mon Sep 17 00:00:00 2001 From: Brummel Date: Tue, 12 May 2026 21:57:21 +0200 Subject: [PATCH] =?UTF-8?q?spec:=20ct-tidy=20=E2=80=94=20canonical-type-na?= =?UTF-8?q?mes=20follow-up=20sweep?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/specs/2026-05-12-ct-tidy.md | 423 +++++++++++++++++++++++++++++++ 1 file changed, 423 insertions(+) create mode 100644 docs/specs/2026-05-12-ct-tidy.md diff --git a/docs/specs/2026-05-12-ct-tidy.md b/docs/specs/2026-05-12-ct-tidy.md new file mode 100644 index 0000000..57a8ef4 --- /dev/null +++ b/docs/specs/2026-05-12-ct-tidy.md @@ -0,0 +1,423 @@ +# Canonical-Type-Names Tidy Sweep — Design Spec + +**Date:** 2026-05-12 +**Status:** Draft — awaiting user spec review +**Authors:** Brummel (orchestrator) + Claude + +## Goal + +Close three follow-up todos that the canonical-type-names milestone +(ct.1 → ct.4, closed 2026-05-11) explicitly deferred or surfaced. +All three touch workspace / env / registry mechanics and share the +theme "structural gaps the canonical-type-names milestone declared +out-of-scope". Bundling them into one tidy milestone keeps the +closeout coherent rather than scattering three orphan tidy patches +across the next several weeks. + +The three items: + +1. **env-overlay shape ratification + narrowing.** Decide whether + the env's parallel `types` / `ctor_index` maps should collapse + into one overlay or stay split, anchor the decision in + `docs/DESIGN.md`, and narrow `check_in_workspace`'s per-module + overlay rebuild symmetric to ct.3.2's mono-side fix. +2. **`Registry.type_def_module` re-key.** Replace the + `BTreeMap` keyed by bare type name with a key + that survives cross-module bare-name collisions. +3. **`KindMismatch` retire.** Delete the + `WorkspaceLoadError::KindMismatch` variant and `walk_kind_mismatch` + helper — structurally unreachable post-ct.1, ratified by the + already-green test `class_param_in_applied_position_fires_canonical_form_rejection` + at `crates/ailang-core/src/workspace.rs:1867`. + +Items deliberately **not** in scope: CLI human-mode diagnostic +surface for `WorkspaceLoadError` (different layer — diagnostic +routing, not workspace mechanics); Split `BadCrossModuleTypeRef` +(diagnostic shape); Workspace search beyond entry-module's +directory (CLI / IO layer); `io/print_float` `.0` fallback +(runtime printer asymmetry); Boehm retirement (memory +subsystem); rustdoc warning sweep (documentation). Each of these +is a coherent tidy in its own right but does not share the +workspace-mechanics theme. + +## Architecture + +The milestone is three iterations, ordered from highest-design- +substance to lowest. The order is deliberate: the env-overlay +decision in ctt.1 is the only one carrying a real choice, so it +happens first while the design context is fresh. ctt.2 and ctt.3 +are mechanical follow-ups whose shape does not depend on ctt.1. + +### Iteration ctt.1 — env-overlay shape ratification + narrowing + +**Design substance:** Today `Env` carries two parallel maps that +are rebuilt in lockstep: + +- `env.types: IndexMap` — bare-type-name → + full TypeDef. The owning index for type definitions. +- `env.ctor_index: IndexMap` — bare-ctor-name → + owning-type-name. A reverse index over `env.types`. + +The "shape question" from `docs/journals/2026-05-10` ("Audit close: +env-construction unify") is: collapse into one overlay +(`Map` with `Type(TypeDef) | Ctor(owner)`) or +stay split? + +**Decision: stay split.** Reason (semantic, not effort): the two +maps have distinct semantic roles. `env.types` is an *owning* +index — it stores the data. `env.ctor_index` is a *reverse* +index — it accelerates `ctor-name → type-name` lookup over the +same data. A collapsed variant-keyed map would conflate these +roles and force every consumer to discriminate `EntryKind` at +the call site. The current split lets type-consumers iterate +`env.types` directly and ctor-consumers query `env.ctor_index` +directly; neither needs to know the other exists. + +This rationale gets anchored in `docs/DESIGN.md` as a new +top-level section `## Env construction`, placed between the +existing `## Convention: qualified cross-module references` +(`docs/DESIGN.md:1941`) and `## Data model` (`docs/DESIGN.md:1959`). +DESIGN.md today has no env / overlay section, so this iter +introduces the heading. The initial body is one short paragraph +naming the split (`env.types` owning vs. `env.ctor_index` +reverse-index) with the rationale above; future env / overlay +mechanics can extend the section without re-anchoring. Future +tidies cannot re-litigate the split without first amending this +section. + +**Code change: one new pinning test.** The DESIGN.md anchor is +the primary deliverable, plus the roadmap close. The iter also +adds one positive test that pins the load-bearing +`DuplicateCtor` consumer, because the in-band check at +`crates/ailang-check/src/lib.rs:1366-1376` has no in-tree test +ratifying it today (the variant is emitted in the production +code path but no fixture currently triggers it). The pinning +test is what makes the ratification load-bearing rather than +documentation-of-belief — the asymmetry-with-mono-side argument +rests on this consumer being real. + +Inspection of the existing per-module rebuild at +`crates/ailang-check/src/lib.rs:1342-1384` confirms both halves +are load-bearing: + +- `env.types.clear()` + rebuild: needed for the local + `DuplicateType` diagnostic at `lib.rs:1358-1364` and for + bare-name `Type::Con` lookup against the current module. +- `env.ctor_index.clear()` + rebuild: emitted into the in-band + `DuplicateCtor` diagnostic at `lib.rs:1366-1376`, which + consults the rebuilt per-module `env.ctor_index` to detect + duplicates *within the current module*. The new pinning test + is the consumer evidence. + +The ct.3.2 mono-side fix narrowed the *mono overlay* because the +mono-side `env.ctor_index` was only feeding runtime ctor lookup +(type-driven post-ct.2.2). The check-side overlay is shaped +differently — it serves the workspace-build-time duplicate +diagnostic — so the analogous narrowing does not apply. + +The roadmap todo "check_in_workspace per-module overlay narrowing" +closes via this finding: the rebuild stays, the asymmetry with +the mono-side is intentional and now documented. The roadmap +todo "types / ctor_index overlay shape question" closes via the +DESIGN.md anchor recording the split rationale. + +If a future recon surfaces a non-DuplicateCtor consumer of the +post-rebuild `env.ctor_index` that *could* survive removal of +the per-module rebuild, the question re-opens; until then the +ratification holds. + +### Iteration ctt.2 — `Registry.type_def_module` re-key + +**Bug being closed:** `crates/ailang-core/src/workspace.rs:106` +declares `type_def_module: BTreeMap`. Today this +map is bare-name → defining-module. The doc-comment at +`workspace.rs:97-105` flags the issue: if two modules each +declare `type Foo`, the second insert at +`workspace.rs:547` silently overwrites the first; subsequently +`normalize_type_for_registry` (`workspace.rs:892`) qualifies +`Foo` to whichever module won the insert race. + +For the current corpus (every in-tree fixture uses distinct bare +type names across modules) the bug is dormant. The proper fix is +explicitly written into the doc-comment: re-key as +`(owning_module, bare_name) -> defining_module` and thread the +calling module through every consumer site. + +**Re-keying choice.** Two shapes are conceivable: + +- `BTreeMap<(String, String), String>` keyed by + `(owning_module, bare_name)` — straightforward extension. +- `BTreeMap>` keyed by bare name, + value being a list of `(owner, defining_module)` pairs — + preserves the bare-name-as-primary-lookup intuition but forces + every consumer to handle the multi-candidate case. + +**Decision: tuple key.** Reason (semantic): the function the map +serves — "given a type reference written in module M, where is +its type defined?" — already requires the owning module M as +input. The tuple key makes that input explicit at the API +surface. The list-valued alternative would defer +multi-candidate handling to every consumer. + +**Consumer site count.** `grep "type_def_module"` returns 13 hits +in `workspace.rs`. Most are inside the workspace-load function +where the calling module is local context (loop variable +`mod_name`), so threading is trivial. The interesting site is +`normalize_type_for_registry` at `workspace.rs:879-928`, which +today takes `type_def_module: &BTreeMap` as a +parameter. The signature changes to accept the calling module +as well, and the map type changes to +`&BTreeMap<(String, String), String>`. All call sites already +have the calling module in scope (the workspace-load loop +threads it through `mod_name`). + +The `Registry::normalize_type_for_lookup` method at +`workspace.rs:121-123` exposes `normalize_type_for_registry` +publicly without a calling-module argument. The signature gains +a `caller_module: &str` argument. The two test-only call sites +at `workspace.rs:2525, 2549` use synthetic modules; both update +trivially. + +**Test:** add a positive fixture where two modules each declare +`type Foo` with distinct ctors, an instance on each, and confirm +both instances resolve correctly under registry lookup. This +catches the regression: under the old bare-name key, one of the +two instances would silently lose its registry entry. + +### Iteration ctt.3 — `KindMismatch` retire + +**Path being deleted:** + +- `WorkspaceLoadError::KindMismatch` variant + (`crates/ailang-core/src/workspace.rs:259-264`). +- `walk_kind_mismatch` helper + (`crates/ailang-core/src/workspace.rs:834-854`). +- Dispatch site in `validate_classdefs` + (`crates/ailang-core/src/workspace.rs:786-795`). +- Display arm in `crates/ail/src/main.rs:1163` for the + human-mode diagnostic. + +**Ratification:** Already-green test +`class_param_in_applied_position_fires_canonical_form_rejection` +(`crates/ailang-core/src/workspace.rs:1867-1888`) demonstrates +that the malformed shape `class C f { foo : f Int -> f Int }` +fires `BareCrossModuleTypeRef` before `validate_classdefs` +runs. This is the structural-unreachability claim made by the +roadmap entry, ratified by a workspace-loaded test that +currently passes. + +The test must keep passing after the deletion. It asserts on +`BareCrossModuleTypeRef`, not `KindMismatch`, so no test edit +is required. + +**Defensive vs. retired:** The doc comment at +`workspace.rs:1858-1866` reads "stays in the codebase as +dead-but-defensive code; a future tidy may retire it." This +iter is that future tidy. The "defensive" framing was a +deliberate hedge during ct.1 in case a canonical-form bug let +malformed shapes through later — none has surfaced. Carrying +the dead branch indefinitely is itself a hazard: future +refactors of the workspace error-enum touch shapes that no +code path produces. Retire now, before the dead branch +accumulates churn. + +If a future canonical-form bug *does* let a malformed shape +through, the right repair is to fix the canonical-form +validator, not to resurrect `KindMismatch`. The diagnostic +that fires today (`BareCrossModuleTypeRef`) is structurally +correct: a class param `f` used as `f Int` is, post-ct.1, a +reference to a non-existent type `f` in the owning module — +that *is* a bare-cross-module type ref. The earlier +`KindMismatch` diagnostic was a less-precise pre-ct.1 framing. + +## Components + +### Files touched per iter + +**ctt.1 — env-overlay shape ratification + DuplicateCtor pin:** +- `docs/DESIGN.md` — new top-level section `## Env construction` + inserted between `## Convention: qualified cross-module + references` (line 1941) and `## Data model` (line 1959). + Initial body: one paragraph naming the `env.types` / + `env.ctor_index` split with the owning-vs-reverse-index + rationale. +- `crates/ailang-check/tests/` — new test (path TBD by planner — + candidate: a new file `duplicate_ctor_pin.rs`, or appended to + an existing fixture-driven test module) that builds a one- + module fixture with two `Def::Type`s declaring same-named + ctors and asserts `check_workspace` returns + `CheckError::DuplicateCtor { ctor, a, b }`. The exact + assertion path follows the existing `CheckError`-asserting + test conventions in the crate. +- `docs/roadmap.md` — strike both relevant todo lines + ("types / ctor_index overlay shape question" and + "check_in_workspace per-module overlay narrowing") with a + reference to `docs/journals/.md`. + +**ctt.2 — `Registry.type_def_module` re-key:** +- `crates/ailang-core/src/workspace.rs:106` — field type change + to `BTreeMap<(String, String), String>`. +- `crates/ailang-core/src/workspace.rs:121-123` — + `Registry::normalize_type_for_lookup` gains `caller_module: &str`. +- `crates/ailang-core/src/workspace.rs:537, 547` — Pass 1 + collection: insert `(mod_name.clone(), t.name.clone())` instead + of `t.name.clone()`. +- `crates/ailang-core/src/workspace.rs:656-657` — + `type_def_module.get(&type_repr)` becomes a per-module-aware + lookup (the call-site context provides `mod_name`). +- `crates/ailang-core/src/workspace.rs:879-928` — + `normalize_type_for_registry` signature + body adapt to tuple + key + `caller_module` argument. +- `crates/ailang-core/src/workspace.rs:2525, 2549` — test-only + call sites update. +- `crates/ailang-core/src/workspace.rs:97-105` — doc-comment + flagging the bug is replaced with the new shape's invariant. +- New test fixture: `examples/ct_tidy_cross_module_foo_collision.ail.json` + (or analogous), exercising two-module bare-name collision. +- `crates/ailang-core/tests/` or `crates/ail/tests/e2e.rs` — + test that exercises the new fixture and asserts both + instances resolve correctly. + +**ctt.3 — `KindMismatch` retire:** +- `crates/ailang-core/src/workspace.rs:259-264` — variant deletion. +- `crates/ailang-core/src/workspace.rs:786-795` — dispatch site + deletion (the `walk_kind_mismatch(&method.ty, &c.param)` call + inside `validate_classdefs`). +- `crates/ailang-core/src/workspace.rs:834-854` — helper deletion. +- `crates/ail/src/main.rs:1163` — Display arm deletion. +- `crates/ailang-core/src/workspace.rs:1858-1866` — the + "dead-but-defensive" doc comment on the existing test gets + dropped (the test stays; its rationale is now "the canonical-form + rejection is the path", which the test name already states). + +### Files NOT touched + +- `crates/ailang-check/src/lib.rs:1342-1384` — the per-module + overlay rebuild stays as-is after ctt.1's recon confirms + `DuplicateCtor` depends on it. The narrowing question closes + via DESIGN.md ratification, not code change. If planner-recon + for ctt.1 surfaces a consumer that *would* survive narrowing, + this changes; default expectation is no edit here. +- `crates/ailang-codegen/` — no codegen edits. None of the three + items touch IR or runtime. +- Snapshots — no IR-shape snapshots are affected. The bench + baseline is not affected (no codepath frequency change). + +## Data flow + +The three iters do not interact at the data level: + +- ctt.1 records a decision in DESIGN.md; no code-side data flow + change. +- ctt.2 changes the registry's internal key shape but preserves + the public `Registry::normalize_type_for_lookup` semantics for + every call site that already has its calling module in scope. + External canonical-form behaviour is unchanged for the current + corpus (which has no bare-name collisions); the fix is + load-bearing only when a future workspace introduces a + collision. +- ctt.3 deletes an unreachable branch; no data flow. + +Canonical-form bytes: unchanged. None of the three iters touches +serialised AST shapes. ctt.2's registry-key change is internal — +the canonical-form output of `normalize_type_for_registry` for +any *valid* input is the same as today (the bug surfaces only on +the malformed-collision input that the current corpus doesn't +contain, and the new behaviour is "qualify to the correct +owner" rather than "qualify to whichever module won the race", +which is by definition a fix not a shape change). + +## Error handling + +**ctt.1:** no new diagnostics. The ratification is documentation- +only at the analyser level. + +**ctt.2:** no new diagnostics. The `DuplicateInstance` / +`OrphanInstance` paths at `workspace.rs:682, 660` already cover +the collision-error space; the re-key fixes the silent +collision by ensuring the right registry entry survives, which +means a previously-masked instance becomes visible to the +existing `DuplicateInstance` detector when its canonical-form +key matches another (and is not falsely promoted to "duplicate" +when it actually belongs to a different owner). + +**ctt.3:** one diagnostic variant goes away. The Display path +in `main.rs:1163` deletes the human-mode `[kind-mismatch]` +line; the structurally-identical class-param-in-applied-position +malformed shape now produces `[bare-cross-module-type-ref]` +instead. Existing user-facing diagnostic for the canonical +malformed-fixture is the latter (the existing test confirms +this). + +## Testing strategy + +**ctt.1:** +- New pinning test: one-module fixture with two `Def::Type`s + declaring same-named ctors. Asserts `check_workspace` returns + `CheckError::DuplicateCtor { ctor, a, b }` with `a` and `b` + naming the two types. Catches any future regression that + silently removes the per-module `env.ctor_index` rebuild — + if the rebuild goes away, this test goes red, which is + exactly the load-bearing-asymmetry-argument's safety net. +- `cargo test --workspace` continues to pass with all other + tests unchanged. +- DESIGN.md anchor presence verified manually (no schema-drift + test exists for the env section yet — see roadmap item + "design_schema_drift.rs fidelity widening", out-of-scope here). +- Roadmap close: both relevant lines struck through. + +**ctt.2:** +- New fixture: two modules each declaring `type Foo` with + distinct ctors; an instance on each. Test: both instances + registerable, both retrievable by their canonical-form + registry key, no `DuplicateInstance` false positive, no + silent overwrite. +- Existing test suite must continue to pass (no current fixture + exercises the collision case, so no existing test regresses). + +**ctt.3:** +- Existing test `class_param_in_applied_position_fires_canonical_form_rejection` + (`crates/ailang-core/src/workspace.rs:1867`) continues to + pass, asserting `BareCrossModuleTypeRef` on the malformed + fixture. +- `cargo test --workspace` continues to pass (no other test + references `KindMismatch` — `grep KindMismatch` returns the + three deletion sites only). +- `cargo build --workspace` succeeds (no orphan match arms, + no dead-code warnings introduced). + +**Workspace-wide gate (all three iters):** +- `bench/cross_lang.py`, `bench/compile_check.py`, + `bench/check.py` all green. No metric should plausibly + shift, but the audit at milestone close confirms. + +## Acceptance criteria + +- `docs/DESIGN.md` carries a new top-level section + `## Env construction` (placed between `## Convention: + qualified cross-module references` and `## Data model`) + whose initial paragraph anchors the `env.types` / + `env.ctor_index` split with the semantic rationale (owning + index vs. reverse index). +- A new pinning test in `crates/ailang-check/tests/` (path + finalised in planning) asserts `CheckError::DuplicateCtor` + fires for a one-module fixture with two `Def::Type`s + declaring same-named ctors — ratifies the consumer the + ctt.1 ratification rests on. +- `Registry.type_def_module` key is `(String, String)`, value + is `String`. Every consumer site provides the calling module + at the call site. A regression test exercises two-module + bare-name collision and confirms both instances resolve + correctly. +- `WorkspaceLoadError::KindMismatch`, `walk_kind_mismatch`, its + dispatch site, and its `main.rs` Display arm are deleted. The + existing test + `class_param_in_applied_position_fires_canonical_form_rejection` + still passes, asserting `BareCrossModuleTypeRef`. +- `cargo test --workspace`, `bench/cross_lang.py`, + `bench/compile_check.py`, `bench/check.py` all green at + milestone close. +- `docs/roadmap.md` has the three relevant todo lines struck + through with reference to this spec or the per-iter journals. +- `docs/WhatsNew.md` carries one entry covering the milestone + in user-as-reader prose (no internals, no iter codes).