Commit 2adb932ad6

Techatrix <19954306+Techatrix@users.noreply.github.com>
2023-09-25 04:54:07
translate-c: convert clang errors messages into `std.zig.ErrorBundle`
1 parent e7bf143
src/Compilation.zig
@@ -33,7 +33,6 @@ const InternPool = @import("InternPool.zig");
 const BuildId = std.Build.CompileStep.BuildId;
 const Cache = std.Build.Cache;
 const translate_c = @import("translate_c.zig");
-const clang = @import("clang.zig");
 const c_codegen = @import("codegen/c.zig");
 const libtsan = @import("libtsan.zig");
 const Zir = @import("Zir.zig");
@@ -2743,7 +2742,7 @@ pub fn totalErrorCount(self: *Compilation) u32 {
             if (module.declFileScope(key).okToReportErrors()) {
                 total += 1;
                 if (module.cimport_errors.get(key)) |errors| {
-                    total += errors.len;
+                    total += errors.errorMessageCount();
                 }
             }
         }
@@ -2867,20 +2866,26 @@ pub fn getAllErrorsAlloc(self: *Compilation) !ErrorBundle {
                 // We'll try again once parsing succeeds.
                 if (module.declFileScope(decl_index).okToReportErrors()) {
                     try addModuleErrorMsg(module, &bundle, entry.value_ptr.*.*);
-                    if (module.cimport_errors.get(entry.key_ptr.*)) |cimport_errors| for (cimport_errors) |c_error| {
-                        try bundle.addRootErrorMessage(.{
-                            .msg = try bundle.addString(std.mem.span(c_error.msg)),
-                            .src_loc = if (c_error.path) |some| try bundle.addSourceLocation(.{
-                                .src_path = try bundle.addString(std.mem.span(some)),
-                                .span_start = c_error.offset,
-                                .span_main = c_error.offset,
-                                .span_end = c_error.offset + 1,
-                                .line = c_error.line,
-                                .column = c_error.column,
-                                .source_line = if (c_error.source_line) |line| try bundle.addString(std.mem.span(line)) else 0,
-                            }) else .none,
-                        });
-                    };
+                    if (module.cimport_errors.get(entry.key_ptr.*)) |errors| {
+                        for (errors.getMessages()) |err_msg_index| {
+                            const err_msg = errors.getErrorMessage(err_msg_index);
+                            try bundle.addRootErrorMessage(.{
+                                .msg = try bundle.addString(errors.nullTerminatedString(err_msg.msg)),
+                                .src_loc = if (err_msg.src_loc != .none) blk: {
+                                    const src_loc = errors.getSourceLocation(err_msg.src_loc);
+                                    break :blk try bundle.addSourceLocation(.{
+                                        .src_path = try bundle.addString(errors.nullTerminatedString(src_loc.src_path)),
+                                        .span_start = src_loc.span_start,
+                                        .span_main = src_loc.span_main,
+                                        .span_end = src_loc.span_end,
+                                        .line = src_loc.line,
+                                        .column = src_loc.column,
+                                        .source_line = if (src_loc.source_line != 0) try bundle.addString(errors.nullTerminatedString(src_loc.source_line)) else 0,
+                                    });
+                                } else .none,
+                            });
+                        }
+                    }
                 }
             }
         }
@@ -3831,9 +3836,15 @@ test "cImport" {
     _ = cImport;
 }
 
