compiler: improve error reporting

The functions `Compilation.create` and `Compilation.update` previously
returned inferred error sets, which had built up a lot of crap over
time. This meant that certain error conditions -- particularly certain
filesystem errors -- were not being reported properly (at best the CLI
would just print the error name). This was also a problem in
sub-compilations, where at times only the error name -- which might just
be something like `LinkFailed` -- would be visible.

This commit makes the error handling here more disciplined by
introducing concrete error sets to these functions (and a few more as a
consequence). These error sets are small: errors in `update` are almost
all reported via compile errors, and errors in `create` are reported
through a new `Compilation.CreateDiagnostic` type, a tagged union of
possible error cases. This allows for better error reporting.

Sub-compilations also report errors more correctly in several cases,
leading to more informative errors in the case of compiler bugs.

Also fixes some race conditions in library building by replacing calls
to `setMiscFailure` with calls to `lockAndSetMiscFailure`. Compilation
of libraries such as libc happens on the thread pool, so the logic must
synchronize its access to shared `Compilation` state.

Author: mlugg

lib/std/zig/LibCDirs.zig

@@ -19,7 +19,7 @@ pub fn detect(
     is_native_abi: bool,
     link_libc: bool,
     libc_installation: ?*const LibCInstallation,
-) !LibCDirs {
+) LibCInstallation.FindError!LibCDirs {
     if (!link_libc) {
         return .{
             .libc_include_dir_list = &[0][]u8{},
@@ -114,7 +114,7 @@ fn detectFromInstallation(arena: Allocator, target: *const std.Target, lci: *con
         }
     }
     if (target.os.tag == .haiku) {
-        const include_dir_path = lci.include_dir orelse return error.LibCInstallationNotAvailable;
+        const include_dir_path = lci.include_dir.?;
         const os_dir = try std.fs.path.join(arena, &[_][]const u8{ include_dir_path, "os" });
         list.appendAssumeCapacity(os_dir);
         // Errors.h

src/Compilation.zig

@@ -92,6 +92,20 @@ pub const ResolvedTarget = struct {
     llvm_cpu_features: ?[*:0]const u8 = null,
 };
 
+pub const CreateError = error{
+    OutOfMemory,
+    ValgrindUnsupportedOnTarget,
+    TargetRequiresSingleThreaded,
+    BackendRequiresSingleThreaded,
+    TargetRequiresPic,
+    PieRequiresPic,
+    DynamicLinkingRequiresPic,
+    TargetHasNoRedZone,
+    StackCheckUnsupportedByTarget,
+    StackProtectorUnsupportedByTarget,
+    StackProtectorUnavailableWithoutLibC,
+};
+
 /// At least one of `parent` and `resolved_target` must be non-null.
 pub fn create(arena: Allocator, options: CreateOptions) !*Package.Module {
     if (options.inherited.sanitize_thread == true) assert(options.global.any_sanitize_thread);

src/Package/Module.zig

@@ -5726,6 +5726,7 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr
             .global = comp.config,
             .parent = parent_mod,
         }) catch |err| switch (err) {
+            error.OutOfMemory => |e| return e,
             // None of these are possible because we are creating a package with
             // the exact same configuration as the parent package, which already
             // passed these checks.
@@ -5739,8 +5740,6 @@ fn zirCImport(sema: *Sema, parent_block: *Block, inst: Zir.Inst.Index) CompileEr
             error.StackCheckUnsupportedByTarget => unreachable,
             error.StackProtectorUnsupportedByTarget => unreachable,
             error.StackProtectorUnavailableWithoutLibC => unreachable,
-
-            else => |e| return e,
         };
         const c_import_file_path: Compilation.Path = try c_import_mod.root.join(gpa, comp.dirs, "cimport.zig");
         errdefer c_import_file_path.deinit(gpa);

src/Sema.zig

@@ -1038,7 +1038,9 @@ pub const File = struct {
         stat: Cache.File.Stat,
     };
 
-    pub fn getSource(file: *File, zcu: *const Zcu) !Source {
+    pub const GetSourceError = error{ OutOfMemory, FileTooBig } || std.fs.File.OpenError || std.fs.File.ReadError;
+
+    pub fn getSource(file: *File, zcu: *const Zcu) GetSourceError!Source {
         const gpa = zcu.gpa;
 
         if (file.source) |source| return .{
@@ -1062,7 +1064,7 @@ pub const File = struct {
 
         var file_reader = f.reader(&.{});
         file_reader.size = stat.size;
-        try file_reader.interface.readSliceAll(source);
+        file_reader.interface.readSliceAll(source) catch return file_reader.err.?;
 
         // Here we do not modify stat fields because this function is the one
         // used for error reporting. We need to keep the stat fields stale so that
@@ -1081,7 +1083,7 @@ pub const File = struct {
         };
     }
 
-    pub fn getTree(file: *File, zcu: *const Zcu) !*const Ast {
+    pub fn getTree(file: *File, zcu: *const Zcu) GetSourceError!*const Ast {
         if (file.tree) |*tree| return tree;
 
         const source = try file.getSource(zcu);
@@ -1127,7 +1129,7 @@ pub const File = struct {
         file: *File,
         zcu: *const Zcu,
         eb: *std.zig.ErrorBundle.Wip,
-    ) !std.zig.ErrorBundle.SourceLocationIndex {
+    ) Allocator.Error!std.zig.ErrorBundle.SourceLocationIndex {
         return eb.addSourceLocation(.{
             .src_path = try eb.printString("{f}", .{file.path.fmt(zcu.comp)}),
             .span_start = 0,
@@ -1138,17 +1140,17 @@ pub const File = struct {
             .source_line = 0,
         });
     }
+    /// Asserts that the tree has already been loaded with `getTree`.
     pub fn errorBundleTokenSrc(
         file: *File,
         tok: Ast.TokenIndex,
         zcu: *const Zcu,
         eb: *std.zig.ErrorBundle.Wip,
-    ) !std.zig.ErrorBundle.SourceLocationIndex {
-        const source = try file.getSource(zcu);
-        const tree = try file.getTree(zcu);
+    ) Allocator.Error!std.zig.ErrorBundle.SourceLocationIndex {
+        const tree = &file.tree.?;
         const start = tree.tokenStart(tok);
         const end = start + tree.tokenSlice(tok).len;
-        const loc = std.zig.findLineColumn(source.bytes, start);
+        const loc = std.zig.findLineColumn(file.source.?, start);
         return eb.addSourceLocation(.{
             .src_path = try eb.printString("{f}", .{file.path.fmt(zcu.comp)}),
             .span_start = start,
@@ -2665,8 +2667,9 @@ pub const LazySrcLoc = struct {
     }
 
     /// Used to sort error messages, so that they're printed in a consistent order.
-    /// If an error is returned, that error makes sorting impossible.
-    pub fn lessThan(lhs_lazy: LazySrcLoc, rhs_lazy: LazySrcLoc, zcu: *Zcu) !bool {
+    /// If an error is returned, a file could not be read in order to resolve a source location.
+    /// In that case, `bad_file_out` is populated, and sorting is impossible.
+    pub fn lessThan(lhs_lazy: LazySrcLoc, rhs_lazy: LazySrcLoc, zcu: *Zcu, bad_file_out: **Zcu.File) File.GetSourceError!bool {
         const lhs_src = lhs_lazy.upgradeOrLost(zcu) orelse {
             // LHS source location lost, so should never be referenced. Just sort it to the end.
             return false;
@@ -2684,8 +2687,14 @@ pub const LazySrcLoc = struct {
             return std.mem.order(u8, lhs_path.sub_path, rhs_path.sub_path).compare(.lt);
         }
 
-        const lhs_span = try lhs_src.span(zcu);
-        const rhs_span = try rhs_src.span(zcu);
+        const lhs_span = lhs_src.span(zcu) catch |err| {
+            bad_file_out.* = lhs_src.file_scope;
+            return err;
+        };
+        const rhs_span = rhs_src.span(zcu) catch |err| {
+            bad_file_out.* = rhs_src.file_scope;
+            return err;
+        };
         return lhs_span.main < rhs_span.main;
     }
 };
@@ -4584,7 +4593,7 @@ pub fn codegenFailTypeMsg(zcu: *Zcu, ty_index: InternPool.Index, msg: *ErrorMsg)
 pub fn addFileInMultipleModulesError(
     zcu: *Zcu,
     eb: *std.zig.ErrorBundle.Wip,
-) !void {
+) Allocator.Error!void {
     const gpa = zcu.gpa;
 
     const info = zcu.multi_module_err.?;
@@ -4631,7 +4640,7 @@ fn explainWhyFileIsInModule(
     file: File.Index,
     in_module: *Package.Module,
     ref: File.Reference,
-) !void {
+) Allocator.Error!void {
     const gpa = zcu.gpa;
 
     // error: file is the root of module 'foo'
@@ -4666,7 +4675,13 @@ fn explainWhyFileIsInModule(
         const thing: []const u8 = if (is_first) "file" else "which";
         is_first = false;
 
-        const import_src = try zcu.fileByIndex(import.importer).errorBundleTokenSrc(import.tok, zcu, eb);
+        const importer_file = zcu.fileByIndex(import.importer);
+        // `errorBundleTokenSrc` expects the tree to be loaded
+        _ = importer_file.getTree(zcu) catch |err| {
+            try Compilation.unableToLoadZcuFile(zcu, eb, importer_file, err);
+            return; // stop the explanation early
+        };
+        const import_src = try importer_file.errorBundleTokenSrc(import.tok, zcu, eb);
 
         const importer_ref = zcu.alive_files.get(import.importer).?;
         const importer_root: ?*Package.Module = switch (importer_ref) {

src/Zcu.zig

@@ -57,7 +57,7 @@ fn libcPath(comp: *Compilation, arena: Allocator, sub_path: []const u8) ![]const
 }
 
 /// TODO replace anyerror with explicit error set, recording user-friendly errors with
-/// setMiscFailure and returning error.SubCompilationFailed. see libcxx.zig for example.
+/// lockAndSetMiscFailure and returning error.AlreadyReported. see libcxx.zig for example.
 pub fn buildCrtFile(comp: *Compilation, crt_file: CrtFile, prog_node: std.Progress.Node) anyerror!void {
     if (!build_options.have_llvm) return error.ZigCompilerNotBuiltWithLLVMExtensions;
 
@@ -414,7 +414,7 @@ fn wordDirective(target: *const std.Target) []const u8 {
 }
 
 /// TODO replace anyerror with explicit error set, recording user-friendly errors with
-/// setMiscFailure and returning error.SubCompilationFailed. see libcxx.zig for example.
+/// lockAndSetMiscFailure and returning error.AlreadyReported. see libcxx.zig for example.
 pub fn buildSharedObjects(comp: *Compilation, prog_node: std.Progress.Node) anyerror!void {
     // See also glibc.zig which this code is based on.
 
@@ -1065,7 +1065,10 @@ fn buildSharedLib(
         },
     };
 
-    const sub_compilation = try Compilation.create(comp.gpa, arena, .{
+    const misc_task: Compilation.MiscTask = .@"freebsd libc shared object";
+
+    var sub_create_diag: Compilation.CreateDiagnostic = undefined;
+    const sub_compilation = Compilation.create(comp.gpa, arena, &sub_create_diag, .{
         .dirs = comp.dirs.withoutLocalCache(),
         .thread_pool = comp.thread_pool,
         .self_exe_path = comp.self_exe_path,
@@ -1090,8 +1093,14 @@ fn buildSharedLib(
         .soname = soname,
         .c_source_files = &c_source_files,
         .skip_linker_dependencies = true,
-    });
+    }) catch |err| switch (err) {
+        error.CreateFail => {
+            comp.lockAndSetMiscFailure(misc_task, "sub-compilation of {t} failed: {f}", .{ misc_task, sub_create_diag });
+            return error.AlreadyReported;
+        },
+        else => |e| return e,
+    };
     defer sub_compilation.destroy();
 
-    try comp.updateSubCompilation(sub_compilation, .@"freebsd libc shared object", prog_node);
+    try comp.updateSubCompilation(sub_compilation, misc_task, prog_node);
 }

src/libs/freebsd.zig

@@ -162,7 +162,7 @@ pub const CrtFile = enum {
 };
 
 /// TODO replace anyerror with explicit error set, recording user-friendly errors with
-/// setMiscFailure and returning error.SubCompilationFailed. see libcxx.zig for example.
+/// lockAndSetMiscFailure and returning error.AlreadyReported. see libcxx.zig for example.
 pub fn buildCrtFile(comp: *Compilation, crt_file: CrtFile, prog_node: std.Progress.Node) anyerror!void {
     if (!build_options.have_llvm) {
         return error.ZigCompilerNotBuiltWithLLVMExtensions;
@@ -656,7 +656,7 @@ fn wordDirective(target: *const std.Target) []const u8 {
 }
 
 /// TODO replace anyerror with explicit error set, recording user-friendly errors with
-/// setMiscFailure and returning error.SubCompilationFailed. see libcxx.zig for example.
+/// lockAndSetMiscFailure and returning error.AlreadyReported. see libcxx.zig for example.
 pub fn buildSharedObjects(comp: *Compilation, prog_node: std.Progress.Node) anyerror!void {
     const tracy = trace(@src());
     defer tracy.end();
@@ -1223,7 +1223,10 @@ fn buildSharedLib(
         },
     };
 
-    const sub_compilation = try Compilation.create(comp.gpa, arena, .{
+    const misc_task: Compilation.MiscTask = .@"glibc shared object";
+
+    var sub_create_diag: Compilation.CreateDiagnostic = undefined;
+    const sub_compilation = Compilation.create(comp.gpa, arena, &sub_create_diag, .{
         .dirs = comp.dirs.withoutLocalCache(),
         .thread_pool = comp.thread_pool,
         .self_exe_path = comp.self_exe_path,
@@ -1248,10 +1251,16 @@ fn buildSharedLib(
         .soname = soname,
         .c_source_files = &c_source_files,
         .skip_linker_dependencies = true,
-    });
+    }) catch |err| switch (err) {
+        error.CreateFail => {
+            comp.lockAndSetMiscFailure(misc_task, "sub-compilation of {t} failed: {f}", .{ misc_task, sub_create_diag });
+            return error.AlreadyReported;
+        },
+        else => |e| return e,
+    };
     defer sub_compilation.destroy();
 
-    try comp.updateSubCompilation(sub_compilation, .@"glibc shared object", prog_node);
+    try comp.updateSubCompilation(sub_compilation, misc_task, prog_node);
 }
 
 pub fn needsCrt0(output_mode: std.builtin.OutputMode) ?CrtFile {