Refactor module system - #2489
Merged
Merged
Conversation
…lver (#2485) Introduces ModuleTable / ModuleScope / DeclEntry and the two resolvers that every module-aware lookup will go through, so that module membership becomes structural data instead of a naming convention encoded into identifiers. Nothing calls into it yet — this stage only lands the structure. The population pass, the typechecker call sites, and the codegen call sites move over in the stages that follow, after which rewrite_labels and the four codegen registries are deleted. Notes on the shape it landed in: - Entries are allocated individually from the compiler arena rather than out of a growing array, so a DeclEntry* cached on an AST node stays valid as the owning module keeps growing. The whole table dies with the arena, so there is no destroy path. - scope.c's djb2 hash is now shared rather than reimplemented; both maps here are name -> index over a dense array, the same shape Scope already uses. - resolve_qualified reports why a lookup failed (no module / no member / private), because the private-access diagnostics need that distinction once the visibility checks consolidate into it. - resolve_unqualified treats a name found in two using'd modules as ambiguous rather than silently taking the first, matching what the flat arrays do today when a using clones a colliding name. - module_mangle is the one place a module name and a declaration name become a C identifier.
…2485) register_declarations now inserts every top-level declaration into its module's scope under the name as written in source, alongside the existing flat arrays. Nothing reads the table yet, so behavior is unchanged. Module attribution comes from the file a declaration was written in, not from its name. The import driver is the only thing that knows which module owns a given .gray file, so main.c collects (file, module) pairs while resolving imports and hands them to the type checker before the check runs. Attributing by file is what makes a directory import land every sibling file's declarations in one scope, and it works for aliases, which the import merge never renamed and which therefore have no prefix to read a module out of. Transitional: the merge still renames imported declarations, so the source name is taken from the *_DISPLAY_NAME macros while the node still carries the mangled one. The two coincide once the merge stops renaming. Module-level constants get a registration pass of their own. They have no flat-array registry today — they are ordinary scope symbols — but `mod.NAME` has to resolve to them, so they are module members like anything else. DeclEntry.gray_type is left NULL for now; it gets filled when the resolver is first consulted for types. Verified against the multi-file suite's fixtures that attribution is exact and that module_mangle reproduces the names the compiler already emits: directory imports merge their files into one scope, aliased imports key on the alias (which is what the merge prefixes with today), private functions and constants carry VIS_PRIVATE, and stdlib imports stay out of the table. tests/test_module_table.c covers the structure and both resolvers directly: 26 cases over definition, hash growth past several rebuilds, file attribution, visibility, aliases, using search order and ambiguity, mangling, and the underscore case that suffix-guessing gets wrong.
…ble (#2485) First stage that puts the resolver on live code paths. The lookups converted here are the ones whose equivalence is provable: each one asked "is this module-qualified, and what does it name" by rebuilding a mangled string, and now asks the symbol table instead. Aliases. The type checker's alias_names/alias_modules arrays are gone; typechecker_resolve_alias delegates to the table, which register_decl_imports now populates. Fifteen call sites keep working unchanged because they all go through that one function. Module variables and constants. find_module_var_decl scanned every top-level statement in the merged program comparing against a rebuilt <mod>_<name> string. It is now one hash lookup keyed by (module, name). It deliberately ignores visibility: both callers want the declaration whether or not they may touch it, and decide separately what to say about a private one. Module functions. find_module_func replaces the resolve-alias/snprintf/ find_func sequence at five call sites: the two E3040 multi-return checks, user-module call dispatch, `using` dispatch, and multi-return temp typing. Sites that mangle something other than a module member — struct-namespaced calls (Type_func), imported struct functions (mod_Struct_func) — are left alone, since those are not module members and the table does not hold them. Deleted a verbatim duplicate: the E3040 check on a call argument inside resolve_call_expr was a second copy of reject_multi_return_in_single_position, down to the diagnostic arguments. It calls the helper now. Also fixes a latent divergence introduced by the previous stage. The entry module was keyed by the entry file's basename, so importing a module whose directory or filename matched — main.gray importing sub/main.gray — merged the two into one scope, where the string mangling had kept them apart. The entry module is now keyed under MODULE_ENTRY_NAME, which no import can produce, and a qualifier can never name it. module_mangle_into writes to a caller buffer so the lookup keys built on every resolution do not each allocate from the arena. Verified behavior-preserving by differential compile against the previous commit over every .gray file under integration-tests: 1611 files, byte- identical generated C and byte-identical diagnostics in all of them. The sole reported difference was the GRAY_VERSION string embedded by core/runtime_introspection.gray, which differs because the baseline was built from a clean worktree. test_module_table gains two cases for the entry-module keying: an imported module may share the entry file's basename, and the entry module is not a valid qualifier. 28 pass.
#2485) Completes the type checker's half. Thirteen more sites that rebuilt a <mod>_<name> string now call module_member_key, which asks the symbol table where the declaration lives and spells the key from that. What is left in typechecker.c is five Struct_func manglings, which are struct-function prefixing rather than module membership and stay as they are. module_member_key generalizes the previous stage's find_module_func, which is now two lines on top of it. It only decides how a key is spelled — the flat registries stay the authority on what a key names until they are replaced. That is what makes each of these a like-for-like substitution rather than a change of meaning. Converted: - qualified enum and struct access: mod.Enum.VARIANT in both call and member-expression position, and the mod.Type.member triple chain - the four `using` searches: bare type names, function marking, module variables, and when-case enum detection - module constant access through mod.NAME - the sites where a qualifier may be either a module or a struct type — func references (bare, through ref(), and inside a [func] array literal), the initializer call-name path, and the #discard check. These land on the same key either way: the table answers for a module, and the fallback reproduces the old string exactly for a struct. Two incidental fixes in code being touched anyway. The initializer call-name buffer was `static`, which it has no reason to be. And the [func] array literal branch xmalloc'd its entry while the branch beside it stored an AST-owned pointer into the same array, so nothing could ever free it — both are arena strings now. The casing heuristics that guard these paths (`mod[0] is lowercase && member[0] is uppercase`) are deliberately still in place. Removing them changes which programs resolve, so they come out with the bug fixes and their regression tests, not here. Verified behavior-preserving by differential compile against the previous commit over every .gray file under integration-tests: 1745 files, byte- identical generated C and diagnostics, with the sole difference again being the GRAY_VERSION string in core/runtime_introspection.gray.
…le (#2485) Codegen kept its own answer to "does this name belong to a module, and to what", rebuilt from its own AST walk. It now shares the type checker's, handed over next to the type table it already receives. Deleted outright: - The alias registry. codegen built alias_names/alias_modules from the same import items the type checker did, then insertion-sorted them so resolve_alias could bsearch. All of it is gone; resolve_alias is one call into the shared table. - resolve_unprefixed_name's suffix guess. It matched a bare name against the text after the last underscore of every declared struct and enum, so any local type whose name contained an underscore could be claimed as a module member. It now reads the owning module from the symbol table. - Two thirds of the is_module chain in member-expression emission. The comment there records that a prefix scan already caused this exact bug once (`item.priority` emitted as `item_priority` because `do item_is_alive` was in scope) and was patched by adding registries to consult. The table answers the question directly and supplies the mangled name at the same time, so the find_function probe and the alias scan are gone. The using and import lists remain as the fallback for stdlib modules, which are not in the table. Verified behavior-preserving by differential compile against the previous commit over every .gray file under integration-tests: 1745 files, byte- identical generated C and diagnostics, sole difference the GRAY_VERSION string in core/runtime_introspection.gray. Also built and ran the directory-import and aliased-directory-import programs end to end; both pass.
Six multi-file cases, one per bug the symbol-table refactor is meant to resolve. They fail today, each with the symptom its issue documents, and the refactor's flip is what makes them pass. Committing them failing first makes that step's effect legible. - alias-cross-module (#2481): a type alias reached as mod.Alias. Today E3001, because the import merge never renames NODE_ALIAS_DECL and the alias is treated as an opaque struct type. - cross-module-struct-func-inference (#2475): `mut x = mod.Struct.func()`. Today infers unknown, which truncates 2.5 to 2 and prints true as 1 with no diagnostic. Asserted through interpolation so the corrupted value is what fails, not just type_of. - func-ref-in-module (#2474): `()name` written in a non-entry file. Today emits the unmangled symbol and leaks a C compiler error, plus a false W1003 on a function that is called. - strict-when-qualified-enum (#2473): #strict over mod.Enum.VARIANT patterns. Today E3056 in both spellings — a tagged enum reports its payload-free variant missing, a plain enum is not seen as an enum. - sibling-name-collision (#2472): a parameter, local, or struct-literal value whose name matches a sibling file of a directory module. Today rewritten to the directory module name, leaking a C compiler error. - struct-default-sibling (#2470): a struct field default reading a sibling file's constant or enum variant. Today E4001, though the field's type resolves fine. These are pass tests, so the runner only checks the exit status. Value correctness is asserted with assert() rather than printed PASS/FAIL lines, which would report a wrong answer and still exit 0 — the failure mode #2475 is about.
…#2485) The struct, enum, and function registries were keyed by the name the import merge had rewritten onto the AST node. They are now keyed by module_mangle() of the symbol-table entry for that declaration — the module name comes from where the declaration actually lives rather than from a string the merge wrote. This is the mangling-as-a-pure-function-of-a-resolved-declaration part of the issue, and it is also the measurement that matters before the merge can be removed: if the resolver's key ever disagreed with the merge's rename, this is the commit where it would show. It does not. Differential compile against the previous commit over every .gray file under integration-tests: 1760 files, byte-identical generated C and diagnostics, sole difference the GRAY_VERSION string in core/runtime_introspection.gray. The six regression tests still fail exactly as before; nothing here was meant to move them.
…le (#2485) The counterpart to the previous commit, one phase later. Function, struct, and enum declarations were emitted under the name the import merge had written onto the node; they are now emitted under module_mangle() of the symbol-table entry that declared them. Recovering the entry needs neither a name nor a file: the table keeps a reverse index from declaration node to entry, so a phase holding the node has the module. That is what lets codegen name a declaration correctly without tracking which file it is emitting from, which it has never done. Generic instantiations are left alone. mangle_generic_name manages its own per-binding suffix and temporarily puts it on the node, so while a wildcard binding is active the name in hand wins. Without that guard the module mangling overwrote the type-argument mangling and every instantiation collapsed onto one symbol — caught by the differential across the thirteen wildcard and generics tests, which is what the differential is for. Byte-identical because the merge still renames and the two spellings agree. That is the point: with the type checker (previous commit) and codegen (this one) both naming declarations from the table, removing the rename can only change what the resolver and the rename disagree about. Differential compile against the previous commit over every .gray file under integration-tests: 1760 files, byte-identical generated C and diagnostics, sole difference the GRAY_VERSION string in core/runtime_introspection.gray. test_module_table covers the node index and its rehash: 30 pass.
…2485) The flip. Names are no longer rewritten when imported files are merged; a reference stays as the programmer wrote it and is resolved against the symbol table, which is what both phases have been naming declarations from since the two preceding commits. Parser. read_type_name, the module-qualified struct literal, and the when-pattern enum name keep the qualifier attached — lib.Score, not lib_Score. Flattening it there produced a name with no way back to the module it came from. main.c. rewrite_labels and rewrite_type_name are deleted, along with the declaration-name collection and the compound alias_Name -> mod_Name mapping that fed them. rewrite_labels handled 27 of 45 node kinds and silently dropped the rest, which is where several of the bugs below came from. Import discovery, path resolution, dedup and E6001 collision detection are untouched. Sibling imports inside a directory module — where types.gray names a file, not a module — are now recorded as an alias of the directory module instead of generating a mapping per declaration. Resolution. The type checker resolves a written name through the module the current file belongs to, then the modules it uses. It is applied inside find_struct, find_func, find_enum_index and is_enum_name, so every existing call site keeps working with the name as written. Registration order no longer matters: declarations enter the symbol table in a pass of their own before any type is resolved, and `using` is collected there too rather than during the statement walk, because a struct field's default is checked while declarations are still being registered. Codegen. It tracks which module's file it is emitting and resolves names the same way — labels, calls, func references, struct literals, new(), for_each collections, and type names. Two more suffix guesses are gone: the first-underscore match over declared functions, and the alias registry keyed by unqualified name. Field defaults are emitted in their own struct's module, not the caller's. types.c. The dot-splitting, the _Uppercase guess and the hardcoded denylist of stdlib opaque type names are deleted. What remains is one rule: anything still carrying a dot is stdlib, which keeps its own registries. Fixes #2481, #2475, #2474, #2473, #2472, #2470 — all six now pass as regression tests. Suite goes from 508 pass / 7 fail to 513 / 2, and all 1052 fail-tests still reject with the expected code, including the harness check that no diagnostic leaks a mangled name. Two known failures are documented in the handoff rather than papered over: struct-array-cross-file relies on a bare imported name resolving without `using`, which STANDARD.md 8.2 says requires dot notation and which only worked because the deleted rewrite pass rewrote the entry file too; and W2011_named_return_unused, which already failed before this branch.
) E4015 and E4021 were decided at four call sites, each reading a different source: FuncSig.is_private for a function, a parallel is_private array plus a file comparison for a type alias, and a scan of the program AST for a variable or constant. They are now one rule in module_decl_visible(), applied inside module_resolve_qualified() and reported by one helper — the kind of the declaration only picks which code to emit. Resolving that meant fixing what the rule compares. RESOLVE_PRIVATE was testing the module, but the error text — and the tests — say a private declaration is private to its *file*: two files merged into one directory module are as much "outside" each other as two separate modules. The declaration already records its origin file, so the check reads that. The resolvers now take a ResolveScope rather than a growing list of context parameters. It carries the module an unqualified name resolves against, the file that decides visibility, and the using list. module_resolve_qualified lost its module argument entirely: a qualifier names its own module, so the only thing that parameter ever did was the visibility test. Deleted with their last reader: TypeChecker.type_alias_files and type_alias_is_private, which were write-only once the declaration became the source of truth, and module_var_is_private. RESOLVE_PRIVATE is consumed now. It was added three commits before anything used it, which was the wrong order. Suite unchanged at 513 pass / 2 fail and 1052 fail-tests rejecting with the expected code. test_module_table is 40 cases, including one pinning that a directory module's files do not see each other's private declarations.
…2485) The last remnant of the is_module chain in member-expression emission was two loops asking the same question of two lists. A user module is answered by the symbol table above it, so what reaches here is the stdlib, which keeps its own registries by design. Named for what it actually asks. Suite unchanged: 513 pass / 2 fail, 1052 fail-tests rejecting as expected.
Written once, always NULL, read nowhere. The resolver returns the declaration and callers take the type from the registry that holds it, so the field never acquired a reader — the same speculative-field mistake as RESOLVE_PRIVATE, which at least got one in the end. Suite unchanged: 513 pass / 2 fail, 1052 fail-tests rejecting as expected. test_module_table 40 pass.
The struct, function and enum registries each kept a sorted copy of their names so a lookup could bsearch it, rebuilt whenever anything was registered. Three indexes, three comparators, three "sorted_built" flags — all keyed by the mangled string the refactor set out to stop treating as identity. They are gone. A declaration now records where its details live (DeclEntry.registry_index), so resolving a name yields the entry and the entry yields the details directly. find_struct, find_func and find_enum_index are three lines each and consult nothing but the symbol table. Two things had to join the table for that to hold. Struct functions, which are namespaced under their struct rather than their module, and the compiler-provided and stdlib types registered without a source declaration — both get an entry now, so no lookup falls outside the table. Where a lookup holds an already-mangled name rather than one as written, the table answers from a mangled-name index it owns, replacing the three it displaced. Two bugs this surfaced, both fixed here. A name declared twice in one module resolves to the first declaration, and the duplicate was repointing that entry at its own registry slot. And a struct/enum collision is visible in the symbol table before either registry holds both halves, so E4007 now asks the table rather than asking two registries that are filled in different passes. Suite unchanged: 513 pass / 2 fail, 1052 fail-tests rejecting with the expected code, test_module_table 40 pass.
AstNode gains resolved_decl: the declaration a reference names, left there by the type checker when it resolves it. Codegen reads it instead of resolving the same name a second time, which is the last place the two phases were doing the same work independently. The field is a forward-declared pointer, so ast.h stays free of the symbol table — the parser has no business knowing about it, and nodes are memset on allocation, so anything unresolved is NULL by construction rather than by convention. Wired at the references where both phases need the answer: bare labels naming a module-level declaration, struct literals, and func references. Where the type checker left nothing, codegen resolves as before. Suite unchanged: 513 pass / 2 fail, 1052 fail-tests rejecting with the expected code, test_module_table 40 pass.
…2485) A parameter or return type written bare resolves against the module the function was declared in, but the forward-declaration loop never entered that module — it emitted with whatever module context the previous pass had left behind. The declaration and the definition then disagreed whenever two modules declared the same type name: static GrayString gray_fn_colors_describe(GrayStruct_shapes_Point); static GrayString gray_fn_colors_describe(GrayStruct_colors_Point p) { ... } Invisible with distinct type names, which is why the suite did not catch it. Suite unchanged: 513 pass / 2 fail, 1052 fail-tests rejecting as expected.
resolve_unprefixed_name scanned every module and took the first that declared the name. With one module declaring it that is the right answer by accident; with two it is a coin flip. A `Color.CRIMSON` written inside the module that declares Color was emitted as another module's GrayEnum_traffic_Color_CRIMSON, which does not exist there. The scope is asked first now, and the scan is left as the fallback for names belonging to no module in scope — stdlib opaque types, which is what it was kept for. Suite unchanged: 513 pass / 2 fail, 1052 fail-tests rejecting as expected.
A stdlib module's functions and opaque types now enter the module table when the module is imported, so `mod.name` resolves the same way whatever kind of module it names. Gated on the import: an unimported stdlib module has to stay unresolvable or the unknown-module and unused-import diagnostics lose their basis. The entries are marked external. They carry identity and visibility only — the C name of a stdlib call comes from its emitter, which knows about type suffixes and argument marshalling that no mangling rule could reproduce. That let two guesses go. resolve_unprefixed_name scanned every module and took the first that declared the name, kept only because stdlib types were not in the table; it is now what the scope says, and the function is gone. type_from_name's dot rule no longer means "assume stdlib". Two more places were emitting without knowing which module they were in, both invisible until two modules declared the same type name: struct bodies (a bare field type resolved against whatever module ran last) and the qualified-type branch of the C type mapping. new() now resolves its type name the way an annotation does, so `new(mod.T)` and `^mod.T` agree on what they name. Suite unchanged: 513 pass / 2 fail, 1052 fail-tests rejecting with the expected code, test_module_table 40 pass.
…he table Stdlib constants join their module alongside its functions and opaque types, so `math.PI` resolves the way every other module member does. That replaces a chain of per-module string compares in resolve_member, one branch per module, which decided a constant's type by asking which module the qualifier named. The math branch returned float for *any* member, so a misspelled constant quietly became a float. Now an unknown member resolves to nothing, which the paths below it are free to report. The constant table describes its own struct-typed entry rather than having the reader hardcode that NIL_UUID means UUID, and both the qualified and the `using` path read it through one mapper instead of two switch statements that disagreed — the `using` one returned unknown where the qualified one returned UUID. The concrete stdlib struct types (HttpRequest, HttpResponse, UUID, Database, Router) are declared by their own module now rather than being swept into the entry file's scope by their registration. 514 pass / 1 fail, up from 513 / 2: multi-file/struct-array-cross-file was failing on a bare name from an `import and use`, and now resolves. 1052 fail-tests reject with the expected code. test_module_table 40 pass.
func_display_name() fell back to FuncSig.name, the flattened registry key, unless the declaration carried original_name. Merging an import no longer renames anything, so original_name is never set and every diagnostic that named a module function printed the mangled key: "function 'lib_add_two' expects 2 argument(s)". The declaration node's own .name is the name as written, which is what FUNC_DISPLAY_NAME already reads for the deprecation warnings. This also clears a false unused-import warning. A bare call through `using` marks the module used by looking its member up under the module prefix; that key was built from the display name, so it searched for lib_lib_greet, found nothing, and left the import marked unused. Every `import and use` reported W1002 against a module it was using. The runner's existing guard rejects a leaked mod_Type on its shape alone, but a mangled function name is indistinguishable from a plain snake_case name, so tests can now pin one by name with an expect-not: marker.
W1003 passed the entry file as the diagnostic's location while def_line
numbers the file the function was declared in. An unused function in an
imported module was reported against whatever the entry file happened to
hold at that line, usually blank:
warning[W1003]: function 'find_heaviest' is declared but never called
--> main.gray:6:1
|
6 |
W1001 passed the entry file as the diagnostic's location while def_line counts lines in the file holding the function body. An unused local in an imported module was reported against whatever the entry file held at that line — unrelated source, or nothing when the entry file was shorter. A local cannot outlive the function it is declared in, so the function declaration node names the file without a def_file on every Symbol. W1003 was the other half of this and is already fixed. The test asserts the location with an expect-not: marker, which the pass runner now honours the way run_fail_test already did. It is the only mechanism here that can pin a warning's text, since a pass test otherwise only has to exit zero.
…gh using
An import was marked used at the sites that handle qualified access, so a
module reached only by the bare name of one of its members was reported as
never used. `import and use "./types.gray"` followed by `mut i Item =
Item{...}` warned W1002 against a module the program cannot compile
without, and adding one qualified reference silenced it.
Marking it where a written name resolves to a declaration covers every
kind of member at once, rather than adding a case per reference site. A
name resolving inside its own module is not a use of an import.
Accessing a member a module does not have left the expression's type unknown and said nothing. The checks downstream skip unknown to avoid cascading off a type that already failed, so an annotated declaration accepted the typo silently and an inferred one surfaced later as an undefined variable, naming the variable rather than the member that was the actual mistake. E4023 covers the same mistake for a call and says "no function named", which is wrong for a constant or a type, so this is a separate code. The symbol table holds every member of an imported module, the stdlib included, so it can answer whether one exists. The check runs only after every other reading of the member has failed, which keeps it from preempting a path that resolves.
A declaration with an unresolved type is deliberately left out of the scope, so that the error already reported upstream is not followed by a second one about the type. A failed call was excepted from that rule; member access was not, so `mut y = p.nope` never declared y and every later read of it added an undefined-variable error on top of the one real mistake: error[E3010]: struct 'P' has no field 'nope' error[E4001]: undefined variable 'y' error[E4001]: undefined variable 'y' The member is where the mistake was named, so the same exception applies: skipping the declaration here turns one error into one per read, which is what the rule exists to prevent.
github-actions
Bot
added
documentation
github-actions
Bot
added
tests
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| char prefixed[MSG_BUF_SIZE]; | ||
| snprintf(prefixed, sizeof(prefixed), "%s_%s", real_mod, function_name); | ||
| FuncSig *sig = find_func(checker, prefixed); | ||
| FuncSig *sig = find_module_func(checker, real_mod, function_name); |
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
2 participants
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.
No description provided.