refactor: address interning review feedback

2026-10-25 00:30:39 +02:00
parent 675fa5049f
commit 678bb2583b
4 changed files with 209 additions and 35 deletions
@@ -67,6 +67,8 @@ name resolution (`@source/sema`) runs after parsing. it predeclares package name

 identifier interning is compilation-owned: initialize a `strpool.StringPool`, pass it to `lexer.scan` and `sema.resolve` before the diagnostic store, and release it after lowering. scanning interns identifier spellings once per pool; parsing preserves their IDs in compact AST storage, exposed through `ast.name_id`, while retaining source spans for diagnostics. name resolution keys scopes by those IDs and interns builtin names into the same pool, so identifier hashing and equality do not scan spelling bytes. IDs are meaningful only within their originating pool; independent compilations may assign the same numeric ID to different names. interning failures propagate as infrastructure errors rather than producing an invalid identifier ID. IR retains no pool dependency.

+resolver scopes use `@std/hashmap.AutoHashMap(K, V)`, which shares `init`, `put`, `get` and `deinit` with custom `HashMap` and `StringHashMap`. automatic keys support integers, booleans, enums, distinct types, fixed arrays and records recursively; record padding is ignored. slices, floating-point values, pointers, optionals and unions require explicit hashing and equality through `HashMap` (`StringHashMap` supplies content-based byte-string operations). keys and values are stored by value; referenced resources remain caller-owned. the builtin declaration count is inferred, and builtin ID storage derives its fixed stack-array length from that declaration array rather than repeating a count or initializer.
+
 `sema.resolve` returns an owned binding table indexed by AST `NodeId`, without modifying the AST. `sema.binding` identifies a declaration, builtin type, or invalid binding carrying its diagnostic ID; nodes with no binding return `.none`. release the result with `sema.deinit`. unknown names and duplicate declarations add source-spanned diagnostics; invalid syntax retains its existing diagnostic identity. same-scope duplicates retain the first declaration for recovery, while forbidden inner declarations invalidate that name's uses rather than silently binding them to an outer declaration.

 type checking (`@source/sema/check`) runs after name resolution. `check.check` borrows the AST and binding table and returns owned per-node types and interned array descriptors; query with `check.type_of` and `check.array_type`, and release with `check.deinit`. declarations infer their initializer types, including forward references and function aliases; function types identify the declaration supplying the callable signature. annotations support builtin types and fixed arrays; type names are not ordinary values. `void` is valid as a function result, not as a parameter or stored value.
@@ -20,7 +20,7 @@ Builtin :: struct {
 }

