Add content-based monotype interning#9273
Draft
lukewilliamboswell wants to merge 8 commits intomainfrom
Draft
Conversation
…tical types The monotype store was append-only with no deduplication, causing structurally identical monotypes to get different Idx values from repeated type var resolution, cross-module remapping, and independent unification paths. This led to downstream inconsistencies in caches keyed by monotype index, papered over by seven separate structural equality bandaids. Add a ContentContext-based intern map to Monotype.Store.addMonotype that hashes and compares by dereferenced span content. Remove ~200 lines of structural equality workarounds across Lower.zig, MirToLir.zig, and mir_monotype_resolver.zig. Fix remapMonotypeBetweenModulesRec to intern remapped monotypes instead of only writing to placeholder slots. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ypeId API Replace the old getMonotype() tagged-union API with a resolve()-based TypeId approach across the compiler pipeline. Monotypes are now interned by structural content (internFunc, internRecord, internTagUnion, etc.) and accessed through typed accessor methods (funcArgs, funcRet, recordFields, tupleElems, etc.) instead of pattern-matching on a tagged union. Key changes: - MonotypeStore uses content-based interning for deduplication - Tag/field names use StructuralNameId from a shared name pool instead of per-module Ident.Idx, eliminating cross-module ident remapping - Callers use resolve() + kind-based switching instead of getMonotype() - @intFromEnum(mono_idx) replaced with @bitcast for new TypeId representation - Cross-module remapMonotypeBetweenModules greatly simplified since structural names are already canonical - Tests updated to use new intern* construction APIs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the var-based nominal_cycle_breakers with a canonical NominalInstanceKey cache that deduplicates by (origin, type_name, args). This avoids identity bugs from multiple type vars referring to the same nominal instantiation, and skips unnecessary .rec indirection for non-recursive nominals. Fix ident resolution to fall back to scratches.ident_store when module_env is unavailable (layout tests). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s nominal dedup The separate cycle-breaking map for recursive nominal types is no longer needed — the nominal instance cache introduced in the prior commit already prevents infinite recursion by caching based on content identity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The `remapMonotypeBetweenModules` function was investigated for removal but proven necessary: it deeply resolves `.rec` placeholder indirections in TypeId children that arise from recursive nominal types. Without this, cross-module TypeId comparisons fail when one module holds `List(rec(0))` while another holds the equivalent `List(tag_union(1))`. Added doc comments explaining the actual purpose (`.rec` resolution, not name/index remapping), regression test comments, and two new cross-module mono tests covering recursive nominal types through import chains. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…bug, dead param - Cache resolve() results in locals where called twice on the same index (MirToLir.zig: 3 sites, Lower.zig: 1 site) - Replace fragile identText() + null-guard + manual eql pattern with identMatchesText() in runtimeTagLayoutFromExpr, eliminating a latent null path that would silently skip all tags - Remove unused allocator parameter from TypeInterner.deinit() - Expand Store alias doc comment Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
internIdxList, internFieldList, and internTagList were doing O(n) linear scans over all meta entries. Replace with AutoHashMapUnmanaged(u64, ListId) keyed by fingerprint, with overflow chaining via a `next` field on the meta structs to handle fingerprint collisions. This brings list interning to O(1) amortized, matching the existing per-kind type intern maps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e tags The function only searched the current module's ident store, so cross-module tag names would silently fall through to Ident.Idx.NONE. Now searches all module envs, matching what identMatchesText in MirToLir already does. Also deduplicate the redundant findByString calls into a shared findIdentInEnv helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
From what I can tell this CI failure is unrelated... I'm not sure what has caused it recently. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the append-only monotype store with content-based interning, ensuring structurally identical types get the same
TypeId. This eliminates ~200 lines of structural equality workarounds and fixes downstream cache inconsistencies caused by duplicate monotype indices.Key changes
internFunc,internRecord,internTagUnion, etc. deduplicate by structural content using per-kindAutoHashMapUnmanagedintern mapsgetMonotype()tagged-union pattern matching withresolve()+ typed accessors (funcArgs,recordFields,tupleElems, etc.)StructuralNameIdfrom a shared pool instead of per-moduleIdent.Idx, eliminating cross-module ident remapping(origin, type_name, args), replacing the oldnominal_cycle_breakersmapinternIdxList/internFieldList/internTagListwith fingerprint-keyed hash maps with overflow chainingremapMonotypeBetweenModulessimplified to just.recplaceholder resolution (documented with regression tests)identMatchesText()instead of fragile null-guard patternFiles changed
src/mir/Monotype.zigsrc/mir/Lower.zigsrc/lir/MirToLir.zigsrc/layout/mir_monotype_resolver.zigsrc/mir/MIR.zigsrc/mir/LambdaSet.zigsrc/cli/main.zigsrc/eval/dev_evaluator.zigsrc/layout/store_test.zigsrc/mir/test/lower_test.zigsrc/check/test/cross_module_mono_test.zigtest/fx/cross_module_recursive_nominal.rocTest plan
zig build test-eval --summary all— 1227/1227 passed