From 0faa026aa550b9cba875bb3146b7cd877eccf9e5 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 30 Oct 2025 23:07:29 -0400 Subject: [PATCH] Fix some canonicalization bugs --- src/canonicalize/Can.zig | 18 +++++++-------- src/check/test/TestEnv.zig | 22 ------------------- src/check/test/type_checking_integration.zig | 18 +++++++++++++++ ...ominal_associated_with_final_expression.md | 13 +++++++++++ 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/src/canonicalize/Can.zig b/src/canonicalize/Can.zig index efbca41b16..432565f86d 100644 --- a/src/canonicalize/Can.zig +++ b/src/canonicalize/Can.zig @@ -673,11 +673,10 @@ fn processAssociatedItemsSecondPass( ); try self.env.store.addScratchDef(def_idx); - // Set node index for exposed associated item - if (self.env.containsExposedById(qualified_idx)) { - const def_idx_u16: u16 = @intCast(@intFromEnum(def_idx)); - try self.env.setExposedNodeIndexById(qualified_idx, def_idx_u16); - } + // Register node index for associated item + // Associated items are always available when their parent type is in scope + const def_idx_u16: u16 = @intCast(@intFromEnum(def_idx)); + try self.env.setExposedNodeIndexById(qualified_idx, def_idx_u16); } } } @@ -722,11 +721,10 @@ fn processAssociatedItemsSecondPass( const def_idx = try self.canonicalizeAssociatedDecl(decl, qualified_idx); try self.env.store.addScratchDef(def_idx); - // Set node index for exposed associated item - if (self.env.containsExposedById(qualified_idx)) { - const def_idx_u16: u16 = @intCast(@intFromEnum(def_idx)); - try self.env.setExposedNodeIndexById(qualified_idx, def_idx_u16); - } + // Register node index for associated item + // Associated items are always available when their parent type is in scope + const def_idx_u16: u16 = @intCast(@intFromEnum(def_idx)); + try self.env.setExposedNodeIndexById(qualified_idx, def_idx_u16); } } else { // Non-identifier patterns are not supported in associated blocks diff --git a/src/check/test/TestEnv.zig b/src/check/test/TestEnv.zig index 8320e95319..c0bd5e4445 100644 --- a/src/check/test/TestEnv.zig +++ b/src/check/test/TestEnv.zig @@ -91,22 +91,6 @@ fn loadCompiledModule(gpa: std.mem.Allocator, bin_data: []const u8, module_name: }; } -/// Helper function to expose all top-level definitions in a module -/// This makes them available for cross-module imports -fn exposeAllDefs(module_env: *ModuleEnv) !void { - const defs_slice = module_env.store.sliceDefs(module_env.all_defs); - for (defs_slice) |def_idx| { - const def = module_env.store.getDef(def_idx); - - // Get the pattern to find the identifier - const pattern = module_env.store.getPattern(def.pattern); - if (pattern == .assign) { - const ident_idx = pattern.assign.ident; - const def_idx_u16: u16 = @intCast(@intFromEnum(def_idx)); - try module_env.setExposedNodeIndexById(ident_idx, def_idx_u16); - } - } -} gpa: std.mem.Allocator, module_env: *ModuleEnv, @@ -211,9 +195,6 @@ pub fn initWithImport(module_name: []const u8, source: []const u8, other_module_ try can.canonicalizeFile(); try can.validateForChecking(); - // Expose all top-level definitions so they can be imported by other modules - try exposeAllDefs(module_env); - // Get Bool and Result statement indices from the IMPORTED modules (not copied!) const bool_stmt_in_bool_module = builtin_indices.bool_type; const try_stmt_in_result_module = builtin_indices.try_type; @@ -330,9 +311,6 @@ pub fn init(module_name: []const u8, source: []const u8) !TestEnv { try can.canonicalizeFile(); try can.validateForChecking(); - // Expose all top-level definitions so they can be imported by other modules - try exposeAllDefs(module_env); - // Get Bool and Result statement indices from the IMPORTED modules (not copied!) const bool_stmt_in_bool_module = builtin_indices.bool_type; const try_stmt_in_result_module = builtin_indices.try_type; diff --git a/src/check/test/type_checking_integration.zig b/src/check/test/type_checking_integration.zig index 9928aead76..70402cb4a3 100644 --- a/src/check/test/type_checking_integration.zig +++ b/src/check/test/type_checking_integration.zig @@ -1322,3 +1322,21 @@ test "associated item can reference another associated item from same type" { ; try checkTypesModule(source, .{ .pass = .{ .def = "x" } }, "Test.MyBool"); } + +test "Bool.not works as builtin associated item" { + const source = + \\Test := [].{} + \\ + \\x = Bool.not(True) + ; + try checkTypesModule(source, .{ .pass = .{ .def = "x" } }, "Bool"); +} + +test "Str.is_empty works as low-level builtin associated item" { + const source = + \\Test := [].{} + \\ + \\x = Str.is_empty("") + ; + try checkTypesModule(source, .{ .pass = .{ .def = "x" } }, "Bool"); +} diff --git a/test/snapshots/nominal/nominal_associated_with_final_expression.md b/test/snapshots/nominal/nominal_associated_with_final_expression.md index 5ccf533177..b135d85307 100644 --- a/test/snapshots/nominal/nominal_associated_with_final_expression.md +++ b/test/snapshots/nominal/nominal_associated_with_final_expression.md @@ -11,6 +11,7 @@ x } # EXPECTED EXPRESSION IN ASSOCIATED ITEMS - nominal_associated_with_final_expression.md:2:1:2:2 UNUSED VARIABLE - nominal_associated_with_final_expression.md:1:20:1:25 +UNUSED VARIABLE - nominal_associated_with_final_expression.md:1:20:1:25 # PROBLEMS **EXPRESSION IN ASSOCIATED ITEMS** Associated items (such as types or methods) can only have associated types and values, not plain expressions. @@ -24,6 +25,18 @@ x } ^ +**UNUSED VARIABLE** +Variable `Foo.x` is not used anywhere in your code. + +If you don't need this variable, prefix it with an underscore like `_Foo.x` to suppress this warning. +The unused variable is declared here: +**nominal_associated_with_final_expression.md:1:20:1:25:** +```roc +Foo := [A, B, C].{ x = 5 +``` + ^^^^^ + + **UNUSED VARIABLE** Variable `x` is not used anywhere in your code.