-const CImportResult = struct {
+pub const CImportResult = struct {
     out_zig_path: []u8,
-    errors: []clang.ErrorMsg,
+    cache_hit: bool,
+    errors: std.zig.ErrorBundle,
+
+    pub fn deinit(result: *CImportResult, gpa: std.mem.Allocator) void {
+        gpa.free(result.out_zig_path);
+        result.errors.deinit(gpa);
+    }
 };
 
 /// Caller owns returned memory.
@@ -3906,25 +3917,22 @@ pub fn cImport(comp: *Compilation, c_src: []const u8) !CImportResult {
             new_argv[i] = try arena.dupeZ(u8, arg);
         }
 
-        const c_headers_dir_path = try comp.zig_lib_directory.join(arena, &[_][]const u8{"include"});
-        const c_headers_dir_path_z = try arena.dupeZ(u8, c_headers_dir_path);
-        var clang_errors: []clang.ErrorMsg = &[0]clang.ErrorMsg{};
+        const c_headers_dir_path_z = try comp.zig_lib_directory.joinZ(arena, &[_][]const u8{"include"});
+        var errors = std.zig.ErrorBundle.empty;
+        errdefer errors.deinit(comp.gpa);
         var tree = translate_c.translate(
             comp.gpa,
             new_argv.ptr,
             new_argv.ptr + new_argv.len,
-            &clang_errors,
+            &errors,
             c_headers_dir_path_z,
         ) catch |err| switch (err) {
             error.OutOfMemory => return error.OutOfMemory,
-            error.ASTUnitFailure => {
-                log.warn("clang API returned errors but due to a clang bug, it is not exposing the errors for zig to see. For more details: https://github.com/ziglang/zig/issues/4455", .{});
-                return error.ASTUnitFailure;
-            },
             error.SemanticAnalyzeFail => {
                 return CImportResult{
                     .out_zig_path = "",
-                    .errors = clang_errors,
+                    .cache_hit = actual_hit,
+                    .errors = errors,
                 };
             },
         };
@@ -3976,7 +3984,8 @@ pub fn cImport(comp: *Compilation, c_src: []const u8) !CImportResult {
     }
     return CImportResult{
         .out_zig_path = out_zig_path,
-        .errors = &[0]clang.ErrorMsg{},
+        .cache_hit = actual_hit,
+        .errors = std.zig.ErrorBundle.empty,
     };
 }
 
src/main.zig
@@ -21,7 +21,6 @@ const introspect = @import("introspect.zig");
 const LibCInstallation = @import("libc_installation.zig").LibCInstallation;
 const wasi_libc = @import("wasi_libc.zig");
 const translate_c = @import("translate_c.zig");
-const clang = @import("clang.zig");
 const BuildId = std.Build.CompileStep.BuildId;
 const Cache = std.Build.Cache;
 const target_util = @import("target.zig");
@@ -3697,11 +3696,16 @@ fn serve(
                     var arena_instance = std.heap.ArenaAllocator.init(gpa);
                     defer arena_instance.deinit();
                     const arena = arena_instance.allocator();
-                    var output: TranslateCOutput = undefined;
+                    var output: Compilation.CImportResult = undefined;
                     try cmdTranslateC(comp, arena, &output);
-                    try server.serveEmitBinPath(output.path, .{
-                        .flags = .{ .cache_hit = output.cache_hit },
-                    });
+                    defer output.deinit(gpa);
+                    if (output.errors.errorMessageCount() != 0) {
+                        try server.serveErrorBundle(output.errors);
+                    } else {
+                        try server.serveEmitBinPath(output.out_zig_path, .{
+                            .flags = .{ .cache_hit = output.cache_hit },
+                        });
+                    }
                     continue;
                 }
 
@@ -4168,12 +4172,7 @@ fn updateModule(comp: *Compilation) !void {
     }
 }
 
