Merge pull request #8608 from roc-lang/fix-snapshots

Fix some snapshot logic

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: struct { var_: Var },
-    anonymous_recursion: struct { var_: Var },
-    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,
 };
@@ -331,12 +332,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 "<unknown type>";
     }
 
     /// Build a report for a problem

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 "<unknown type>";
             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 "<unknown type>" 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 <unknown type>", 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);
+}

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);
         },
     }
 }

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_| {