From aec6663179d01c94dab3a3df2349ebcefe85e8ab Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 9 Dec 2025 17:06:40 -0500 Subject: [PATCH 1/2] Fix some snapshot logic --- src/check/problem.zig | 13 ++++--- src/check/snapshot.zig | 65 ++++++++++++++++++++++++++++++++--- src/check/test/unify_test.zig | 20 +++++++++++ src/check/unify.zig | 8 ++--- 4 files changed, 91 insertions(+), 15 deletions(-) diff --git a/src/check/problem.zig b/src/check/problem.zig index c22635c507..30f2644f95 100644 --- a/src/check/problem.zig +++ b/src/check/problem.zig @@ -48,8 +48,8 @@ pub const Problem = union(enum) { negative_unsigned_int: NegativeUnsignedInt, invalid_numeric_literal: InvalidNumericLiteral, unused_value: UnusedValue, - infinite_recursion: struct { var_: Var }, - anonymous_recursion: struct { var_: Var }, + infinite_recursion: VarProblem1, + anonymous_recursion: VarProblem1, invalid_number_type: VarProblem1, invalid_record_ext: VarProblem1, invalid_tag_union_ext: VarProblem1, @@ -331,12 +331,11 @@ pub const ReportBuilder = struct { self.bytes_buf.deinit(); } - /// Get the formatted string for a snapshot, asserting it exists + /// Get the formatted string for a snapshot. + /// Returns a placeholder if the formatted string is missing, allowing error reporting + /// to continue gracefully even if snapshots are incomplete. fn getFormattedString(self: *const Self, idx: SnapshotContentIdx) []const u8 { - return self.snapshots.getFormattedString(idx) orelse { - std.debug.assert(false); // Missing formatted string for snapshot - unreachable; - }; + return self.snapshots.getFormattedString(idx) orelse ""; } /// Build a report for a problem diff --git a/src/check/snapshot.zig b/src/check/snapshot.zig index 0cc5054244..5fe1d00e02 100644 --- a/src/check/snapshot.zig +++ b/src/check/snapshot.zig @@ -594,13 +594,70 @@ pub const Store = struct { const args = self.content_indexes.sliceRange(tag.args); for (args) |arg_idx| { try result.append(' '); - const formatted = self.getFormattedString(arg_idx) orelse { - std.debug.assert(false); // Missing formatted string for tag argument - snapshotVarForError must be called for all nested types - unreachable; - }; + const formatted = self.getFormattedString(arg_idx) orelse ""; try result.appendSlice(formatted); } return result.toOwnedSlice(); } }; + +// Tests + +test "formatTagString - gracefully handles missing formatted strings" { + const gpa = std.testing.allocator; + + var store = try Store.initCapacity(gpa, 16); + defer store.deinit(); + + // Create a tag with an argument that doesn't have a formatted string + // This should use the "" fallback instead of crashing + const unknown_content_idx = try store.contents.append(gpa, .err); + const args_range = try store.content_indexes.appendSlice(gpa, &[_]SnapshotContentIdx{unknown_content_idx}); + + // Create an ident store for the tag name + var ident_store = try Ident.Store.initCapacity(gpa, 64); + defer ident_store.deinit(gpa); + const tag_name = try ident_store.insert(gpa, Ident.for_text("MyTag")); + + const tag = SnapshotTag{ + .name = tag_name, + .args = args_range, + }; + + // Format should succeed and include the fallback placeholder + const result = try store.formatTagString(gpa, tag, &ident_store); + defer gpa.free(result); + + try std.testing.expectEqualStrings("MyTag ", result); +} + +test "formatTagString - uses stored formatted strings when available" { + const gpa = std.testing.allocator; + + var store = try Store.initCapacity(gpa, 16); + defer store.deinit(); + + // Create a content index and store a formatted string for it + const content_idx = try store.contents.append(gpa, .err); + const formatted_str = try gpa.dupe(u8, "U64"); + try store.formatted_strings.put(gpa, content_idx, formatted_str); + + const args_range = try store.content_indexes.appendSlice(gpa, &[_]SnapshotContentIdx{content_idx}); + + // Create an ident store for the tag name + var ident_store = try Ident.Store.initCapacity(gpa, 64); + defer ident_store.deinit(gpa); + const tag_name = try ident_store.insert(gpa, Ident.for_text("Some")); + + const tag = SnapshotTag{ + .name = tag_name, + .args = args_range, + }; + + // Format should use the stored formatted string + const result = try store.formatTagString(gpa, tag, &ident_store); + defer gpa.free(result); + + try std.testing.expectEqualStrings("Some U64", result); +} diff --git a/src/check/test/unify_test.zig b/src/check/test/unify_test.zig index 9f0df1a0ae..49469b1750 100644 --- a/src/check/test/unify_test.zig +++ b/src/check/test/unify_test.zig @@ -1324,6 +1324,16 @@ test "unify - fails on infinite type" { .problem => |problem_idx| { const problem = env.problems.get(problem_idx); try std.testing.expectEqual(.infinite_recursion, @as(Problem.Tag, problem)); + + // Verify that a snapshot was created for the recursion error + const snapshot_idx = problem.infinite_recursion.snapshot; + const snapshot_content = env.snapshots.getContent(snapshot_idx); + // The snapshot should be some valid content (not just err) + try std.testing.expect(snapshot_content != .err); + + // Verify a formatted string was created + const formatted = env.snapshots.getFormattedString(snapshot_idx); + try std.testing.expect(formatted != null); }, } } @@ -1352,6 +1362,16 @@ test "unify - fails on anonymous recursion" { .problem => |problem_idx| { const problem = env.problems.get(problem_idx); try std.testing.expectEqual(.anonymous_recursion, @as(Problem.Tag, problem)); + + // Verify that a snapshot was created for the recursion error + const snapshot_idx = problem.anonymous_recursion.snapshot; + const snapshot_content = env.snapshots.getContent(snapshot_idx); + // The snapshot should be some valid content (not just err) + try std.testing.expect(snapshot_content != .err); + + // Verify a formatted string was created + const formatted = env.snapshots.getFormattedString(snapshot_idx); + try std.testing.expect(formatted != null); }, } } diff --git a/src/check/unify.zig b/src/check/unify.zig index 12c75bc438..6aebf9df22 100644 --- a/src/check/unify.zig +++ b/src/check/unify.zig @@ -280,17 +280,17 @@ pub fn unifyWithConf( if (unify_scratch.err) |unify_err| { switch (unify_err) { .recursion_anonymous => |var_| { - // TODO: Snapshot infinite recursion - // const snapshot = snapshots.deepCopyVar(types, var_); + const snapshot = try snapshots.snapshotVarForError(types, type_writer, var_); break :blk .{ .anonymous_recursion = .{ .var_ = var_, + .snapshot = snapshot, } }; }, .recursion_infinite => |var_| { - // TODO: Snapshot infinite recursion - // const snapshot = snapshots.deepCopyVar(types, var_); + const snapshot = try snapshots.snapshotVarForError(types, type_writer, var_); break :blk .{ .infinite_recursion = .{ .var_ = var_, + .snapshot = snapshot, } }; }, .invalid_number_type => |var_| { From 5a3f900b6fb565c8e2714ef6d3d0071f8851d605 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Tue, 9 Dec 2025 17:11:51 -0500 Subject: [PATCH 2/2] Rename VarProblem1 to VarWithSnapshot --- src/check/problem.zig | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/check/problem.zig b/src/check/problem.zig index 30f2644f95..5a502f73c1 100644 --- a/src/check/problem.zig +++ b/src/check/problem.zig @@ -48,11 +48,11 @@ pub const Problem = union(enum) { negative_unsigned_int: NegativeUnsignedInt, invalid_numeric_literal: InvalidNumericLiteral, unused_value: UnusedValue, - infinite_recursion: VarProblem1, - anonymous_recursion: VarProblem1, - invalid_number_type: VarProblem1, - invalid_record_ext: VarProblem1, - invalid_tag_union_ext: VarProblem1, + infinite_recursion: VarWithSnapshot, + anonymous_recursion: VarWithSnapshot, + invalid_number_type: VarWithSnapshot, + invalid_record_ext: VarWithSnapshot, + invalid_tag_union_ext: VarWithSnapshot, bug: Bug, comptime_crash: ComptimeCrash, comptime_expect_failed: ComptimeExpectFailed, @@ -80,8 +80,9 @@ pub const ComptimeEvalError = struct { region: base.Region, }; -/// A single var problem -pub const VarProblem1 = struct { +/// A problem involving a single type variable, with a snapshot for error reporting. +/// Used for recursion errors, invalid extension types, etc. +pub const VarWithSnapshot = struct { var_: Var, snapshot: SnapshotContentIdx, };