@hide:file
-builtins [8]Builtin :: [
+builtins [_]Builtin :: [
    { name = "int", type = .int },
    { name = "float", type = .float },
    { name = "bool", type = .bool },
@@ -31,6 +31,17 @@ builtins [8]Builtin :: [
    { name = "usize", type = .usize },
 ]

+@hide:file
+intern_builtins proc(
+    $N usize,
+    pool @mut strpool.StringPool,
+    entries @[N]Builtin,
+) [N]mut strpool.StringId ! (mem.AllocError | strpool.InternError) {
+    ids [N]mut strpool.StringId := undefined
+    for (entries) |builtin, index| ids[index] = try strpool.intern(pool, builtin.name)
+    return ids
+}
+
 BindingKind :: enum { none, declaration, builtin, invalid }

 BindingData :: union {
@@ -56,7 +67,7 @@ Resolution :: struct { storage Storage }

@hide:file
 Scope :: struct {
-    names hashmap.HashMap(strpool.StringId, Binding, name_hash, names_equal)
+    names hashmap.AutoHashMap(strpool.StringId, Binding)
    parent ?@Scope
 }

@@ -65,7 +76,7 @@ State :: struct {
    tree @ast.Tree
    diagnostics @mut diagnostics.Store
    bindings []mut Binding
-    builtin_ids [8]mut strpool.StringId = [0, 0, 0, 0, 0, 0, 0, 0]
+    builtin_ids []strpool.StringId
    allocator mem.Allocator
 }

@@ -81,19 +92,18 @@ resolve proc(
    bindings :: try mem.alloc(Binding, allocator, ast.node_count(tree))
    errdefer mem.free(allocator, bindings)
    mem.set(bindings, Binding{})
+    builtin_ids :: try intern_builtins(pool, &builtins)

    state := State{
        tree = tree,
        diagnostics = diagnostic_store,
        bindings = bindings,
+        builtin_ids = builtin_ids[..],
        allocator = allocator,
    }

-    for (builtins) |builtin, index| {
-        state.builtin_ids[index] = try strpool.intern(pool, builtin.name)
-    }
    package_scope := Scope{
-        names = hashmap.init(strpool.StringId, Binding, name_hash, names_equal, allocator),
+        names = hashmap.init(allocator),
        parent = null,
    }
    defer hashmap.deinit(&package_scope.names)
@@ -196,7 +206,7 @@ resolve_function proc(
    }

    scope := Scope{
-        names = hashmap.init(strpool.StringId, Binding, name_hash, names_equal, state.allocator),
+        names = hashmap.init(state.allocator),
        parent = package_scope,
    }
    defer hashmap.deinit(&scope.names)
@@ -210,7 +220,7 @@ resolve_function proc(
@hide:file
 resolve_arm proc(state @mut State, parent @Scope, id ast.NodeId) void ! (mem.AllocError | diagnostics.CapacityError) {
    scope := Scope{
-        names = hashmap.init(strpool.StringId, Binding, name_hash, names_equal, state.allocator),
+        names = hashmap.init(state.allocator),
        parent = parent,
    }
    defer hashmap.deinit(&scope.names)
@@ -255,7 +265,7 @@ resolve_block proc(state @mut State, scope @mut Scope, id ast.NodeId) void ! (me
                loop :: ast.for_stmt(tree, statement)
                try resolve_expr(state, scope, loop.iterable)
                body_scope := Scope{
-                    names = hashmap.init(strpool.StringId, Binding, name_hash, names_equal, state.allocator),
+                    names = hashmap.init(state.allocator),
                    parent = scope,
                }
                defer hashmap.deinit(&body_scope.names)
@@ -352,12 +362,3 @@ resolve_expr proc(
    }
 }

-@hide:file
-name_hash proc(name strpool.StringId) usize {
-    return usize(name)
-}
-
-@hide:file
-names_equal proc(left, right strpool.StringId) bool {
-    return left == right
-}
@@ -1,4 +1,5 @@
 import "@std/mem"
+import "@std/meta"

 PutError :: enum { key_exists }

@@ -13,10 +14,11 @@ Entry proc($K, $V type) type {

 #! owns entry storage and retains its allocator; release storage with deinit.
 #! keys and values are stored by value; any resources they reference remain caller-owned.
+#! null callbacks select automatic operations; equal keys must always produce equal hashes.
 HashMap proc(
    $K, $V type,
-    $hash_key proc(key K) usize,
-    $keys_eql proc(a, b K) bool,
+    $hash_key ?proc(key K) usize,
+    $keys_eql ?proc(a, b K) bool,
 ) type {
    return struct {
        entries []mut Entry(K, V)
@@ -29,11 +31,18 @@ StringHashMap proc($V type) type {
    return HashMap([]u8, V, str_hash, str_eql)
 }

+#! automatic value hashing for integers, booleans, enums, distinct types, arrays and records.
+#! slices and floating-point keys require explicit hash/equality operations through HashMap.
+#! record padding is ignored; pointer, union and optional keys are not supported.
+AutoHashMap proc($K, $V type) type {
+    return HashMap(K, V, null, null)
+}
+
 #! retain `allocator` for later growth and cleanup; this call does not allocate.
 init proc(
    $K, $V type,
-    $hash_key proc(key K) usize,
-    $keys_eql proc(a, b K) bool,
+    $hash_key ?proc(key K) usize,
+    $keys_eql ?proc(a, b K) bool,
    allocator mem.Allocator,
 ) HashMap(K, V, hash_key, keys_eql) {
    return HashMap(K, V, hash_key, keys_eql){
@@ -47,29 +56,38 @@ init proc(
 #! note: this operation invalidates the map.
 deinit proc(
    $K, $V type,
-    $hash_key proc(key K) usize,
-    $keys_eql proc(a, b K) bool,
+    $hash_key ?proc(key K) usize,
+    $keys_eql ?proc(a, b K) bool,
    map @HashMap(K, V, hash_key, keys_eql),
 ) void { mem.free(map.allocator, map.entries) }

 get proc(
    $K, $V type,
-    $hash_key proc(key K) usize,
-    $keys_eql proc(a, b K) bool,
+    $hash_key ?proc(key K) usize,
+    $keys_eql ?proc(a, b K) bool,
    map @HashMap(K, V, hash_key, keys_eql),
    key K,
 ) ?V {
    if (map.count == 0) return null

-    hash :: normalize(hash_key(key))
+    hash usize := 0
+    if $(hash_key == null) {
+        hash = normalize(auto_hash(K, key))
+    } else {
+        hash = normalize(call_hash(K, hash_key?, key))
+    }
    idx := hash & (map.entries.len - 1)

    while true {
        entry :: map.entries[idx]
        if (entry.hash == 0) return null

-        if (entry.hash == hash and keys_eql(entry.key, key)) {
-            return entry.value
+        if (entry.hash == hash) {
+            if $(keys_eql == null) {
+                if (auto_eql(K, entry.key, key)) return entry.value
+            } else {
+                if (call_eql(K, keys_eql?, entry.key, key)) return entry.value
+            }
        }

        idx = (idx + 1) & (map.entries.len - 1)
@@ -78,8 +96,8 @@ get proc(

 put proc(
    $K, $V type,
-    $hash_key proc(key K) usize,
-    $keys_eql proc(a, b K) bool,
+    $hash_key ?proc(key K) usize,
+    $keys_eql ?proc(a, b K) bool,
    map @mut HashMap(K, V, hash_key, keys_eql),
    key K,
    value V,
@@ -107,7 +125,12 @@ put proc(
        mem.free(map.allocator, old_entries)
    }

-    hash :: normalize(hash_key(key))
+    hash usize := 0
+    if $(hash_key == null) {
+        hash = normalize(auto_hash(K, key))
+    } else {
+        hash = normalize(call_hash(K, hash_key?, key))
+    }
    idx := hash & (map.entries.len - 1)

    while true {
@@ -122,14 +145,29 @@ put proc(
            return
        }

-        if entry.hash == hash and keys_eql(entry.key, key) {
-            return .key_exists
+        if (entry.hash == hash) {
+            if $(keys_eql == null) {
+                if (auto_eql(K, entry.key, key)) return .key_exists
+            } else {
+                if (call_eql(K, keys_eql?, entry.key, key)) return .key_exists
+            }
        }

        idx = (idx + 1) & (map.entries.len - 1)
    }
 }

+# concrete comptime callbacks use named calls so their specializations are registered.
+@hide:file
+call_hash proc($K type, $callback proc(key K) usize, key K) usize {
+    return callback(key)
+}
+
+@hide:file
+call_eql proc($K type, $callback proc(a, b K) bool, a, b K) bool {
+    return callback(a, b)
+}
+
@hide:file
 normalize proc(hash usize) usize {
    # mapping both 0 and 1 to 1 is safe because equality resolves
@@ -155,3 +193,61 @@ str_hash proc(key []u8) usize {

@hide:file
 str_eql proc(a, b []u8) bool { return mem.eql(a, b) }
+
+@hide:file
+auto_hash proc($K type, key K) usize {
+    match typeinfo!(K) {
+        .integer: {
+            bytes @[sizeof!(K)]u8 :: ptrcast!(&key)
+            return str_hash(bytes[..])
+        }
+        .bool: return if (key) 1 else 0
+        .enum |info|: {
+            inline for (info.fields) |field| {
+                if (key == field!(K, field.name)) return field.index
+            }
+            unreachable
+        }
+        .distinct |backing|: return auto_hash(ptrcast!(@backing, &key)^)
+        .array: {
+            hash usize := 0
+            for (key) |element| hash = combine_hash(hash, auto_hash(element))
+            return hash
+        }
+        .record |info|: {
+            hash usize := 0
+            inline for (info.fields) |field| {
+                hash = combine_hash(hash, auto_hash(field!(key, field.name)))
+            }
+            return hash
+        }
+        else: compile_error!("AutoHashMap key requires integers, booleans, enums, distinct types, arrays or records; use HashMap for custom hashing")
+    }
+}
+
+@hide:file
+combine_hash proc(left, right usize) usize {
+    # rotate before mixing so ordered fields do not cancel like a plain xor.
+    return ((left << 5) xor (left >> (sizeof!(usize) * 8 - 5))) xor right
+}
+
+@hide:file
+auto_eql proc($K type, left, right K) bool {
+    match typeinfo!(K) {
+        .integer, .bool, .enum: return left == right
+        .distinct |backing|: return auto_eql(ptrcast!(@backing, &left)^, ptrcast!(@backing, &right)^)
+        .array: {
+            for (left) |element, index| {
+                if (!auto_eql(element, right[index])) return false
+            }
+            return true
+        }
+        .record |info|: {
+            inline for (info.fields) |field| {
+                if (!auto_eql(field!(left, field.name), field!(right, field.name))) return false
+            }
+            return true
+        }
+        else: compile_error!("AutoHashMap key requires integers, booleans, enums, distinct types, arrays or records; use HashMap for custom equality")
+    }
+}
@@ -11,3 +11,78 @@ handles_put_and_get test {
    try testing.expect_type(?u32, value)
    try testing.expect_equal(42, value?)
 }
+
+@hide:file
+collision_hash proc(_ u32) usize { return 0 }
+
+@hide:file
+collision_eql proc(left, right u32) bool {
+    return rem!(left, 100) == rem!(right, 100)
+}
+
+custom_callbacks_preserve_collisions_and_equivalent_keys test {
+    map HashMap(u32, u32, collision_hash, collision_eql) := init(mem.c_allocator)
+    defer deinit(&map)
+    for (u32(1)..u32(32)) |key| try put(&map, key, key * 2)
+    for (u32(1)..u32(32)) |key| {
+        try testing.expect_equal(key * 2, get(&map, key + 100)?)
+    }
+    try testing.expect_equal(null, get(&map, 99))
+    duplicate := false
+    put(&map, 101, 999) catch |error| { duplicate = error == .key_exists }
+    try testing.expect(duplicate)
+    try testing.expect_equal(u32(31), u32(map.count))
+    try testing.expect_equal(u32(2), get(&map, 1)?)
+}
+
+auto_integer_keys_preserve_boundaries_and_rehashing test {
+    map AutoHashMap(i64, usize) := init(mem.c_allocator)
+    defer deinit(&map)
+    keys [_]i64 :: [minval!(i64), -1, 0, 1, maxval!(i64)]
+    for (keys) |key, index| try put(&map, key, index)
+    for (i64(2)..i64(100)) |key| try put(&map, key, usize(key))
+    for (keys) |key, index| try testing.expect_equal(index, get(&map, key)?)
+    for (i64(2)..i64(100)) |key| try testing.expect_equal(usize(key), get(&map, key)?)
+    try testing.expect_equal(null, get(&map, -2))
+    duplicate := false
+    put(&map, -1, 999) catch |error| { duplicate = error == .key_exists }
+    try testing.expect(duplicate)
+    try testing.expect_equal(usize(1), get(&map, -1)?)
+    wide AutoHashMap(u64, bool) := init(mem.c_allocator)
+    defer deinit(&wide)
+    try put(&wide, maxval!(u64), true)
+    try testing.expect_equal(true, get(&wide, maxval!(u64))?)
+}
+
+@hide:file
+TestKeyKind :: enum(u8) { first = 3, second = 91 }
+
+@hide:file
+TestKeyId :: distinct u32
+
+@hide:file
+TestKey :: struct {
+    enabled bool
+    id TestKeyId
+    kind TestKeyKind
+    indices [2]u16
+}
+
+auto_record_keys_use_field_values_and_array_order test {
+    map AutoHashMap(TestKey, i32) := init(mem.c_allocator)
+    defer deinit(&map)
+    first :: TestKey{ enabled = true, id = TestKeyId(17), kind = .first, indices = [2, 9] }
+    reversed :: TestKey{ enabled = true, id = TestKeyId(17), kind = .first, indices = [9, 2] }
+    second :: TestKey{ enabled = false, id = TestKeyId(18), kind = .second, indices = [2, 9] }
+    try put(&map, first, 42)
+    try put(&map, reversed, 51)
+    try put(&map, second, 63)
+    try testing.expect_equal(i32(42), get(&map, TestKey{
+        enabled = true, id = TestKeyId(17), kind = .first, indices = [2, 9],
+    })?)
+    try testing.expect_equal(i32(51), get(&map, reversed)?)
+    try testing.expect_equal(i32(63), get(&map, second)?)
+    try testing.expect_equal(null, get(&map, TestKey{
+        enabled = true, id = TestKeyId(18), kind = .first, indices = [2, 9],
+    }))
+}