language-servers/roc
1
0
Fork 0
mirror of https://github.com/roc-lang/roc.git synced 2025-12-23 08:48:03 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

Add some tests for attribute-related edge cases

Browse source
This commit is contained in:
Richard Feldman 2025-07-26 22:05:10 -04:00
parent f56ddfc1f4
commit 7da16a3a72
No known key found for this signature in database
6 changed files with 107 additions and 64 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/base/Ident.zig
View file

@ -77,7 +77,7 @@ pub const Attributes = packed struct(u3) {
effectful: bool,
ignored: bool,
reassignable: bool,
pub fn fromString(text: []const u8) Attributes {
return .{
.effectful = std.mem.endsWith(u8, text, "!"),

16
src/check/canonicalize.zig
View file

@ -636,7 +636,7 @@ pub fn canonicalizeFile(
const idx = try self.env.idents.insert(self.env.gpa, ident);
// Store the def index as u16 in exposed_items
const def_idx_u16: u16 = @intCast(@intFromEnum(def_idx));
try self.env.exposed_items.setNodeIndexById(self.env.gpa, idx.idx, def_idx_u16);
try self.env.exposed_items.setNodeIndexById(self.env.gpa, @bitCast(idx), def_idx_u16);
}
_ = self.exposed_ident_texts.remove(ident_text);
@ -910,7 +910,7 @@ fn createExposedScope(
// Get the interned identifier
if (self.parse_ir.tokens.resolveIdentifier(ident.ident)) |ident_idx| {
// Add to exposed_items for permanent storage (unconditionally)
try self.env.exposed_items.addExposedById(gpa, ident_idx.idx);
try self.env.exposed_items.addExposedById(gpa, @bitCast(ident_idx));
// Use a dummy pattern index - we just need to track that it's exposed
const dummy_idx = @as(Pattern.Idx, @enumFromInt(0));
@ -943,7 +943,7 @@ fn createExposedScope(
// Get the interned identifier
if (self.parse_ir.tokens.resolveIdentifier(type_name.ident)) |ident_idx| {
// Add to exposed_items for permanent storage (unconditionally)
try self.env.exposed_items.addExposedById(gpa, ident_idx.idx);
try self.env.exposed_items.addExposedById(gpa, @bitCast(ident_idx));
// Use a dummy statement index - we just need to track that it's exposed
const dummy_idx = @as(Statement.Idx, @enumFromInt(0));
@ -976,7 +976,7 @@ fn createExposedScope(
// Get the interned identifier
if (self.parse_ir.tokens.resolveIdentifier(type_with_constructors.ident)) |ident_idx| {
// Add to exposed_items for permanent storage (unconditionally)
try self.env.exposed_items.addExposedById(gpa, ident_idx.idx);
try self.env.exposed_items.addExposedById(gpa, @bitCast(ident_idx));
// Use a dummy statement index - we just need to track that it's exposed
const dummy_idx = @as(Statement.Idx, @enumFromInt(0));
@ -1383,7 +1383,7 @@ fn introduceExposedItemsIntoScope(
// We need to look up by string because the identifiers are from different modules
// First, try to find this identifier in the target module's ident store
const is_exposed = if (module_env.idents.findByString(item_name_text)) |target_ident|
module_env.exposed_items.containsById(self.env.gpa, target_ident.idx)
module_env.exposed_items.containsById(self.env.gpa, @bitCast(target_ident))
else
false;
@ -1718,7 +1718,7 @@ pub fn canonicalizeExpr(
const target_node_idx = if (self.module_envs) |envs_map| blk: {
if (envs_map.get(module_text)) |module_env| {
if (module_env.idents.findByString(field_text)) |target_ident| {
break :blk module_env.exposed_items.getNodeIndexById(self.env.gpa, target_ident.idx) orelse 0;
break :blk module_env.exposed_items.getNodeIndexById(self.env.gpa, @bitCast(target_ident)) orelse 0;
} else {
break :blk 0;
}
@ -1782,7 +1782,7 @@ pub fn canonicalizeExpr(
const target_node_idx = if (self.module_envs) |envs_map| blk: {
if (envs_map.get(module_text)) |module_env| {
if (module_env.idents.findByString(field_text)) |target_ident| {
break :blk module_env.exposed_items.getNodeIndexById(self.env.gpa, target_ident.idx) orelse 0;
break :blk module_env.exposed_items.getNodeIndexById(self.env.gpa, @bitCast(target_ident)) orelse 0;
} else {
break :blk 0;
}
@ -6567,7 +6567,7 @@ fn tryModuleQualifiedLookup(self: *Self, field_access: AST.BinOp) std.mem.Alloca
const target_node_idx = if (self.module_envs) |envs_map| blk: {
if (envs_map.get(module_text)) |module_env| {
if (module_env.idents.findByString(field_text)) |target_ident| {
break :blk module_env.exposed_items.getNodeIndexById(self.env.gpa, target_ident.idx) orelse 0;
break :blk module_env.exposed_items.getNodeIndexById(self.env.gpa, @bitCast(target_ident)) orelse 0;
} else {
break :blk 0;
}

66
src/check/canonicalize/test/exposed_shadowing_test.zig
View file

@ -371,15 +371,15 @@ test "exposed_items is populated correctly" {
// Should have exactly 3 entries (duplicates not stored)
try testing.expectEqual(@as(usize, 3), env.exposed_items.count());
// Check that exposed_items contains all exposed items
const foo_idx = env.idents.findByString("foo").?;
const bar_idx = env.idents.findByString("bar").?;
const mytype_idx = env.idents.findByString("MyType").?;
try testing.expect(env.exposed_items.containsById(env.gpa, foo_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, bar_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, mytype_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(foo_idx)));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(bar_idx)));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(mytype_idx)));
}
test "exposed_items persists after canonicalization" {
@ -409,10 +409,10 @@ test "exposed_items persists after canonicalization" {
const x_idx = env.idents.findByString("x").?;
const y_idx = env.idents.findByString("y").?;
const z_idx = env.idents.findByString("z").?;
try testing.expect(env.exposed_items.containsById(env.gpa, x_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, y_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, z_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(x_idx)));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(y_idx)));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(z_idx)));
// Verify the map persists in env after canonicalization is complete
try testing.expectEqual(@as(usize, 3), env.exposed_items.count());
@ -447,11 +447,51 @@ test "exposed_items never has entries removed" {
const foo_idx = env.idents.findByString("foo").?;
const bar_idx = env.idents.findByString("bar").?;
const baz_idx = env.idents.findByString("baz").?;
try testing.expect(env.exposed_items.containsById(env.gpa, foo_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, bar_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, baz_idx.idx));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(foo_idx)));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(bar_idx)));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(baz_idx)));
// Should have exactly 3 unique entries
try testing.expectEqual(@as(usize, 3), env.exposed_items.count());
}
test "exposed_items handles identifiers with different attributes" {
const allocator = testing.allocator;
// Module exposing foo and foo! - these should be treated as different identifiers
// Note: Using foo and foo! to test that attributes are properly included in the key
const source =
\\module [foo, foo!]
\\
\\foo = 42
\\foo! = \x -> x + 1
;
var env = try ModuleEnv.init(allocator, source);
defer env.deinit();
try env.initCIRFields(allocator, "Test");
var ast = try parse.parse(&env);
defer ast.deinit(allocator);
var canonicalizer = try canonicalize.init(&env, &ast, null);
defer canonicalizer.deinit();
try canonicalizer.canonicalizeFile();
// Both should be in exposed_items as separate entries
const foo_idx = env.idents.findByString("foo").?;
const foo_effectful_idx = env.idents.findByString("foo!").?;
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(foo_idx)));
try testing.expect(env.exposed_items.containsById(env.gpa, @bitCast(foo_effectful_idx)));
// Should have exactly 2 entries - if we only used u29 without attributes, they might incorrectly merge
try testing.expectEqual(@as(usize, 2), env.exposed_items.count());
// Verify they have different full u32 values (index + attributes)
const foo_u32 = @as(u32, @bitCast(foo_idx));
const foo_effectful_u32 = @as(u32, @bitCast(foo_effectful_idx));
try testing.expect(foo_u32 != foo_effectful_u32);
}

28
src/check/canonicalize/test/import_validation_test.zig
View file

@ -61,13 +61,13 @@ test "import validation - mix of MODULE NOT FOUND, TYPE NOT EXPOSED, VALUE NOT E
// Add exposed items to Json module
const Ident = base.Ident;
const decode_idx = try json_env.idents.insert(allocator, Ident.for_text("decode"));
try json_env.exposed_items.addExposedById(allocator, decode_idx.idx);
try json_env.exposed_items.addExposedById(allocator, @bitCast(decode_idx));
const encode_idx = try json_env.idents.insert(allocator, Ident.for_text("encode"));
try json_env.exposed_items.addExposedById(allocator, encode_idx.idx);
try json_env.exposed_items.addExposedById(allocator, @bitCast(encode_idx));
const json_error_idx = try json_env.idents.insert(allocator, Ident.for_text("JsonError"));
try json_env.exposed_items.addExposedById(allocator, json_error_idx.idx);
try json_env.exposed_items.addExposedById(allocator, @bitCast(json_error_idx));
const decode_problem_idx = try json_env.idents.insert(allocator, Ident.for_text("DecodeProblem"));
try json_env.exposed_items.addExposedById(allocator, decode_problem_idx.idx);
try json_env.exposed_items.addExposedById(allocator, @bitCast(decode_problem_idx));
try module_envs.put("Json", json_env);
@ -81,11 +81,11 @@ test "import validation - mix of MODULE NOT FOUND, TYPE NOT EXPOSED, VALUE NOT E
// Add exposed items to Utils module
const map_idx = try utils_env.idents.insert(allocator, Ident.for_text("map"));
try utils_env.exposed_items.addExposedById(allocator, map_idx.idx);
try utils_env.exposed_items.addExposedById(allocator, @bitCast(map_idx));
const filter_idx = try utils_env.idents.insert(allocator, Ident.for_text("filter"));
try utils_env.exposed_items.addExposedById(allocator, filter_idx.idx);
try utils_env.exposed_items.addExposedById(allocator, @bitCast(filter_idx));
const result_idx = try utils_env.idents.insert(allocator, Ident.for_text("Result"));
try utils_env.exposed_items.addExposedById(allocator, result_idx.idx);
try utils_env.exposed_items.addExposedById(allocator, @bitCast(result_idx));
try module_envs.put("Utils", utils_env);
@ -530,17 +530,17 @@ test "exposed_items - tracking CIR node indices for exposed items" {
// Add exposed items and set their node indices
const Ident = base.Ident;
const add_idx = try math_env.idents.insert(allocator, Ident.for_text("add"));
try math_env.exposed_items.addExposedById(allocator, add_idx.idx);
try math_env.exposed_items.addExposedById(allocator, @bitCast(add_idx));
const multiply_idx = try math_env.idents.insert(allocator, Ident.for_text("multiply"));
try math_env.exposed_items.addExposedById(allocator, multiply_idx.idx);
try math_env.exposed_items.addExposedById(allocator, @bitCast(multiply_idx));
const pi_idx = try math_env.idents.insert(allocator, Ident.for_text("PI"));
try math_env.exposed_items.addExposedById(allocator, pi_idx.idx);
try math_env.exposed_items.addExposedById(allocator, @bitCast(pi_idx));
// Simulate having CIR node indices for these exposed items
// In real usage, these would be set during canonicalization of MathUtils
try math_env.exposed_items.setNodeIndexById(allocator, add_idx.idx, 100);
try math_env.exposed_items.setNodeIndexById(allocator, multiply_idx.idx, 200);
try math_env.exposed_items.setNodeIndexById(allocator, pi_idx.idx, 300);
try math_env.exposed_items.setNodeIndexById(allocator, @bitCast(add_idx), 100);
try math_env.exposed_items.setNodeIndexById(allocator, @bitCast(multiply_idx), 200);
try math_env.exposed_items.setNodeIndexById(allocator, @bitCast(pi_idx), 300);
try module_envs.put("MathUtils", math_env);
@ -606,7 +606,7 @@ test "exposed_items - tracking CIR node indices for exposed items" {
allocator.destroy(empty_env);
}
const undefined_idx = try empty_env.idents.insert(allocator, Ident.for_text("undefined"));
try empty_env.exposed_items.addExposedById(allocator, undefined_idx.idx);
try empty_env.exposed_items.addExposedById(allocator, @bitCast(undefined_idx));
// Don't set node index - should default to 0
try module_envs.put("EmptyModule", empty_env);

2
src/check/check_types.zig
View file

@ -865,7 +865,7 @@ pub fn checkExpr(self: *Self, expr_idx: ModuleEnv.Expr.Idx) std.mem.Allocator.Er
// Search through the module's exposed items
const node_idx_opt = if (module.idents.findByString(method_name_str)) |target_ident|
module.exposed_items.getNodeIndexById(self.gpa, target_ident.idx)
module.exposed_items.getNodeIndexById(self.gpa, @bitCast(target_ident))
else
null;

57
src/collections/ExposedItems.zig
View file

@ -14,8 +14,9 @@ const std = @import("std");
const Allocator = std.mem.Allocator;
const SortedArrayBuilder = @import("SortedArrayBuilder.zig").SortedArrayBuilder;
// We use u32 for the identifier index type to match Ident.Idx
// This avoids needing to import base module but maintains type compatibility
// We use u32 which is the bit representation of base.Ident.Idx
// This includes both the 29-bit index AND the 3-bit attributes (effectful, ignored, reassignable)
// This is critical because foo, foo!, and _foo are different identifiers
const IdentIdx = u32;
/// A collection that tracks exposed items by their names and associated CIR node indices
@ -35,7 +36,7 @@ pub const ExposedItems = struct {
self.items.deinit(allocator);
}
/// Add an exposed item by its interned ID
/// Add an exposed item by its interned ID (pass @bitCast(base.Ident.Idx) to u32)
pub fn addExposedById(self: *Self, allocator: Allocator, ident_idx: IdentIdx) !void {
// Add with value 0 to indicate "exposed but not yet defined"
// The SortedArrayBuilder will handle duplicates by keeping the last value,
@ -51,11 +52,11 @@ pub const ExposedItems = struct {
}
}
/// Set the node index for an exposed item by its interned ID
/// Set the node index for an exposed item by its interned ID (pass @bitCast(base.Ident.Idx) to u32)
pub fn setNodeIndexById(self: *Self, allocator: Allocator, ident_idx: IdentIdx, node_idx: u16) !void {
// First ensure the array is sorted so we can search
self.items.ensureSorted(allocator);
// Find the existing entry and update its value
const entries = self.items.entries.items;
for (entries) |*entry| {
@ -64,18 +65,18 @@ pub const ExposedItems = struct {
return;
}
}
// If not found, add a new entry
try self.items.put(allocator, ident_idx, node_idx);
}
/// Check if an item is exposed by its interned ID
/// Check if an item is exposed by its interned ID (pass @bitCast(base.Ident.Idx) to u32)
pub fn containsById(self: *const Self, allocator: Allocator, ident_idx: IdentIdx) bool {
var mutable_self = @constCast(self);
return mutable_self.items.get(allocator, ident_idx) != null;
}
/// Get the node index for an exposed item by its interned ID
/// Get the node index for an exposed item by its interned ID (pass @bitCast(base.Ident.Idx) to u32)
pub fn getNodeIndexById(self: *const Self, allocator: Allocator, ident_idx: IdentIdx) ?u16 {
var mutable_self = @constCast(self);
return mutable_self.items.get(allocator, ident_idx);
@ -109,79 +110,81 @@ pub const ExposedItems = struct {
// 1. The count of items (u32)
// 2. For each item: key (u32) + value (u16)
var size: usize = @sizeOf(u32); // count
for (self.items.entries.items) |_| {
size += @sizeOf(u32); // key (interned ID)
size += @sizeOf(u16); // value
}
// Align to SERIALIZATION_ALIGNMENT
const SERIALIZATION_ALIGNMENT = 16;
return std.mem.alignForward(usize, size, SERIALIZATION_ALIGNMENT);
}
/// Serialize this ExposedItems into the provided buffer
pub fn serializeInto(self: *const Self, buffer: []u8) ![]u8 {
const size = self.serializedSize();
if (buffer.len < size) return error.BufferTooSmall;
var offset: usize = 0;
// Write count
std.mem.writeInt(u32, buffer[offset..][0..4], @intCast(self.items.entries.items.len), .little);
offset += @sizeOf(u32);
// Write entries
for (self.items.entries.items) |entry| {
// Write key (interned ID)
std.mem.writeInt(u32, buffer[offset..][0..4], entry.key, .little);
offset += @sizeOf(u32);
// Write value
std.mem.writeInt(u16, buffer[offset..][0..2], entry.value, .little);
offset += @sizeOf(u16);
}
// Zero padding
if (offset < size) {
@memset(buffer[offset..size], 0);
}
return buffer[0..size];
}
/// Deserialize ExposedItems from the provided buffer
pub fn deserializeFrom(buffer: []const u8, allocator: Allocator) !Self {
if (buffer.len < @sizeOf(u32)) return error.BufferTooSmall;
var offset: usize = 0;
// Read count
const entry_count = std.mem.readInt(u32, buffer[offset..][0..4], .little);
offset += @sizeOf(u32);
var result = Self.init();
errdefer result.deinit(allocator);
// Read entries
for (0..entry_count) |_| {
// Read key (interned ID)
if (offset + @sizeOf(u32) > buffer.len) return error.BufferTooSmall;
const key = std.mem.readInt(u32, buffer[offset..][0..4], .little);
const key_u32 = std.mem.readInt(u32, buffer[offset..][0..4], .little);
// Use the full u32 (includes both index and attributes)
const key = key_u32;
offset += @sizeOf(u32);
// Read value
if (offset + @sizeOf(u16) > buffer.len) return error.BufferTooSmall;
const value = std.mem.readInt(u16, buffer[offset..][0..2], .little);
offset += @sizeOf(u16);
// Add to builder
try result.items.put(allocator, key, value);
}
return result;
}
/// Append to iovec writer for serialization
pub fn appendToIovecs(self: *const Self, writer: anytype) !usize {
return self.items.appendToIovecs(writer);