Eliminate need for forward type declarations in recursive datatypes#443
Eliminate need for forward type declarations in recursive datatypes#443joehendrix wants to merge 1 commit intomainfrom
Conversation
4971e76 to
64accb0
Compare
64accb0 to
ab52636
Compare
Mutually recursive datatypes previously required users to manually write `forward type` declarations before a `mutual` block. This eliminates that boilerplate by having the elaborator automatically discover and pre-register type names before elaborating the block's children. - Add @[preRegisterTypes(scope)] annotation triggering two-phase elaboration: Phase 1 extracts type names/params, Phase 2 elaborates with mutual references resolved - Simplify GlobalContext by removing DeclState enum and forward declaration bookkeeping; symbols are now present or absent - Add precomputed argElabIndex array to SyntaxElaborator for O(1) argument lookup by argLevel - Adapt Translate.lean to allocate placeholder entries on the fly instead of requiring forward declarations Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ab52636 to
ad51d1d
Compare
MikaelMayer
left a comment
There was a problem hiding this comment.
Clean, well-motivated PR that removes a real source of user friction. The two-phase elaboration design in elaborateWithPreRegistration is sound, the GlobalContext simplification is a clear win, and the removal of DeclState/forward declarations is thorough — no dangling references remain.
One minor inconsistency between extractParamNames and the inline code in runSyntaxElaborator (.tvar handling) is worth a look. The rest is solid.
I'd approve after the one question below is addressed.
| let newBindings := tpCtx.bindings.toArray.extract inheritedCount tpCtx.bindings.size | ||
| let names := newBindings.filterMap fun (b : Binding) => | ||
| match b.kind with | ||
| | .type _ [] _ => some b.ident |
There was a problem hiding this comment.
extractParamNames matches both .type _ [] _ and .tvar _ _, but the analogous inline code in runSyntaxElaborator (around line 1101) only matches .type _ [] _:
|>.filterMap fun b =>
match b.kind with
| .type _ [] _ => some b.ident
| _ => noneIs the .tvar case intentionally included here but not there? If both should behave the same, consider extracting the shared logic into extractParamNames and calling it from both sites.
Summary
Mutually recursive datatypes previously required users to manually write
forward typedeclarations before amutualblock. This PR eliminatesthat boilerplate by having the elaborator automatically discover and
pre-register type names before elaborating the block's children.
Details
@[preRegisterTypes(scope)]annotation oncommand_mutualtriggersa two-phase elaboration: Phase 1 partially elaborates each child to
extract type names and params, then pre-registers them in the
GlobalContextso Phase 2 can resolve mutual references. This removesa common source of user confusion — forgetting or misordering forward
declarations — and makes the
mutualblock self-contained.GlobalContextsimplified. TheDeclStateenum(
.forward/.defined) and its associated bookkeeping(
declareForward,isForward, three-elementvarstuples) areremoved. Symbols are now either present or absent, tracked by a single
ensureDefinedoperation. This eliminates a class of bugs whereforward-declared symbols could be left in a half-defined state.
argElabIndex. A precomputedArray (Option Nat)maps eachargLevelto its position inargElaborators, replacing linear searches throughargLevelToSyntaxLevel. Callers get the fullArgElaborator,not just one field.
Translate.leanno longer requiresforward declarations before mutual blocks. Instead it allocates
placeholder entries on the fly when a datatype isn't already
pre-registered, making the AST-to-CST path work for both DDM-parsed
and programmatically-constructed programs.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.