From 962681423676308ab94122c69787a559beb220d3 Mon Sep 17 00:00:00 2001 From: Felipe Vogel Date: Sat, 7 Mar 2026 15:33:47 -0500 Subject: [PATCH 1/5] Error message for static dispatch missing parens --- src/check/Check.zig | 44 ++++++++++++++++++++ src/check/problem/context.zig | 3 ++ src/check/report.zig | 17 ++++++++ src/check/test/type_checking_integration.zig | 24 +++++++++++ 4 files changed, 88 insertions(+) diff --git a/src/check/Check.zig b/src/check/Check.zig index d30445bc161..fdd9b3224c5 100644 --- a/src/check/Check.zig +++ b/src/check/Check.zig @@ -4380,10 +4380,16 @@ fn checkExpr(self: *Self, expr_idx: CIR.Expr.Idx, env: *Env, expected: Expected) .record = .{ .fields = record_field_range, .ext = record_ext_var }, } }, env, expr_region); + // Check if the field name is actually a method on the receiver type. + // This helps provide a better error when the user writes `receiver.method` + // instead of `receiver.method()`. + const is_method = try self.isMethodOnNominalType(receiver_var, dot_access.field_name); + // Then, unify the actual receiver type with the expected record _ = try self.unifyInContext(record_being_accessed, receiver_var, env, .{ .record_access = .{ .field_name = dot_access.field_name, .field_region = dot_access.field_name_region, + .is_method = is_method, } }); _ = try self.unify(expr_var, record_field_var, env); } @@ -5933,6 +5939,44 @@ fn checkConstraints(self: *Self, env: *Env) std.mem.Allocator.Error!void { self.constraints.items.clearRetainingCapacity(); } +/// Returns true if `method_ident` is a known method on the nominal type of `receiver_var`. +/// Used to detect when a user writes `receiver.method` (missing parentheses) instead of +/// `receiver.method()`. +fn isMethodOnNominalType(self: *Self, receiver_var: Var, method_ident: Ident.Idx) std.mem.Allocator.Error!bool { + const nominal = switch (self.types.resolveVar(receiver_var).desc.content) { + .structure => |s| switch (s) { + .nominal_type => |n| n, + else => return false, + }, + else => return false, + }; + + const origin_module_ident = nominal.origin_module; + // Find the module env that owns the type's method definitions. + const original_env: *const ModuleEnv = blk: { + // Type is defined in the module we're currently compiling. + if (origin_module_ident == self.builtin_ctx.module_name) break :blk self.cir; + // Type is defined in the Builtin module (and we're not compiling it). + if (origin_module_ident == self.cir.idents.builtin_module) { + break :blk if (self.builtin_ctx.builtin_module) |builtin_env| builtin_env else self.cir; + } + // Type is defined in an imported module; find it by matching idents. + // insertIdent converts the module name string to an Ident.Idx for + // index-based comparison (same approach as checkStaticDispatchConstraints). + for (self.imported_modules) |imported_env| { + const imported_name = if (!imported_env.qualified_module_ident.isNone()) + imported_env.getIdent(imported_env.qualified_module_ident) + else + imported_env.module_name; + const imported_module_ident = try self.cir.insertIdent(base.Ident.for_text(imported_name)); + if (imported_module_ident == origin_module_ident) break :blk imported_env; + } + return false; // Origin module not found; cannot determine if it's a method. + }; + + return original_env.lookupMethodIdentFromEnvConst(self.cir, nominal.ident.ident_idx, method_ident) != null; +} + /// Check static dispatch constraints /// /// Note that new constraints can be added as we are processing. For example: diff --git a/src/check/problem/context.zig b/src/check/problem/context.zig index 3c8e6dfb074..f902dfba14d 100644 --- a/src/check/problem/context.zig +++ b/src/check/problem/context.zig @@ -181,6 +181,9 @@ pub const Context = union(enum) { /// We have to use the real region here, because the field does not /// have its own CIR node field_region: base.Region, + /// True if field_name is actually a method on the receiver type, + /// i.e. the user wrote `receiver.method` instead of `receiver.method()` + is_method: bool = false, }; /// Context for record update type errors diff --git a/src/check/report.zig b/src/check/report.zig index f38421452cb..85fd167e546 100644 --- a/src/check/report.zig +++ b/src/check/report.zig @@ -2118,6 +2118,23 @@ pub const ReportBuilder = struct { const region = ProblemRegion{ .simple = regionIdxFrom(types.actual_var) }; switch (record) { .not_a_record => { + if (ctx.is_method) { + return try self.makeCustomReport( + region, + &.{ + D.ident(ctx.field_name).withAnnotation(.inline_code), + D.bytes("is a method, not a record field. Did you forget the parentheses?"), + }, + &.{ + &.{ + D.bytes("Method calls require parentheses even with no arguments. Use"), + D.ident(ctx.field_name).withAnnotation(.inline_code), + D.bytes("()").withNoPrecedingSpace(), + D.bytes("instead."), + }, + }, + ); + } return try self.makeBadTypeReport( region, &.{D.bytes("This is not a record, so it does not have any fields to access:")}, diff --git a/src/check/test/type_checking_integration.zig b/src/check/test/type_checking_integration.zig index 149449a4223..5730e34c9c9 100644 --- a/src/check/test/type_checking_integration.zig +++ b/src/check/test/type_checking_integration.zig @@ -1810,6 +1810,30 @@ test "check type - record - access - not a record" { try checkTypesModule(source, .fail, "TYPE MISMATCH"); } +test "check type - record - access - method missing parentheses" { + const source = + \\MyType := [A].{ + \\ my_method = || 42 + \\} + \\ + \\x = MyType.A + \\y = x.my_method + ; + try checkTypesModule(source, .fail_with, + \\**TYPE MISMATCH** + \\`my_method` is a method, not a record field. Did you forget the parentheses? + \\**test:6:5:6:6:** + \\```roc + \\y = x.my_method + \\``` + \\ ^ + \\ + \\Method calls require parentheses even with no arguments. Use `my_method`() instead. + \\ + \\ + ); +} + // record update test "check type - record - update 1" { From eb37ac3f959fb19ac21430c0ccce4ed2b16284dc Mon Sep 17 00:00:00 2001 From: Felipe Vogel Date: Sat, 7 Mar 2026 15:36:45 -0500 Subject: [PATCH 2/5] Change problem region to the method --- src/check/report.zig | 7 ++++++- src/check/test/type_checking_integration.zig | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/check/report.zig b/src/check/report.zig index 85fd167e546..706562c1ec4 100644 --- a/src/check/report.zig +++ b/src/check/report.zig @@ -224,6 +224,8 @@ pub const ReportBuilder = struct { const ProblemRegion = union(enum) { simple: Region.Idx, focused: struct { outer: Region.Idx, highlight: Region.Idx }, + /// A direct Region value, for when only a Region (not a Region.Idx) is available. + region: Region, }; /// Region for source highlighting - can be either a Region.Idx or a direct Region @@ -435,6 +437,7 @@ pub const ReportBuilder = struct { .focused => |ctx| { try self.addFocusedSourceHighlight(&report, ctx.outer, ctx.highlight); }, + .region => |r| try self.addSourceHighlightRegion(&report, r), } try report.document.addLineBreak(); @@ -498,6 +501,7 @@ pub const ReportBuilder = struct { switch (region) { .simple => |region_idx| try self.addSourceHighlight(&report, region_idx), .focused => |ctx| try self.addFocusedSourceHighlight(&report, ctx.outer, ctx.highlight), + .region => |r| try self.addSourceHighlightRegion(&report, r), } try report.document.addLineBreak(); @@ -541,6 +545,7 @@ pub const ReportBuilder = struct { .focused => |ctx| { try self.addFocusedSourceHighlight(&report, ctx.outer, ctx.highlight); }, + .region => |r| try self.addSourceHighlightRegion(&report, r), } // Print static hints @@ -2120,7 +2125,7 @@ pub const ReportBuilder = struct { .not_a_record => { if (ctx.is_method) { return try self.makeCustomReport( - region, + ProblemRegion{ .region = ctx.field_region }, &.{ D.ident(ctx.field_name).withAnnotation(.inline_code), D.bytes("is a method, not a record field. Did you forget the parentheses?"), diff --git a/src/check/test/type_checking_integration.zig b/src/check/test/type_checking_integration.zig index 5730e34c9c9..29914e0cf5d 100644 --- a/src/check/test/type_checking_integration.zig +++ b/src/check/test/type_checking_integration.zig @@ -1822,11 +1822,11 @@ test "check type - record - access - method missing parentheses" { try checkTypesModule(source, .fail_with, \\**TYPE MISMATCH** \\`my_method` is a method, not a record field. Did you forget the parentheses? - \\**test:6:5:6:6:** + \\**test:6:6:6:16:** \\```roc \\y = x.my_method \\``` - \\ ^ + \\ ^^^^^^^^^^ \\ \\Method calls require parentheses even with no arguments. Use `my_method`() instead. \\ From 25f0afa05ed8308a51e1e270b61b8916510a17df Mon Sep 17 00:00:00 2001 From: Felipe Vogel Date: Sat, 7 Mar 2026 15:45:33 -0500 Subject: [PATCH 3/5] Highlight parentheses as well as identifier in suggestion --- src/check/report.zig | 23 ++++++++++++++++++-- src/check/test/type_checking_integration.zig | 2 +- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/check/report.zig b/src/check/report.zig index 706562c1ec4..e0f265efd14 100644 --- a/src/check/report.zig +++ b/src/check/report.zig @@ -281,6 +281,7 @@ pub const ReportBuilder = struct { bytes: []const u8, link: []const u8, ident: Ident.Idx, + ident_call: Ident.Idx, num_ord: u32, num: u32, }, @@ -316,6 +317,15 @@ pub const ReportBuilder = struct { }; } + /// Create a Doc that renders as "name()" — for showing a method call suggestion. + fn identCall(b: Ident.Idx) Doc { + return Doc{ + .type_ = .{ .ident_call = b }, + .preceding_space = true, + .annotation = null, + }; + } + /// Create a Doc containing an ordinal number (1st, 2nd, 3rd, etc.). fn num_ord(b: u32) Doc { return Doc{ @@ -369,6 +379,16 @@ pub const ReportBuilder = struct { try report.document.addReflowingText(ident_bytes); } }, + .ident_call => |i| { + var buf: [512]u8 = undefined; + const with_parens = try std.fmt.bufPrint(&buf, "{s}()", .{builder.can_ir.getIdent(i)}); + const owned = try report.addOwnedString(with_parens); + if (doc.annotation) |annotation| { + try report.document.addAnnotated(owned, annotation); + } else { + try report.document.addReflowingText(owned); + } + }, .num_ord => |ord| { const ord_bytes = try builder.getOrdinalOwned(report, ord); if (doc.annotation) |annotation| { @@ -2133,8 +2153,7 @@ pub const ReportBuilder = struct { &.{ &.{ D.bytes("Method calls require parentheses even with no arguments. Use"), - D.ident(ctx.field_name).withAnnotation(.inline_code), - D.bytes("()").withNoPrecedingSpace(), + D.identCall(ctx.field_name).withAnnotation(.inline_code), D.bytes("instead."), }, }, diff --git a/src/check/test/type_checking_integration.zig b/src/check/test/type_checking_integration.zig index 29914e0cf5d..fcf3a4be74f 100644 --- a/src/check/test/type_checking_integration.zig +++ b/src/check/test/type_checking_integration.zig @@ -1828,7 +1828,7 @@ test "check type - record - access - method missing parentheses" { \\``` \\ ^^^^^^^^^^ \\ - \\Method calls require parentheses even with no arguments. Use `my_method`() instead. + \\Method calls require parentheses even with no arguments. Use `my_method()` instead. \\ \\ ); From 5f2dbb6e48ea1db5b40c815b03558e70d5c0cc7a Mon Sep 17 00:00:00 2001 From: Felipe Vogel Date: Fri, 20 Mar 2026 22:34:11 -0400 Subject: [PATCH 4/5] Call isMethodOnNominalType only on unification failure --- src/check/Check.zig | 43 ++++++++++++++++++++++++------------- src/check/problem/store.zig | 6 ++++++ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/check/Check.zig b/src/check/Check.zig index fdd9b3224c5..4c4b8b6fcb5 100644 --- a/src/check/Check.zig +++ b/src/check/Check.zig @@ -4380,17 +4380,27 @@ fn checkExpr(self: *Self, expr_idx: CIR.Expr.Idx, env: *Env, expected: Expected) .record = .{ .fields = record_field_range, .ext = record_ext_var }, } }, env, expr_region); - // Check if the field name is actually a method on the receiver type. - // This helps provide a better error when the user writes `receiver.method` - // instead of `receiver.method()`. - const is_method = try self.isMethodOnNominalType(receiver_var, dot_access.field_name); - - // Then, unify the actual receiver type with the expected record - _ = try self.unifyInContext(record_being_accessed, receiver_var, env, .{ .record_access = .{ + // Snapshot the receiver's nominal type (O(1)) before unifyInContext, which + // clobbers vars to .err on failure. The expensive method lookup (isMethodOnNominal) + // is deferred to the failure path so we don't pay for it on successful field accesses. + // + // If more context fields need this lazy-evaluation pattern, refactor unifyInContext + // to accept a context-builder (anytype with a build() method) called only on failure, + // rather than patching the stored problem afterward. + const receiver_nominal = self.nominalTypeOf(receiver_var); + const unify_result = try self.unifyInContext(record_being_accessed, receiver_var, env, .{ .record_access = .{ .field_name = dot_access.field_name, .field_region = dot_access.field_name_region, - .is_method = is_method, } }); + if (unify_result == .problem) { + if (receiver_nominal) |nominal| { + const is_method = try self.isMethodOnNominal(nominal, dot_access.field_name); + // This assumes unifyInContext always stores a type_mismatch problem, which is + // currently true. If that changes, this will panic. The refactor mentioned above + // would eliminate this assumption. + self.problems.getPtr(unify_result.problem).type_mismatch.context.record_access.is_method = is_method; + } + } _ = try self.unify(expr_var, record_field_var, env); } }, @@ -5939,18 +5949,21 @@ fn checkConstraints(self: *Self, env: *Env) std.mem.Allocator.Error!void { self.constraints.items.clearRetainingCapacity(); } -/// Returns true if `method_ident` is a known method on the nominal type of `receiver_var`. -/// Used to detect when a user writes `receiver.method` (missing parentheses) instead of -/// `receiver.method()`. -fn isMethodOnNominalType(self: *Self, receiver_var: Var, method_ident: Ident.Idx) std.mem.Allocator.Error!bool { - const nominal = switch (self.types.resolveVar(receiver_var).desc.content) { +/// Extracts the NominalType from a var if it resolves to one, otherwise returns null. +/// Cheap (O(1)) — safe to call before unification, which may clobber the var. +fn nominalTypeOf(self: *Self, var_: Var) ?types_mod.NominalType { + return switch (self.types.resolveVar(var_).desc.content) { .structure => |s| switch (s) { .nominal_type => |n| n, - else => return false, + else => null, }, - else => return false, + else => null, }; +} +/// Returns true if `method_ident` is a known method on `nominal`. +/// Expensive — searches module environments. Call only when needed (e.g. on unification failure). +fn isMethodOnNominal(self: *Self, nominal: types_mod.NominalType, method_ident: Ident.Idx) std.mem.Allocator.Error!bool { const origin_module_ident = nominal.origin_module; // Find the module env that owns the type's method definitions. const original_env: *const ModuleEnv = blk: { diff --git a/src/check/problem/store.zig b/src/check/problem/store.zig index 5f8f74cdbdb..555baf795cc 100644 --- a/src/check/problem/store.zig +++ b/src/check/problem/store.zig @@ -91,6 +91,12 @@ pub const Store = struct { return self.problems.items[@intFromEnum(idx)]; } + /// Get a mutable pointer to a stored problem. Useful when some context fields can only + /// be computed after unification (e.g. expensive lookups deferred to the failure path). + pub fn getPtr(self: *Self, idx: Problem.Idx) *Problem { + return &self.problems.items[@intFromEnum(idx)]; + } + pub fn len(self: *Self) usize { return self.problems.items.len; } From 224432bffd204a1cba0baf016c9530ca16f44331 Mon Sep 17 00:00:00 2001 From: Felipe Vogel Date: Fri, 20 Mar 2026 22:32:28 -0400 Subject: [PATCH 5/5] Refactor lazy isMethodOnNominalType check using a context builder --- src/check/Check.zig | 127 +++++++++++++++++++++--------------- src/check/problem/store.zig | 6 -- src/check/unify.zig | 118 +++++++++++++++++++++------------ 3 files changed, 149 insertions(+), 102 deletions(-) diff --git a/src/check/Check.zig b/src/check/Check.zig index 4c4b8b6fcb5..39b6e0e2c22 100644 --- a/src/check/Check.zig +++ b/src/check/Check.zig @@ -419,57 +419,92 @@ fn unify(self: *Self, a: Var, b: Var, env: *Env) std.mem.Allocator.Error!unifier return self.unifyInContext(a, b, env, .none); } -/// Unify two types where `a` is the expected type and `b` is the actual type -/// Accepts a full unifier config for fine-grained control +/// Context builder for unifyBuildingContext that wraps an already-constructed context value. +/// Use this when the context is cheap to build upfront and no lazy evaluation is needed. +const EagerContextBuilder = struct { + ctx: problem.Context, + fn build(self: @This()) std.mem.Allocator.Error!problem.Context { + return self.ctx; + } +}; + +/// Context builder for unifyBuildingContext when checking dot access on a record. +/// Lazily checks whether the field name is actually a method on a nominal type, +/// so the expensive isMethodOnNominalType lookup only runs on unification failure. +const RecordAccessContextBuilder = struct { + checker: *Self, + field_name: Ident.Idx, + field_region: base.Region, + receiver_var: Var, + fn build(self: @This()) std.mem.Allocator.Error!problem.Context { + const is_method = try self.checker.isMethodOnNominalType(self.receiver_var, self.field_name); + return .{ .record_access = .{ + .field_name = self.field_name, + .field_region = self.field_region, + .is_method = is_method, + } }; + } +}; + +/// Unify two types where `a` is the expected type and `b` is the actual type. +/// Delegates to unifyBuildingContext (not unifier.unifyInContext — Check uses unifyRaw +/// directly so it can control when vars are clobbered and problems are recorded). +/// For lazy context construction (e.g. to skip expensive lookups on the success path), +/// use unifyBuildingContext instead. fn unifyInContext(self: *Self, a: Var, b: Var, env: *Env, ctx: problem.Context) std.mem.Allocator.Error!unifier.Result { + return self.unifyBuildingContext(a, b, env, EagerContextBuilder{ .ctx = ctx }); +} + +/// Like unifyInContext, but the context is built lazily via a callback, only if unification +/// fails. This avoids expensive context computation on the success path. +/// +/// `ctx_builder` must have a `build() !problem.Context` method. It is only called on failure, +/// while the type variables are still live (before they are clobbered to `.err`). +fn unifyBuildingContext(self: *Self, a: Var, b: Var, env: *Env, ctx_builder: anytype) std.mem.Allocator.Error!unifier.Result { + comptime { + const T = @TypeOf(ctx_builder); + if (!@hasDecl(T, "build")) @compileError(@typeName(T) ++ " must have a `build() !problem.Context` method"); + } const trace = tracy.trace(@src()); defer trace.end(); - // Unify - const result = try unifier.unifyInContext( + const raw = try unifier.unifyRaw( self.cir, self.types, - &self.problems, &self.snapshots, &self.type_writer, &self.unify_scratch, &self.occurs_scratch, a, b, - ctx, ); - // Set regions and add to the current rank all variables created during unification. - // - // We assign all fresh variables the region of `b` (the "actual" type), since `a` is - // typically the "expected" type from an annotation. This heuristic works well for - // most cases but can be imprecise for deeply nested unifications where fresh variables - // are created for sub-components (e.g., record fields, tag payloads). In those cases, - // error messages may point to the outer expression rather than the specific field. - // - // A more precise solution would track the origin of each fresh variable during - // unification and propagate that back, but the current approach is sufficient for - // typical error reporting scenarios. + // Process fresh vars and deferred constraints regardless of success or failure. const region = self.getRegionAt(b); for (self.unify_scratch.fresh_vars.items.items) |fresh_var| { - // Set the rank const fresh_rank = self.types.resolveVar(fresh_var).desc.rank; try env.var_pool.addVarToRank(fresh_var, fresh_rank); - - // Set the region try self.fillInRegionsThrough(fresh_var); self.setRegionAt(fresh_var, region); } - - // Copy any constraints created during unification into our own array for (self.unify_scratch.deferred_constraints.items.items) |deferred_constraint| { _ = try env.deferred_static_dispatch_constraints.append(self.gpa, deferred_constraint); } - - // Ensure arrays are in sync self.debugAssertArraysInSync(); - return result; + switch (raw) { + .ok => return .ok, + .mismatch => |types_pair| { + // Vars are still live here — build the context before clobbering them. + const context = try ctx_builder.build(); + const problem_idx = try self.problems.appendProblem(self.cir.gpa, .{ .type_mismatch = .{ + .types = types_pair, + .context = context, + } }); + self.types.union_(a, b, .{ .content = .err, .rank = Rank.generalized }); + return .{ .problem = problem_idx }; + }, + } } /// Check if a variable contains an infinite type after solving a definition. @@ -4380,27 +4415,14 @@ fn checkExpr(self: *Self, expr_idx: CIR.Expr.Idx, env: *Env, expected: Expected) .record = .{ .fields = record_field_range, .ext = record_ext_var }, } }, env, expr_region); - // Snapshot the receiver's nominal type (O(1)) before unifyInContext, which - // clobbers vars to .err on failure. The expensive method lookup (isMethodOnNominal) - // is deferred to the failure path so we don't pay for it on successful field accesses. - // - // If more context fields need this lazy-evaluation pattern, refactor unifyInContext - // to accept a context-builder (anytype with a build() method) called only on failure, - // rather than patching the stored problem afterward. - const receiver_nominal = self.nominalTypeOf(receiver_var); - const unify_result = try self.unifyInContext(record_being_accessed, receiver_var, env, .{ .record_access = .{ + // Use unifyBuildingContext so the is_method check only runs on failure, + // while receiver_var is still live (not yet clobbered to .err). + _ = try self.unifyBuildingContext(record_being_accessed, receiver_var, env, RecordAccessContextBuilder{ + .checker = self, .field_name = dot_access.field_name, .field_region = dot_access.field_name_region, - } }); - if (unify_result == .problem) { - if (receiver_nominal) |nominal| { - const is_method = try self.isMethodOnNominal(nominal, dot_access.field_name); - // This assumes unifyInContext always stores a type_mismatch problem, which is - // currently true. If that changes, this will panic. The refactor mentioned above - // would eliminate this assumption. - self.problems.getPtr(unify_result.problem).type_mismatch.context.record_access.is_method = is_method; - } - } + .receiver_var = receiver_var, + }); _ = try self.unify(expr_var, record_field_var, env); } }, @@ -5949,21 +5971,18 @@ fn checkConstraints(self: *Self, env: *Env) std.mem.Allocator.Error!void { self.constraints.items.clearRetainingCapacity(); } -/// Extracts the NominalType from a var if it resolves to one, otherwise returns null. -/// Cheap (O(1)) — safe to call before unification, which may clobber the var. -fn nominalTypeOf(self: *Self, var_: Var) ?types_mod.NominalType { - return switch (self.types.resolveVar(var_).desc.content) { +/// Returns true if `method_ident` is a known method on the nominal type of `receiver_var`. +/// Used to detect when a user writes `receiver.method` (missing parentheses) instead of +/// `receiver.method()`. +fn isMethodOnNominalType(self: *Self, receiver_var: Var, method_ident: Ident.Idx) std.mem.Allocator.Error!bool { + const nominal = switch (self.types.resolveVar(receiver_var).desc.content) { .structure => |s| switch (s) { .nominal_type => |n| n, - else => null, + else => return false, }, - else => null, + else => return false, }; -} -/// Returns true if `method_ident` is a known method on `nominal`. -/// Expensive — searches module environments. Call only when needed (e.g. on unification failure). -fn isMethodOnNominal(self: *Self, nominal: types_mod.NominalType, method_ident: Ident.Idx) std.mem.Allocator.Error!bool { const origin_module_ident = nominal.origin_module; // Find the module env that owns the type's method definitions. const original_env: *const ModuleEnv = blk: { diff --git a/src/check/problem/store.zig b/src/check/problem/store.zig index 555baf795cc..5f8f74cdbdb 100644 --- a/src/check/problem/store.zig +++ b/src/check/problem/store.zig @@ -91,12 +91,6 @@ pub const Store = struct { return self.problems.items[@intFromEnum(idx)]; } - /// Get a mutable pointer to a stored problem. Useful when some context fields can only - /// be computed after unification (e.g. expensive lookups deferred to the failure path). - pub fn getPtr(self: *Self, idx: Problem.Idx) *Problem { - return &self.problems.items[@intFromEnum(idx)]; - } - pub fn len(self: *Self) usize { return self.problems.items.len; } diff --git a/src/check/unify.zig b/src/check/unify.zig index cf3cc6ed7c3..597c3cc52b1 100644 --- a/src/check/unify.zig +++ b/src/check/unify.zig @@ -11,19 +11,25 @@ //! * numbers (polymorphic & compacted) //! * effect and purity tracking //! -//! The primary entrypoint is `unify`, which takes two type variables (`Var`) and attempts -//! to unify their structure in the given `Store`, using a mutable `Scratch` context -//! to hold intermediate buffers and allocations. +//! Entrypoints: +//! * `unify` — unify two vars with no context (no error reporting) +//! * `unifyInContext` — unify and record a problem with a pre-built context on failure +//! * `unifyRaw` — unify and capture snapshots on failure, without recording the problem +//! or clobbering vars; lets callers build the context lazily before committing +//! +//! All entrypoints take two type variables (`Var`) and attempt to unify their structure +//! in the given `Store`, using a mutable `Scratch` context to hold intermediate buffers +//! and allocations. //! //! The `Scratch` struct is designed to be reused across many unification calls. //! It owns internal scratch buffers for record fields, shared fields, and fresh variables. //! -//! Each call to `unify` will reset the scratch buffer. If the unification succeeds, +//! Each unification call will reset the scratch buffer. If the unification succeeds, //! the caller can access `scratch.fresh_vars` to retrieve all type variables created //! during that unification pass. These fresh variables are useful for later type //! generalization, let-binding, or monomorphization. //! -//! NOTE: Subsequent calls to `unify` will reset `fresh_vars`. It is up to the caller +//! NOTE: Subsequent unification calls will reset `fresh_vars`. It is up to the caller //! to use/store them if necessary. //! //! ### Example @@ -140,6 +146,56 @@ pub fn unify( ); } +/// The raw result of unification: either success, or a type mismatch with snapshots +/// captured before vars are clobbered. Does not include a context. +pub const RawResult = union(enum) { + ok, + mismatch: problem_mod.TypePair, +}; + +/// Runs unification and captures snapshots on failure, but does NOT record the problem, +/// build a context, or clobber vars. This enables lazy context construction on the failure +/// path (e.g. to skip expensive lookups on the success path). +/// +/// Callers must handle the `.mismatch` case by: building the context, recording the +/// problem via `problems.appendProblem`, and clobbering vars via `types.union_`. +pub fn unifyRaw( + module_env: *ModuleEnv, + types: *types_mod.Store, + snapshots: *snapshot_mod.Store, + type_writer: *types_mod.TypeWriter, + unify_scratch: *Scratch, + occurs_scratch: *occurs.Scratch, + /// The "expected" variable + a: Var, + /// The "actual" variable + b: Var, +) std.mem.Allocator.Error!RawResult { + const trace = tracy.trace(@src()); + defer trace.end(); + + unify_scratch.reset(); + + var unifier = Unifier.init(module_env, types, unify_scratch, occurs_scratch); + unifier.unifyGuarded(a, b) catch |err| { + switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.TypeMismatch => { + const expected_snapshot = try snapshots.snapshotVarForError(types, type_writer, a); + const actual_snapshot = try snapshots.snapshotVarForError(types, type_writer, b); + return .{ .mismatch = .{ + .expected_var = a, + .expected_snapshot = expected_snapshot, + .actual_var = b, + .actual_snapshot = actual_snapshot, + } }; + }, + } + }; + + return .ok; +} + /// Unify two type variables /// /// This function @@ -147,7 +203,9 @@ pub fn unify( /// * Compares variable contents for equality /// * Merges unified variables so 1 is "root" and the other is "redirect" /// -/// This function accepts a context and optional constraint origin var (for better error reporting) +/// Accepts a context for error reporting. For lazy context construction, call +/// unifyRaw directly and handle the mismatch case yourself. Note: Check.zig uses +/// unifyRaw via unifyBuildingContext rather than calling this function. pub fn unifyInContext( module_env: *ModuleEnv, types: *types_mod.Store, @@ -165,42 +223,18 @@ pub fn unifyInContext( const trace = tracy.trace(@src()); defer trace.end(); - // First reset the scratch store - unify_scratch.reset(); - - // Unify - var unifier = Unifier.init(module_env, types, unify_scratch, occurs_scratch); - unifier.unifyGuarded(a, b) catch |err| { - const problem: Problem = blk: { - switch (err) { - error.OutOfMemory => { - return error.OutOfMemory; - }, - error.TypeMismatch => { - const expected_snapshot = try snapshots.snapshotVarForError(types, type_writer, a); - const actual_snapshot = try snapshots.snapshotVarForError(types, type_writer, b); - - break :blk .{ .type_mismatch = .{ - .types = .{ - .expected_var = a, - .expected_snapshot = expected_snapshot, - .actual_var = b, - .actual_snapshot = actual_snapshot, - }, - .context = context, - } }; - }, - } - }; - const problem_idx = try problems.appendProblem(module_env.gpa, problem); - types.union_(a, b, .{ - .content = .err, - .rank = Rank.generalized, - }); - return Result{ .problem = problem_idx }; - }; - - return .ok; + const raw = try unifyRaw(module_env, types, snapshots, type_writer, unify_scratch, occurs_scratch, a, b); + switch (raw) { + .ok => return .ok, + .mismatch => |types_pair| { + const problem_idx = try problems.appendProblem(module_env.gpa, .{ .type_mismatch = .{ + .types = types_pair, + .context = context, + } }); + types.union_(a, b, .{ .content = .err, .rank = Rank.generalized }); + return .{ .problem = problem_idx }; + }, + } } /// A temporary unification context used to unify two type variables within a `Store`.