-const TranslateCOutput = struct {
-    path: []const u8,
-    cache_hit: bool,
-};
-
-fn cmdTranslateC(comp: *Compilation, arena: Allocator, fancy_output: ?*TranslateCOutput) !void {
+fn cmdTranslateC(comp: *Compilation, arena: Allocator, fancy_output: ?*Compilation.CImportResult) !void {
     if (!build_options.have_llvm)
         fatal("cannot translate-c: compiler built without LLVM extensions", .{});
 
@@ -4231,29 +4230,24 @@ fn cmdTranslateC(comp: *Compilation, arena: Allocator, fancy_output: ?*Translate
             new_argv[argv.items.len + i] = try arena.dupeZ(u8, arg);
         }
 
-        const c_headers_dir_path = try comp.zig_lib_directory.join(arena, &[_][]const u8{"include"});
-        const c_headers_dir_path_z = try arena.dupeZ(u8, c_headers_dir_path);
-        var clang_errors: []clang.ErrorMsg = &[0]clang.ErrorMsg{};
+        const c_headers_dir_path_z = try comp.zig_lib_directory.joinZ(arena, &[_][]const u8{"include"});
+        var errors = std.zig.ErrorBundle.empty;
         var tree = translate_c.translate(
             comp.gpa,
             new_argv.ptr,
             new_argv.ptr + new_argv.len,
-            &clang_errors,
+            &errors,
             c_headers_dir_path_z,
         ) catch |err| switch (err) {
             error.OutOfMemory => return error.OutOfMemory,
-            error.ASTUnitFailure => fatal("clang API returned errors but due to a clang bug, it is not exposing the errors for zig to see. For more details: https://github.com/ziglang/zig/issues/4455", .{}),
             error.SemanticAnalyzeFail => {
-                // TODO convert these to zig errors
-                for (clang_errors) |clang_err| {
-                    std.debug.print("{s}:{d}:{d}: {s}\n", .{
-                        if (clang_err.filename_ptr) |p| p[0..clang_err.filename_len] else "(no file)",
-                        clang_err.line + 1,
-                        clang_err.column + 1,
-                        clang_err.msg_ptr[0..clang_err.msg_len],
-                    });
+                if (fancy_output) |p| {
+                    p.errors = errors;
+                    return;
+                } else {
+                    errors.renderToStdErr(renderOptions(comp.color));
+                    process.exit(1);
                 }
-                process.exit(1);
             },
         };
         defer tree.deinit(comp.gpa);
@@ -4290,10 +4284,10 @@ fn cmdTranslateC(comp: *Compilation, arena: Allocator, fancy_output: ?*Translate
     };
 
     if (fancy_output) |p| {
-        const full_zig_path = try comp.local_cache_directory.join(arena, &[_][]const u8{
+        p.out_zig_path = try comp.local_cache_directory.join(comp.gpa, &[_][]const u8{
             "o", &digest, translated_zig_basename,
         });
-        p.path = full_zig_path;
+        p.errors = std.zig.ErrorBundle.empty;
     } else {
         const out_zig_path = try fs.path.join(arena, &[_][]const u8{ "o", &digest, translated_zig_basename });
         const zig_file = comp.local_cache_directory.handle.openFile(out_zig_path, .{}) catch |err| {
src/Module.zig
@@ -131,7 +131,7 @@ failed_embed_files: std.AutoArrayHashMapUnmanaged(*EmbedFile, *ErrorMsg) = .{},
 failed_exports: std.AutoArrayHashMapUnmanaged(*Export, *ErrorMsg) = .{},
 /// If a decl failed due to a cimport error, the corresponding Clang errors
 /// are stored here.
-cimport_errors: std.AutoArrayHashMapUnmanaged(Decl.Index, []CImportError) = .{},
+cimport_errors: std.AutoArrayHashMapUnmanaged(Decl.Index, std.zig.ErrorBundle) = .{},
 
 /// Candidates for deletion. After a semantic analysis update completes, this list
 /// contains Decls that need to be deleted if they end up having no references to them.
@@ -2603,8 +2603,8 @@ pub fn deinit(mod: *Module) void {
     }
     mod.failed_exports.deinit(gpa);
 
-    for (mod.cimport_errors.values()) |errs| {
-        for (errs) |err| err.deinit(gpa);
+    for (mod.cimport_errors.values()) |*errs| {
+        errs.deinit(gpa);
     }
     mod.cimport_errors.deinit(gpa);
 
@@ -4583,7 +4583,8 @@ pub fn clearDecl(
         kv.value.destroy(gpa);
     }
     if (mod.cimport_errors.fetchSwapRemove(decl_index)) |kv| {
-        for (kv.value) |err| err.deinit(gpa);
+        var errors = kv.value;
+        errors.deinit(gpa);
     }
     if (mod.emit_h) |emit_h| {
         if (emit_h.failed_decls.fetchSwapRemove(decl_index)) |kv| {
@@ -4965,7 +4966,8 @@ fn markOutdatedDecl(mod: *Module, decl_index: Decl.Index) !void {
         kv.value.destroy(mod.gpa);
     }
     if (mod.cimport_errors.fetchSwapRemove(decl_index)) |kv| {
-        for (kv.value) |err| err.deinit(mod.gpa);
+        var errors = kv.value;
+        errors.deinit(mod.gpa);
     }
     if (mod.emit_h) |emit_h| {
         if (emit_h.failed_decls.fetchSwapRemove(decl_index)) |kv| {
src/Sema.zig
@@ -5767,13 +5767,12 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr
     _ = try sema.analyzeBodyBreak(&child_block, body);
 
     const mod = sema.mod;
-    const c_import_res = mod.comp.cImport(c_import_buf.items) catch |err|
+    var c_import_res = mod.comp.cImport(c_import_buf.items) catch |err|
         return sema.fail(&child_block, src, "C import failed: {s}", .{@errorName(err)});
+    defer c_import_res.deinit(mod.comp.gpa);
 
-    if (c_import_res.errors.len != 0) {
+    if (c_import_res.errors.errorMessageCount() != 0) {
         const msg = msg: {
-            defer @import("clang.zig").ErrorMsg.delete(c_import_res.errors.ptr, c_import_res.errors.len);
-
             const msg = try sema.errMsg(&child_block, src, "C import failed", .{});
             errdefer msg.destroy(sema.gpa);
 
@@ -5782,47 +5781,8 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr
 
             const gop = try mod.cimport_errors.getOrPut(sema.gpa, sema.owner_decl_index);
             if (!gop.found_existing) {
-                var errs = try std.ArrayListUnmanaged(Module.CImportError).initCapacity(sema.gpa, c_import_res.errors.len);
-                errdefer {
-                    for (errs.items) |err| err.deinit(sema.gpa);
-                    errs.deinit(sema.gpa);
-                }
-
-                for (c_import_res.errors) |c_error| {
-                    const path = if (c_error.filename_ptr) |some|
-                        try sema.gpa.dupeZ(u8, some[0..c_error.filename_len])
-                    else
-                        null;
-                    errdefer if (path) |some| sema.gpa.free(some);
-
-                    const c_msg = try sema.gpa.dupeZ(u8, c_error.msg_ptr[0..c_error.msg_len]);
-                    errdefer sema.gpa.free(c_msg);
-
-                    const line = line: {
-                        const source = c_error.source orelse break :line null;
-                        var start = c_error.offset;
-                        while (start > 0) : (start -= 1) {
-                            if (source[start - 1] == '\n') break;
-                        }
-                        var end = c_error.offset;
-                        while (true) : (end += 1) {
-                            if (source[end] == 0) break;
-                            if (source[end] == '\n') break;
-                        }
-                        break :line try sema.gpa.dupeZ(u8, source[start..end]);
-                    };
-                    errdefer if (line) |some| sema.gpa.free(some);
-
-                    errs.appendAssumeCapacity(.{
-                        .path = path orelse null,
-                        .source_line = line orelse null,
-                        .line = c_error.line,
-                        .column = c_error.column,
-                        .offset = c_error.offset,
-                        .msg = c_msg,
-                    });
-                }
-                gop.value_ptr.* = errs.items;
+                gop.value_ptr.* = c_import_res.errors;
+                c_import_res.errors = std.zig.ErrorBundle.empty;
             }
             break :msg msg;
         };
src/translate_c.zig
@@ -372,19 +372,54 @@ pub fn translate(
     gpa: mem.Allocator,
     args_begin: [*]?[*]const u8,
     args_end: [*]?[*]const u8,
-    errors: *[]clang.ErrorMsg,
+    errors: *std.zig.ErrorBundle,
     resources_path: [*:0]const u8,
 ) !std.zig.Ast {
-    // TODO stage2 bug
-    var tmp = errors;
+    var clang_errors: []clang.ErrorMsg = &.{};
+
     const ast_unit = clang.LoadFromCommandLine(
         args_begin,
         args_end,
-        &tmp.ptr,
-        &tmp.len,
+        &clang_errors.ptr,
+        &clang_errors.len,
         resources_path,
     ) orelse {
-        if (errors.len == 0) return error.ASTUnitFailure;
+        defer clang.ErrorMsg.delete(clang_errors.ptr, clang_errors.len);
+
+        var bundle: std.zig.ErrorBundle.Wip = undefined;
+        try bundle.init(gpa);
+        defer bundle.deinit();
+
+        for (clang_errors) |c_error| {
+            const line = line: {
+                const source = c_error.source orelse break :line 0;
+                var start = c_error.offset;
+                while (start > 0) : (start -= 1) {
+                    if (source[start - 1] == '\n') break;
+                }
+                var end = c_error.offset;
+                while (true) : (end += 1) {
+                    if (source[end] == 0) break;
+                    if (source[end] == '\n') break;
+                }
+                break :line try bundle.addString(source[start..end]);
+            };
+
+            try bundle.addRootErrorMessage(.{
+                .msg = try bundle.addString(c_error.msg_ptr[0..c_error.msg_len]),
+                .src_loc = if (c_error.filename_ptr) |filename_ptr| try bundle.addSourceLocation(.{
+                    .src_path = try bundle.addString(filename_ptr[0..c_error.filename_len]),
+                    .span_start = c_error.offset,
+                    .span_main = c_error.offset,
+                    .span_end = c_error.offset + 1,
+                    .line = c_error.line,
+                    .column = c_error.column,
+                    .source_line = line,
+                }) else .none,
+            });
+        }
+        errors.* = try bundle.toOwnedBundle("");
+
         return error.SemanticAnalyzeFail;
     };
     defer ast_unit.delete();