Add TODO/detection/test case for known bug regarding source mapping/c…

…omment removal
squeek502 · Nov 18, 2023 · e29db80 · e29db80
1 parent 4781340
commit e29db80
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 3 deletions.
diff --git a/src/main.zig b/src/main.zig
@@ -173,7 +173,13 @@ pub fn main() !void {
     // may be a mismatch in how the line directive strings are parsed versus
     // how they are escaped/written by the preprocessor.
 
-    var final_input = try removeComments(mapping_results.result, mapping_results.result, &mapping_results.mappings);
+    var final_input = removeComments(mapping_results.result, mapping_results.result, &mapping_results.mappings) catch |err| switch (err) {
+        error.InvalidSourceMappingCollapse => {
+            try renderErrorMessage(stderr.writer(), stderr_config, .err, "failed during comment removal; this is a known bug", .{});
+            std.os.exit(1);
+        },
+        else => |e| return e,
+    };
 
     var output_file = std.fs.cwd().createFile(options.output_filename, .{}) catch |err| {
         try renderErrorMessage(stderr.writer(), stderr_config, .err, "unable to create output file '{s}': {s}", .{ options.output_filename, @errorName(err) });

diff --git a/src/source_mapping.zig b/src/source_mapping.zig
@@ -575,10 +575,22 @@ pub const SourceMappings = struct {
             .current = node,
             .previous = node.children[0],
         };
-        // skip past current
-        _ = it.next();
+        // skip past current, but store it
+        var prev = it.next().?;
         while (it.next()) |inorder_node| {
             inorder_node.key.start_line -= span_diff;
+
+            // This can only really happen if there are #line commands within
+            // a multiline comment, which in theory should be skipped over.
+            // However, currently, parseAndRemoveLineCommands is not aware of
+            // comments at all.
+            //
+            // TODO: Make parseAndRemoveLineCommands aware of comments/strings
+            //       and turn this into an assertion
+            if (prev.key.start_line > inorder_node.key.start_line) {
+                return error.InvalidSourceMappingCollapse;
+            }
+            prev = inorder_node;
         }
         self.end_line -= span_diff;
     }
@@ -792,3 +804,28 @@ test "in place" {
     defer result.mappings.deinit(std.testing.allocator);
     try std.testing.expectEqualStrings("", result.result);
 }
+
+test "line command within a multiline comment" {
+    // TODO: Enable once parseAndRemoveLineCommands is comment-aware
+    if (true) return error.SkipZigTest;
+
+    try testParseAndRemoveLineCommands(
+        \\/*
+        \\#line 1 "irrelevant.rc"
+        \\
+        \\
+        \\*/
+    , &[_]ExpectedSourceSpan{
+        .{ .start_line = 1, .end_line = 1, .filename = "blah.rc" },
+        .{ .start_line = 2, .end_line = 2, .filename = "blah.rc" },
+        .{ .start_line = 3, .end_line = 3, .filename = "blah.rc" },
+        .{ .start_line = 4, .end_line = 4, .filename = "blah.rc" },
+        .{ .start_line = 5, .end_line = 5, .filename = "blah.rc" },
+    },
+        \\/*
+        \\#line 1 "irrelevant.rc"
+        \\
+        \\
+        \\*/
+    , .{ .initial_filename = "blah.rc" });
+}