Symbol identity is a (module, name-string) key, so distinct declarations can share one SymbolId
Limitation
The canonical symbol table, exposed as pub symbols: HashMap<(Vec<String>, String), SymbolId> on ResolvedNames (a copy of SymbolTable::map), maps a
module path and a name string to an id. Anything that maps to the same key
gets the same SymbolId, and intern_all_symbols does not check for a clash:
-
One key space for every declaration kind. Functions, structs, enums, aspects,
let/mutglobals and type aliases are keyed by their bare name (decl_any_name). A struct and a function both calledFooin one module share an id, anddefinitions.entry(id).or_insert(span)keeps only the first declaration's span, so a second declaration is invisible to goto-definition. I found no runtime misbehaviour for this pair (a structFooandfun Foo, used locally and through an import, both behave correctly): the registries that consume the id happen to be per-kind. -
A method's key drops its impl's type arguments.
method_symbol_namebuildsTarget::Aspect::method(orTarget::method) from the target's base name.extend W<i64>: Tag { fun t … }andextend W<String>: Tag { fun t … }both produceW::Tag::t, so both methods receive oneSymbolIdand the id-keyed method registry keeps whichever was registered last. -
Methods are not overloaded, so a repeated name collides even inside one impl. Two methods called
min oneextend Sblock, whatever their parameter types, shareS::mand one id; the later definition replaces the earlier. Free functions are different: each overloaded definition gets its own id from the overload allocator (LIMIT-NAME-RESOLUTION-002), and only the bare name shares one table entry.
Impact
Cases 2 and 3 are silent wrong results, reproduced against current develop.
Case 2: with the two impls above, W { v = 1 }.t() and W { v = "s" }.t() both
return the last impl's value, and swapping the blocks swaps the answer (also
for inherent extend blocks and through a generic bound); coherence accepts the
program (metel-core#1228). Case 3: extend S { fun m(&self, x: i64) …; fun m(&self, x: String) … } is accepted and only the String method exists
(s.m(5) then fails to typecheck), and two m() methods with identical
signatures are accepted with no duplicate error, the second running (free
functions get T0011 for the same mistake; metel-core#1229). Case 1 has no
observed runtime effect today but is the same weakness: identity is a string
key, not the declaration.
Affects
arch.name-resolution.requirement-1arch.resolution.requirement-1metel-frontend/src/pipeline/name_resolution/name_resolver/mod.rs::intern_all_symbolsmetel-frontend/src/pipeline/name_resolution/name_resolver/mod.rs::decl_any_namemetel-frontend/src/pipeline/name_resolution/name_resolver/mod.rs::method_symbol_namemetel-frontend/src/identity/allocate.rs(method-symbol lookup)
metel-frontend/src/pipeline/name_resolution/name_resolver/mod.rs::decl_any_namemetel-frontend/src/pipeline/name_resolution/name_resolver/mod.rs::intern_all_symbolsmetel-frontend/src/pipeline/name_resolution/name_resolver/mod.rs::method_symbol_name
Resolution
None yet; the bugs are metel-core#1228 and metel-core#1229. The fix is to key a method by its
declaration (for example the impl's position or a canonical form of its target
type including arguments) and to make intern_all_symbols reject or
disambiguate two declarations that map to one key.