Merge pull request #7637 from roc-lang/gpa-and-interner

Fix up SmallStringInterner and use GPA
roc-lang · Feb 25, 2025 · 578e925 · 578e925
2 parents e6410d8 + 2a5b1c5
commit 578e925
Show file tree

Hide file tree

Showing 12 changed files with 190 additions and 187 deletions.
diff --git a/src/base/Ident.zig b/src/base/Ident.zig
@@ -57,15 +57,19 @@ pub const Store = struct {
     exposing_modules: std.ArrayList(Module.Idx),
     next_unique_name: u32,
 
-    pub fn init(arena: *std.heap.ArenaAllocator) Store {
+    pub fn init(gpa: std.mem.Allocator) Store {
         return Store{
-            .interner = SmallStringInterner.init(arena),
-            // error: expected type 'heap.arena_allocator.ArenaAllocator', found '*heap.arena_allocator.ArenaAllocator'
-            .exposing_modules = std.ArrayList(Module.Idx).init(arena.allocator()),
+            .interner = SmallStringInterner.init(gpa),
+            .exposing_modules = std.ArrayList(Module.Idx).init(gpa),
             .next_unique_name = 0,
         };
     }
 
+    pub fn deinit(self: *Store) void {
+        self.interner.deinit();
+        self.exposing_modules.deinit();
+    }
+
     pub fn insert(self: *Store, ident: Ident, region: Region) Idx {
         const idx = self.interner.insert(ident.raw_text, region);
         self.exposing_modules.append(@enumFromInt(0)) catch exitOnOom();
@@ -141,10 +145,4 @@ pub const Store = struct {
     pub fn setExposingModule(self: *Store, idx: Idx, exposing_module: Module.Idx) void {
         self.exposing_modules.items[@as(usize, idx.idx)] = exposing_module;
     }
-
-    /// Look up text in this store, returning a slice of all string interner IDs
-    /// that match this ident's text.
-    pub fn lookup(self: *Store, string: []u8) []SmallStringInterner.Idx {
-        return self.interner.lookup(string);
-    }
 };
diff --git a/src/base/Module.zig b/src/base/Module.zig
@@ -32,28 +32,28 @@ pub const Idx = List.Idx;
 pub const Store = struct {
     modules: List,
     ident_store: *Ident.Store,
-    arena: *std.heap.ArenaAllocator,
+    gpa: std.mem.Allocator,
 
     pub const LookupResult = struct {
         module_idx: Idx,
         was_present: bool,
     };
 
-    pub fn init(arena: *std.heap.ArenaAllocator, ident_store: *Ident.Store) Store {
-        var modules = collections.SafeMultiList(Module).init(arena.allocator());
+    pub fn init(gpa: std.mem.Allocator, ident_store: *Ident.Store) Store {
+        var modules = collections.SafeMultiList(Module).init(gpa);
         _ = modules.append(Module{
             .name = &.{},
             .package_shorthand = null,
             .is_builtin = false,
-            .exposed_idents = collections.SafeList(Ident.Idx).init(arena.allocator()),
+            .exposed_idents = collections.SafeList(Ident.Idx).init(gpa),
         });
 
         // TODO: insert builtins automatically?
 
         return Store{
             .modules = modules,
             .ident_store = ident_store,
-            .arena = arena,
+            .gpa = gpa,
         };
     }
 
@@ -109,7 +109,7 @@ pub const Store = struct {
                 .name = name,
                 .package_shorthand = package_shorthand,
                 .is_builtin = false,
-                .exposed_idents = collections.SafeList(Ident.Idx).init(self.arena.allocator()),
+                .exposed_idents = collections.SafeList(Ident.Idx).init(self.gpa),
             });
 
             return LookupResult{ .module_idx = idx, .was_present = false };

diff --git a/src/base/ModuleEnv.zig b/src/base/ModuleEnv.zig
@@ -23,22 +23,29 @@ modules: Module.Store,
 strings: StringLiteral.Store,
 problems: std.ArrayList(Problem),
 type_store: Type.Store,
-arena: *std.heap.ArenaAllocator,
 
-pub fn init(arena: *std.heap.ArenaAllocator) Self {
-    var ident_store = Ident.Store.init(arena);
+pub fn init(gpa: std.mem.Allocator) Self {
+    var ident_store = Ident.Store.init(gpa);
 
     return Self{
         .idents = ident_store,
-        .ident_ids_for_slicing = collections.SafeList(Ident.Idx).init(arena.allocator()),
-        .modules = Module.Store.init(arena, &ident_store),
-        .strings = StringLiteral.Store.init(arena.allocator()),
-        .problems = std.ArrayList(Problem).init(arena.allocator()),
-        .type_store = Type.Store.init(arena.allocator()),
-        .arena = arena,
+        .ident_ids_for_slicing = collections.SafeList(Ident.Idx).init(gpa),
+        .modules = Module.Store.init(gpa, &ident_store),
+        .strings = StringLiteral.Store.init(gpa),
+        .problems = std.ArrayList(Problem).init(gpa),
+        .type_store = Type.Store.init(gpa),
     };
 }
 
+pub fn deinit(self: *Self) void {
+    self.modules.deinit();
+    self.idents.deinit();
+    self.ident_ids_for_slicing.deinit();
+    self.strings.deinit();
+    self.problems.deinit();
+    self.type_store.deinit();
+}
+
 pub fn addExposedIdentForModule(self: *Self, ident: Ident.Idx, module: Module.Idx) void {
     self.modules.addExposedIdent(module, ident, &self.problems);
     self.idents.setExposingModule(ident, module);

diff --git a/src/base/StringLiteral.zig b/src/base/StringLiteral.zig
@@ -32,9 +32,9 @@ pub const Store = struct {
     /// continues to the previous byte
     buffer: std.ArrayList(u8),
 
-    pub fn init(allocator: std.mem.Allocator) Store {
+    pub fn init(gpa: std.mem.Allocator) Store {
         return Store{
-            .buffer = std.ArrayList(u8).init(allocator),
+            .buffer = std.ArrayList(u8).init(gpa),
         };
     }
 

diff --git a/src/build/solve_functions.zig b/src/build/solve_functions.zig
@@ -48,10 +48,9 @@ test "solves for uncaptured functions" {
     // It will then run `solveFunctions` on that IR and assert that the
     // resulting FunctionSet.List correctly labels each function
 
-    var arena = std.heap.ArenaAllocator.init(testing.allocator);
-    defer arena.deinit();
-
-    _ = base.Ident.Store.init(&arena);
+    const gpa = testing.allocator;
+    var store = base.Ident.Store.init(gpa);
+    defer store.deinit();
 
     try testing.expect(true);
 }
diff --git a/src/check/canonicalize/IR.zig b/src/check/canonicalize/IR.zig
@@ -39,24 +39,40 @@ ingested_files: IngestedFile.List,
 ///
 /// Since the can IR holds indices into the `ModuleEnv`, we need
 /// the `ModuleEnv` to also be owned by the can IR to cache it.
-pub fn init(arena: *std.heap.ArenaAllocator) Self {
+pub fn init(gpa: std.mem.Allocator) Self {
     return Self{
-        .env = base.ModuleEnv.init(arena),
-        .aliases = Alias.List.init(arena.allocator()),
-        .defs = Def.List.init(arena.allocator()),
-        .exprs = Expr.List.init(arena.allocator()),
-        .exprs_at_regions = ExprAtRegion.List.init(arena.allocator()),
-        .typed_exprs_at_regions = TypedExprAtRegion.List.init(arena.allocator()),
-        .when_branches = WhenBranch.List.init(arena.allocator()),
-        .patterns = Pattern.List.init(arena.allocator()),
-        .patterns_at_regions = PatternAtRegion.List.init(arena.allocator()),
-        .typed_patterns_at_regions = TypedPatternAtRegion.List.init(arena.allocator()),
-        .type_vars = collections.SafeList(TypeVar).init(arena.allocator()),
-        // .type_var_names = Ident.Store.init(arena.allocator()),
-        .ingested_files = IngestedFile.List.init(arena.allocator()),
+        .env = base.ModuleEnv.init(gpa),
+        .aliases = Alias.List.init(gpa),
+        .defs = Def.List.init(gpa),
+        .exprs = Expr.List.init(gpa),
+        .exprs_at_regions = ExprAtRegion.List.init(gpa),
+        .typed_exprs_at_regions = TypedExprAtRegion.List.init(gpa),
+        .when_branches = WhenBranch.List.init(gpa),
+        .patterns = Pattern.List.init(gpa),
+        .patterns_at_regions = PatternAtRegion.List.init(gpa),
+        .typed_patterns_at_regions = TypedPatternAtRegion.List.init(gpa),
+        .type_vars = collections.SafeList(TypeVar).init(gpa),
+        // .type_var_names = Ident.Store.init(gpa),
+        .ingested_files = IngestedFile.List.init(gpa),
     };
 }
 
+pub fn deinit(self: *Self) void {
+    self.env.deinit();
+    self.aliases.deinit();
+    self.defs.deinit();
+    self.exprs.deinit();
+    self.exprs_at_regions.deinit();
+    self.typed_exprs_at_regions.deinit();
+    self.when_branches.deinit();
+    self.patterns.deinit();
+    self.patterns_at_regions.deinit();
+    self.typed_patterns_at_regions.deinit();
+    self.type_vars.deinit();
+    // self.type_var_names.deinit();
+    self.ingested_files.deinit();
+}
+
 pub const RigidVariables = struct {
     named: std.AutoHashMap(TypeVar, Ident.Idx),
     // with_methods: std.AutoHashMap(TypeVar, WithMethods),

diff --git a/src/check/canonicalize/Scope.zig b/src/check/canonicalize/Scope.zig
@@ -20,20 +20,20 @@ focused_custom_alias: ?Alias.Idx,
 custom_tags: std.AutoHashMap(Ident.Idx, Alias.Idx),
 /// Identifiers/aliases that are in scope, and defined in the current module.
 levels: Levels,
-allocator: std.mem.Allocator,
+gpa: std.mem.Allocator,
 
 pub fn init(
     env: *base.ModuleEnv,
     builtin_aliases: []const struct { alias: Alias.Idx, name: Ident.Idx },
     builtin_idents: []const Ident.Idx,
-    allocator: std.mem.Allocator,
+    gpa: std.mem.Allocator,
 ) Self {
     var scope = Self{
         .env = env,
         .focused_custom_alias = null,
-        .custom_tags = std.AutoHashMap(Ident.Idx, Alias.Idx).init(allocator),
-        .levels = Levels.init(env, allocator),
-        .allocator = allocator,
+        .custom_tags = std.AutoHashMap(Ident.Idx, Alias.Idx).init(gpa),
+        .levels = Levels.init(env, gpa),
+        .gpa = gpa,
     };
 
     scope.levels.enter();
@@ -127,10 +127,10 @@ pub const Level = struct {
         alias: Alias.Idx,
     };
 
-    pub fn init(allocator: std.mem.Allocator) Level {
+    pub fn init(gpa: std.mem.Allocator) Level {
         return Level{
-            .idents = std.ArrayList(IdentInScope).init(allocator),
-            .aliases = std.ArrayList(AliasInScope).init(allocator),
+            .idents = std.ArrayList(IdentInScope).init(gpa),
+            .aliases = std.ArrayList(AliasInScope).init(gpa),
         };
     }
 
@@ -143,13 +143,13 @@ pub const Level = struct {
 pub const Levels = struct {
     env: *base.ModuleEnv,
     levels: std.ArrayList(Level),
-    allocator: std.mem.Allocator,
+    gpa: std.mem.Allocator,
 
-    pub fn init(env: *base.ModuleEnv, allocator: std.mem.Allocator) Levels {
+    pub fn init(env: *base.ModuleEnv, gpa: std.mem.Allocator) Levels {
         return Levels{
             .env = env,
-            .levels = std.ArrayList(Level).init(allocator),
-            .allocator = allocator,
+            .levels = std.ArrayList(Level).init(gpa),
+            .gpa = gpa,
         };
     }
 
@@ -158,7 +158,7 @@ pub const Levels = struct {
     }
 
     pub fn enter(self: *Levels) void {
-        self.levels.append(Level.init(self.allocator)) catch exitOnOom();
+        self.levels.append(Level.init(self.gpa)) catch exitOnOom();
     }
 
     pub fn exit(self: *Levels) void {
@@ -340,16 +340,18 @@ pub const Levels = struct {
 };
 
 fn createTestScope(idents: [][]Level.IdentInScope, aliases: [][]Level.AliasInScope) Self {
-    const allocator = std.testing.allocator;
-    var env = base.ModuleEnv.init(allocator);
+    const gpa = std.testing.allocator;
+    var env = base.ModuleEnv.init(gpa);
+    defer env.deinit();
 
     var scope = Self{
         .env = &env,
         .focused_custom_alias = null,
-        .custom_tags = std.AutoHashMap(Ident.Idx, Alias.Idx).init(allocator),
-        .levels = Levels.init(&env, allocator),
-        .allocator = allocator,
+        .custom_tags = std.AutoHashMap(Ident.Idx, Alias.Idx).init(gpa),
+        .levels = Levels.init(&env, gpa),
+        .gpa = gpa,
     };
+    scope.deinit();
 
     const max_level = @min(idents.len, aliases.len);
     for (0..max_level) |_| {

diff --git a/src/check/parse.zig b/src/check/parse.zig
@@ -101,10 +101,10 @@ test "example s-expr" {
         \\
         \\foo = "bar"
     ;
-    var arena = std.heap.ArenaAllocator.init(testing.allocator);
-    defer arena.deinit();
-    var env = base.ModuleEnv.init(&arena);
-    var parse_ir = parse(&env, testing.allocator, source);
+    const gpa = testing.allocator;
+    var env = base.ModuleEnv.init(gpa);
+    defer env.deinit();
+    var parse_ir = parse(&env, gpa, source);
     defer parse_ir.deinit();
 
     // std.debug.print("{}", .{parse_ir});

diff --git a/src/check/parse/tokenize.zig b/src/check/parse/tokenize.zig
@@ -1251,15 +1251,13 @@ pub const Tokenizer = struct {
     }
 };
 
-fn testTokenization(allocator: std.mem.Allocator, input: []const u8, expected: []const Token.Tag) !void {
+fn testTokenization(gpa: std.mem.Allocator, input: []const u8, expected: []const Token.Tag) !void {
     var messages: [10]Diagnostic = undefined;
 
-    var arena = std.heap.ArenaAllocator.init(allocator);
-    defer arena.deinit();
+    var env = base.ModuleEnv.init(gpa);
+    defer env.deinit();
 
-    var env = base.ModuleEnv.init(&arena);
-
-    var tokenizer = Tokenizer.init(&env, input, &messages, allocator);
+    var tokenizer = Tokenizer.init(&env, input, &messages, gpa);
     defer tokenizer.deinit();
 
     tokenizer.tokenize();
@@ -1269,17 +1267,16 @@ fn testTokenization(allocator: std.mem.Allocator, input: []const u8, expected: [
     try std.testing.expectEqual(tokens[tokens.len - 1], Token.Tag.EndOfFile);
     try std.testing.expectEqualSlices(Token.Tag, expected[0..expected.len], tokens[0 .. tokens.len - 1]);
 
-    checkTokenizerInvariants(allocator, input, true);
+    checkTokenizerInvariants(gpa, input, true);
 }
 
-pub fn checkTokenizerInvariants(allocator: std.mem.Allocator, input: []const u8, debug: bool) void {
-    var arena = std.heap.ArenaAllocator.init(allocator);
-    defer arena.deinit();
-    var env = base.ModuleEnv.init(&arena);
+pub fn checkTokenizerInvariants(gpa: std.mem.Allocator, input: []const u8, debug: bool) void {
+    var env = base.ModuleEnv.init(gpa);
+    defer env.deinit();
 
     // Initial tokenization.
     var messages: [32]Diagnostic = undefined;
-    var tokenizer = Tokenizer.init(&env, input, &messages, allocator);
+    var tokenizer = Tokenizer.init(&env, input, &messages, gpa);
     tokenizer.tokenize();
     var output = tokenizer.finish_and_deinit();
     defer output.tokens.deinit();
@@ -1303,7 +1300,7 @@ pub fn checkTokenizerInvariants(allocator: std.mem.Allocator, input: []const u8,
         return;
     }
 
-    const buf2 = rebuildBufferForTesting(input, &output.tokens, allocator) catch |err| switch (err) {
+    const buf2 = rebuildBufferForTesting(input, &output.tokens, gpa) catch |err| switch (err) {
         error.Unsupported => return,
         error.OutOfMemory => std.debug.panic("OOM", .{}),
     };
@@ -1314,7 +1311,7 @@ pub fn checkTokenizerInvariants(allocator: std.mem.Allocator, input: []const u8,
     }
 
     // Second tokenization.
-    tokenizer = Tokenizer.init(&env, buf2.items, &messages, allocator);
+    tokenizer = Tokenizer.init(&env, buf2.items, &messages, gpa);
     tokenizer.tokenize();
     var output2 = tokenizer.finish_and_deinit();
     defer output2.tokens.deinit();