From 55ba335e0ffc2af76bf0743d98f5a959ccce0409 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 11 Mar 2022 19:31:29 -0700 Subject: [PATCH] Sema: fix resolution of inferred error sets Introduce `Module.ensureFuncBodyAnalyzed` and corresponding `Sema` function. This mirrors `ensureDeclAnalyzed` except also waits until the function body has been semantically analyzed, meaning that inferred error sets will have been populated. Resolving error sets can now emit a "unable to resolve inferred error set" error instead of producing an incorrect error set type. Resolving error sets now calls `ensureFuncBodyAnalyzed`. Closes #11046. `coerceInMemoryAllowedErrorSets` now does a lot more work to avoid resolving an inferred error set if possible. Same with `wrapErrorUnionSet`. Inferred error set types no longer check the `func` field to determine if they are equal. That was incorrect because an inline or comptime function call produces a unique error set which has the same `*Module.Fn` value for this field. Instead we use the `*Module.Fn.InferredErrorSet` pointers to test equality of inferred error sets. --- src/Compilation.zig | 134 ++++++--------------------- src/Module.zig | 121 +++++++++++++++++++++---- src/Sema.zig | 170 +++++++++++++++++++++-------------- src/type.zig | 10 +-- test/behavior.zig | 1 + test/behavior/array.zig | 3 - test/behavior/bugs/11046.zig | 21 +++++ test/behavior/error.zig | 6 +- 8 files changed, 271 insertions(+), 195 deletions(-) create mode 100644 test/behavior/bugs/11046.zig diff --git a/src/Compilation.zig b/src/Compilation.zig index fa2e4ca68b..5e02d8dba2 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -15,7 +15,6 @@ const Package = @import("Package.zig"); const link = @import("link.zig"); const tracy = @import("tracy.zig"); const trace = tracy.trace; -const Liveness = @import("Liveness.zig"); const build_options = @import("build_options"); const LibCInstallation = @import("libc_installation.zig").LibCInstallation; const glibc = @import("glibc.zig"); @@ -2702,12 +2701,12 @@ fn processOneJob(comp: *Compilation, job: Job, main_progress_node: *std.Progress => return, .complete, .codegen_failure_retryable => { - const named_frame = tracy.namedFrame("codegen_decl"); - defer named_frame.end(); - if (build_options.omit_stage2) @panic("sadly stage2 is omitted from this build to save memory on the CI server"); + const named_frame = tracy.namedFrame("codegen_decl"); + defer named_frame.end(); + const module = comp.bin_file.options.module.?; assert(decl.has_tv); @@ -2722,100 +2721,18 @@ fn processOneJob(comp: *Compilation, job: Job, main_progress_node: *std.Progress return; }, }, - .codegen_func => |func| switch (func.owner_decl.analysis) { - .unreferenced => unreachable, - .in_progress => unreachable, - .outdated => unreachable, + .codegen_func => |func| { + if (build_options.omit_stage2) + @panic("sadly stage2 is omitted from this build to save memory on the CI server"); - .file_failure, - .sema_failure, - .codegen_failure, - .dependency_failure, - .sema_failure_retryable, - => return, + const named_frame = tracy.namedFrame("codegen_func"); + defer named_frame.end(); - .complete, .codegen_failure_retryable => { - if (build_options.omit_stage2) - @panic("sadly stage2 is omitted from this build to save memory on the CI server"); - switch (func.state) { - .sema_failure, .dependency_failure => return, - .queued => {}, - .in_progress => unreachable, - .inline_only => unreachable, // don't queue work for this - .success => unreachable, // don't queue it twice - } - - const gpa = comp.gpa; - const module = comp.bin_file.options.module.?; - const decl = func.owner_decl; - - var tmp_arena = std.heap.ArenaAllocator.init(gpa); - defer tmp_arena.deinit(); - const sema_arena = tmp_arena.allocator(); - - const sema_frame = tracy.namedFrame("sema"); - var sema_frame_ended = false; - errdefer if (!sema_frame_ended) sema_frame.end(); - - var air = module.analyzeFnBody(decl, func, sema_arena) catch |err| switch (err) { - error.AnalysisFail => { - if (func.state == .in_progress) { - // If this decl caused the compile error, the analysis field would - // be changed to indicate it was this Decl's fault. Because this - // did not happen, we infer here that it was a dependency failure. - func.state = .dependency_failure; - } - return; - }, - error.OutOfMemory => return error.OutOfMemory, - }; - defer air.deinit(gpa); - - sema_frame.end(); - sema_frame_ended = true; - - if (comp.bin_file.options.emit == null) return; - - const liveness_frame = tracy.namedFrame("liveness"); - var liveness_frame_ended = false; - errdefer if (!liveness_frame_ended) liveness_frame.end(); - - log.debug("analyze liveness of {s}", .{decl.name}); - var liveness = try Liveness.analyze(gpa, air); - defer liveness.deinit(gpa); - - liveness_frame.end(); - liveness_frame_ended = true; - - if (builtin.mode == .Debug and comp.verbose_air) { - std.debug.print("# Begin Function AIR: {s}:\n", .{decl.name}); - @import("print_air.zig").dump(gpa, air, liveness); - std.debug.print("# End Function AIR: {s}\n\n", .{decl.name}); - } - - const named_frame = tracy.namedFrame("codegen"); - defer named_frame.end(); - - comp.bin_file.updateFunc(module, func, air, liveness) catch |err| switch (err) { - error.OutOfMemory => return error.OutOfMemory, - error.AnalysisFail => { - decl.analysis = .codegen_failure; - return; - }, - else => { - try module.failed_decls.ensureUnusedCapacity(gpa, 1); - module.failed_decls.putAssumeCapacityNoClobber(decl, try Module.ErrorMsg.create( - gpa, - decl.srcLoc(), - "unable to codegen: {s}", - .{@errorName(err)}, - )); - decl.analysis = .codegen_failure_retryable; - return; - }, - }; - return; - }, + const module = comp.bin_file.options.module.?; + module.ensureFuncBodyAnalyzed(func) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.AnalysisFail => return, + }; }, .emit_h_decl => |decl| switch (decl.analysis) { .unreferenced => unreachable, @@ -2831,11 +2748,12 @@ fn processOneJob(comp: *Compilation, job: Job, main_progress_node: *std.Progress // emit-h only requires semantic analysis of the Decl to be complete, // it does not depend on machine code generation to succeed. .codegen_failure, .codegen_failure_retryable, .complete => { + if (build_options.omit_stage2) + @panic("sadly stage2 is omitted from this build to save memory on the CI server"); + const named_frame = tracy.namedFrame("emit_h_decl"); defer named_frame.end(); - if (build_options.omit_stage2) - @panic("sadly stage2 is omitted from this build to save memory on the CI server"); const gpa = comp.gpa; const module = comp.bin_file.options.module.?; const emit_h = module.emit_h.?; @@ -2871,11 +2789,12 @@ fn processOneJob(comp: *Compilation, job: Job, main_progress_node: *std.Progress }, }, .analyze_decl => |decl| { + if (build_options.omit_stage2) + @panic("sadly stage2 is omitted from this build to save memory on the CI server"); + const named_frame = tracy.namedFrame("analyze_decl"); defer named_frame.end(); - if (build_options.omit_stage2) - @panic("sadly stage2 is omitted from this build to save memory on the CI server"); const module = comp.bin_file.options.module.?; module.ensureDeclAnalyzed(decl) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, @@ -2883,11 +2802,12 @@ fn processOneJob(comp: *Compilation, job: Job, main_progress_node: *std.Progress }; }, .update_embed_file => |embed_file| { + if (build_options.omit_stage2) + @panic("sadly stage2 is omitted from this build to save memory on the CI server"); + const named_frame = tracy.namedFrame("update_embed_file"); defer named_frame.end(); - if (build_options.omit_stage2) - @panic("sadly stage2 is omitted from this build to save memory on the CI server"); const module = comp.bin_file.options.module.?; module.updateEmbedFile(embed_file) catch |err| switch (err) { error.OutOfMemory => return error.OutOfMemory, @@ -2895,11 +2815,12 @@ fn processOneJob(comp: *Compilation, job: Job, main_progress_node: *std.Progress }; }, .update_line_number => |decl| { + if (build_options.omit_stage2) + @panic("sadly stage2 is omitted from this build to save memory on the CI server"); + const named_frame = tracy.namedFrame("update_line_number"); defer named_frame.end(); - if (build_options.omit_stage2) - @panic("sadly stage2 is omitted from this build to save memory on the CI server"); const gpa = comp.gpa; const module = comp.bin_file.options.module.?; comp.bin_file.updateDeclLineNumber(module, decl) catch |err| { @@ -2914,11 +2835,12 @@ fn processOneJob(comp: *Compilation, job: Job, main_progress_node: *std.Progress }; }, .analyze_pkg => |pkg| { + if (build_options.omit_stage2) + @panic("sadly stage2 is omitted from this build to save memory on the CI server"); + const named_frame = tracy.namedFrame("analyze_pkg"); defer named_frame.end(); - if (build_options.omit_stage2) - @panic("sadly stage2 is omitted from this build to save memory on the CI server"); const module = comp.bin_file.options.module.?; module.semaPkg(pkg) catch |err| switch (err) { error.CurrentWorkingDirectoryUnlinked, diff --git a/src/Module.zig b/src/Module.zig index 693cc3b5a0..b3e0344d77 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -3,6 +3,7 @@ //! there is or is not any zig source code, respectively. const std = @import("std"); +const builtin = @import("builtin"); const mem = std.mem; const Allocator = std.mem.Allocator; const ArrayListUnmanaged = std.ArrayListUnmanaged; @@ -28,6 +29,7 @@ const AstGen = @import("AstGen.zig"); const Sema = @import("Sema.zig"); const target_util = @import("target.zig"); const build_options = @import("build_options"); +const Liveness = @import("Liveness.zig"); /// General-purpose allocator. Used for both temporary and long-term storage. gpa: Allocator, @@ -1438,8 +1440,11 @@ pub const Fn = struct { is_cold: bool = false, is_noinline: bool = false, - /// Any inferred error sets that this function owns, both it's own inferred error set and - /// inferred error sets of any inline/comptime functions called. + /// Any inferred error sets that this function owns, both its own inferred error set and + /// inferred error sets of any inline/comptime functions called. Not to be confused + /// with inferred error sets of generic instantiations of this function, which are + /// *not* tracked here - they are tracked in the new `Fn` object created for the + /// instantiations. inferred_error_sets: InferredErrorSetList = .{}, pub const Analysis = enum { @@ -1457,28 +1462,29 @@ pub const Fn = struct { }; /// This struct is used to keep track of any dependencies related to functions instances - /// that return inferred error sets. Note that a function may be associated to multiple different error sets, - /// for example an inferred error set which this function returns, but also any inferred error sets - /// of called inline or comptime functions. + /// that return inferred error sets. Note that a function may be associated to + /// multiple different error sets, for example an inferred error set which + /// this function returns, but also any inferred error sets of called inline + /// or comptime functions. pub const InferredErrorSet = struct { /// The function from which this error set originates. - /// Note: may be the function itself. func: *Fn, - /// All currently known errors that this error set contains. This includes direct additions - /// via `return error.Foo;`, and possibly also errors that are returned from any dependent functions. - /// When the inferred error set is fully resolved, this map contains all the errors that the function might return. + /// All currently known errors that this error set contains. This includes + /// direct additions via `return error.Foo;`, and possibly also errors that + /// are returned from any dependent functions. When the inferred error set is + /// fully resolved, this map contains all the errors that the function might return. errors: ErrorSet.NameMap = .{}, /// Other inferred error sets which this inferred error set should include. inferred_error_sets: std.AutoHashMapUnmanaged(*InferredErrorSet, void) = .{}, - /// Whether the function returned anyerror. This is true if either of the dependent functions - /// returns anyerror. + /// Whether the function returned anyerror. This is true if either of + /// the dependent functions returns anyerror. is_anyerror: bool = false, - /// Whether this error set is already fully resolved. If true, resolving can skip resolving any dependents - /// of this inferred error set. + /// Whether this error set is already fully resolved. If true, resolving + /// can skip resolving any dependents of this inferred error set. is_resolved: bool = false, pub fn addErrorSet(self: *InferredErrorSet, gpa: Allocator, err_set_ty: Type) !void { @@ -1494,8 +1500,8 @@ pub const Fn = struct { try self.errors.put(gpa, name, {}); }, .error_set_inferred => { - const set = err_set_ty.castTag(.error_set_inferred).?.data; - try self.inferred_error_sets.put(gpa, set, {}); + const ies = err_set_ty.castTag(.error_set_inferred).?.data; + try self.inferred_error_sets.put(gpa, ies, {}); }, .error_set_merged => { const names = err_set_ty.castTag(.error_set_merged).?.data.keys(); @@ -3441,6 +3447,10 @@ pub fn mapOldZirToNew( } } +/// This ensures that the Decl will have a Type and Value populated. +/// However the resolution status of the Type may not be fully resolved. +/// For example an inferred error set is not resolved until after `analyzeFnBody`. +/// is called. pub fn ensureDeclAnalyzed(mod: *Module, decl: *Decl) SemaError!void { const tracy = trace(@src()); defer tracy.end(); @@ -3533,6 +3543,87 @@ pub fn ensureDeclAnalyzed(mod: *Module, decl: *Decl) SemaError!void { } } +pub fn ensureFuncBodyAnalyzed(mod: *Module, func: *Fn) SemaError!void { + const tracy = trace(@src()); + defer tracy.end(); + + switch (func.owner_decl.analysis) { + .unreferenced => unreachable, + .in_progress => unreachable, + .outdated => unreachable, + + .file_failure, + .sema_failure, + .codegen_failure, + .dependency_failure, + .sema_failure_retryable, + => return error.AnalysisFail, + + .complete, .codegen_failure_retryable => { + switch (func.state) { + .sema_failure, .dependency_failure => return error.AnalysisFail, + .queued => {}, + .in_progress => unreachable, + .inline_only => unreachable, // don't queue work for this + .success => return, + } + + const gpa = mod.gpa; + const decl = func.owner_decl; + + var tmp_arena = std.heap.ArenaAllocator.init(gpa); + defer tmp_arena.deinit(); + const sema_arena = tmp_arena.allocator(); + + var air = mod.analyzeFnBody(decl, func, sema_arena) catch |err| switch (err) { + error.AnalysisFail => { + if (func.state == .in_progress) { + // If this decl caused the compile error, the analysis field would + // be changed to indicate it was this Decl's fault. Because this + // did not happen, we infer here that it was a dependency failure. + func.state = .dependency_failure; + } + return error.AnalysisFail; + }, + error.OutOfMemory => return error.OutOfMemory, + }; + defer air.deinit(gpa); + + if (mod.comp.bin_file.options.emit == null) return; + + log.debug("analyze liveness of {s}", .{decl.name}); + var liveness = try Liveness.analyze(gpa, air); + defer liveness.deinit(gpa); + + if (builtin.mode == .Debug and mod.comp.verbose_air) { + std.debug.print("# Begin Function AIR: {s}:\n", .{decl.name}); + @import("print_air.zig").dump(gpa, air, liveness); + std.debug.print("# End Function AIR: {s}\n\n", .{decl.name}); + } + + mod.comp.bin_file.updateFunc(mod, func, air, liveness) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.AnalysisFail => { + decl.analysis = .codegen_failure; + return; + }, + else => { + try mod.failed_decls.ensureUnusedCapacity(gpa, 1); + mod.failed_decls.putAssumeCapacityNoClobber(decl, try Module.ErrorMsg.create( + gpa, + decl.srcLoc(), + "unable to codegen: {s}", + .{@errorName(err)}, + )); + decl.analysis = .codegen_failure_retryable; + return; + }, + }; + return; + }, + } +} + pub fn updateEmbedFile(mod: *Module, embed_file: *EmbedFile) SemaError!void { const tracy = trace(@src()); defer tracy.end(); diff --git a/src/Sema.zig b/src/Sema.zig index 44b2f33d56..31295707de 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -5346,14 +5346,14 @@ fn zirMergeErrorSets(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileEr } if (lhs_ty.castTag(.error_set_inferred)) |payload| { - try sema.resolveInferredErrorSet(payload.data); + try sema.resolveInferredErrorSet(block, src, payload.data); // isAnyError might have changed from a false negative to a true positive after resolution. if (lhs_ty.isAnyError()) { return Air.Inst.Ref.anyerror_type; } } if (rhs_ty.castTag(.error_set_inferred)) |payload| { - try sema.resolveInferredErrorSet(payload.data); + try sema.resolveInferredErrorSet(block, src, payload.data); // isAnyError might have changed from a false negative to a true positive after resolution. if (rhs_ty.isAnyError()) { return Air.Inst.Ref.anyerror_type; @@ -6927,7 +6927,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError } } - try sema.resolveInferredErrorSetTy(operand_ty); + try sema.resolveInferredErrorSetTy(block, src, operand_ty); if (operand_ty.isAnyError()) { if (special_prong != .@"else") { @@ -10437,7 +10437,7 @@ fn zirTypeInfo(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai }; // If the error set is inferred it has to be resolved at this point - try sema.resolveInferredErrorSetTy(ty); + try sema.resolveInferredErrorSetTy(block, src, ty); // Build our list of Error values // Optional value is only null if anyerror @@ -12627,7 +12627,7 @@ fn zirErrSetCast(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError! try sema.checkErrorSetType(block, operand_src, operand_ty); if (try sema.resolveDefinedValue(block, operand_src, operand)) |val| { - try sema.resolveInferredErrorSetTy(dest_ty); + try sema.resolveInferredErrorSetTy(block, src, dest_ty); if (!dest_ty.isAnyError()) { const error_name = val.castTag(.@"error").?.data.name; @@ -16616,7 +16616,7 @@ fn coerceInMemoryAllowed( // Error Sets if (dest_tag == .ErrorSet and src_tag == .ErrorSet) { - return try sema.coerceInMemoryAllowedErrorSets(dest_ty, src_ty); + return try sema.coerceInMemoryAllowedErrorSets(block, dest_ty, src_ty, dest_src, src_src); } // Arrays @@ -16646,8 +16646,11 @@ fn coerceInMemoryAllowed( fn coerceInMemoryAllowedErrorSets( sema: *Sema, + block: *Block, dest_ty: Type, src_ty: Type, + dest_src: LazySrcLoc, + src_src: LazySrcLoc, ) !InMemoryCoercionResult { // Coercion to `anyerror`. Note that this check can return false negatives // in case the error sets did not get resolved. @@ -16655,24 +16658,43 @@ fn coerceInMemoryAllowedErrorSets( return .ok; } - // If both are inferred error sets of functions, and - // the dest includes the source function, the coercion is OK. - // This check is important because it works without forcing a full resolution - // of inferred error sets. - if (src_ty.castTag(.error_set_inferred)) |src_payload| { - if (dest_ty.castTag(.error_set_inferred)) |dst_payload| { - const src_func = src_payload.data.func; - const dst_func = dst_payload.data.func; + if (dest_ty.castTag(.error_set_inferred)) |dst_payload| { + const dst_ies = dst_payload.data; + // We will make an effort to return `ok` without resolving either error set, to + // avoid unnecessary "unable to resolve error set" dependency loop errors. + switch (src_ty.tag()) { + .error_set_inferred => { + // If both are inferred error sets of functions, and + // the dest includes the source function, the coercion is OK. + // This check is important because it works without forcing a full resolution + // of inferred error sets. + const src_ies = src_ty.castTag(.error_set_inferred).?.data; - if (src_func == dst_func or dst_payload.data.inferred_error_sets.contains(src_payload.data)) { - return .ok; - } - return .no_match; + if (dst_ies.inferred_error_sets.contains(src_ies)) { + return .ok; + } + }, + .error_set_single => { + const name = src_ty.castTag(.error_set_single).?.data; + if (dst_ies.errors.contains(name)) return .ok; + }, + .error_set_merged => { + const names = src_ty.castTag(.error_set_merged).?.data.keys(); + for (names) |name| { + if (!dst_ies.errors.contains(name)) break; + } else return .ok; + }, + .error_set => { + const names = src_ty.castTag(.error_set).?.data.names.keys(); + for (names) |name| { + if (!dst_ies.errors.contains(name)) break; + } else return .ok; + }, + .anyerror => {}, + else => unreachable, } - } - if (dest_ty.castTag(.error_set_inferred)) |payload| { - try sema.resolveInferredErrorSet(payload.data); + try sema.resolveInferredErrorSet(block, dest_src, dst_payload.data); // isAnyError might have changed from a false negative to a true positive after resolution. if (dest_ty.isAnyError()) { return .ok; @@ -16683,7 +16705,7 @@ fn coerceInMemoryAllowedErrorSets( .error_set_inferred => { const src_data = src_ty.castTag(.error_set_inferred).?.data; - try sema.resolveInferredErrorSet(src_data); + try sema.resolveInferredErrorSet(block, src_src, src_data); // src anyerror status might have changed after the resolution. if (src_ty.isAnyError()) { // dest_ty.isAnyError() == true is already checked for at this point. @@ -17969,6 +17991,17 @@ fn ensureDeclAnalyzed(sema: *Sema, decl: *Decl) CompileError!void { }; } +fn ensureFuncBodyAnalyzed(sema: *Sema, func: *Module.Fn) CompileError!void { + sema.mod.ensureFuncBodyAnalyzed(func) catch |err| { + if (sema.owner_func) |owner_func| { + owner_func.state = .dependency_failure; + } else { + sema.owner_decl.analysis = .dependency_failure; + } + return err; + }; +} + fn refValue(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type, val: Value) !Value { var anon_decl = try block.startAnonDecl(src); defer anon_decl.deinit(); @@ -18635,10 +18668,16 @@ fn wrapErrorUnionSet( }, .error_set_inferred => ok: { const expected_name = val.castTag(.@"error").?.data.name; - const data = dest_err_set_ty.castTag(.error_set_inferred).?.data; - try sema.resolveInferredErrorSet(data); - if (data.is_anyerror) break :ok; - if (data.errors.contains(expected_name)) break :ok; + const ies = dest_err_set_ty.castTag(.error_set_inferred).?.data; + + // We carefully do this in an order that avoids unnecessarily + // resolving the destination error set type. + if (ies.is_anyerror) break :ok; + if (ies.errors.contains(expected_name)) break :ok; + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, dest_err_set_ty, inst_ty, inst_src, inst_src)) { + break :ok; + } + return sema.failWithErrorSetCodeMissing(block, inst_src, dest_err_set_ty, inst_ty); }, .error_set_merged => { @@ -18794,10 +18833,10 @@ fn resolvePeerTypes( // If neither is a superset, merge errors. const chosen_set_ty = err_set_ty orelse chosen_ty; - if (.ok == try sema.coerceInMemoryAllowedErrorSets(chosen_set_ty, candidate_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, chosen_set_ty, candidate_ty, src, src)) { continue; } - if (.ok == try sema.coerceInMemoryAllowedErrorSets(candidate_ty, chosen_set_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, candidate_ty, chosen_set_ty, src, src)) { err_set_ty = null; chosen = candidate; chosen_i = candidate_i + 1; @@ -18810,10 +18849,10 @@ fn resolvePeerTypes( .ErrorUnion => { const chosen_set_ty = err_set_ty orelse chosen_ty.errorUnionSet(); - if (.ok == try sema.coerceInMemoryAllowedErrorSets(chosen_set_ty, candidate_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, chosen_set_ty, candidate_ty, src, src)) { continue; } - if (.ok == try sema.coerceInMemoryAllowedErrorSets(candidate_ty, chosen_set_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, candidate_ty, chosen_set_ty, src, src)) { err_set_ty = candidate_ty; continue; } @@ -18823,10 +18862,10 @@ fn resolvePeerTypes( }, else => { if (err_set_ty) |chosen_set_ty| { - if (.ok == try sema.coerceInMemoryAllowedErrorSets(chosen_set_ty, candidate_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, chosen_set_ty, candidate_ty, src, src)) { continue; } - if (.ok == try sema.coerceInMemoryAllowedErrorSets(candidate_ty, chosen_set_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, candidate_ty, chosen_set_ty, src, src)) { err_set_ty = candidate_ty; continue; } @@ -18844,9 +18883,9 @@ fn resolvePeerTypes( const chosen_set_ty = err_set_ty orelse chosen_ty; const candidate_set_ty = candidate_ty.errorUnionSet(); - if (.ok == try sema.coerceInMemoryAllowedErrorSets(chosen_set_ty, candidate_set_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, chosen_set_ty, candidate_set_ty, src, src)) { err_set_ty = chosen_set_ty; - } else if (.ok == try sema.coerceInMemoryAllowedErrorSets(candidate_set_ty, chosen_set_ty)) { + } else if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, candidate_set_ty, chosen_set_ty, src, src)) { err_set_ty = null; } else { err_set_ty = try chosen_set_ty.errorSetMerge(sema.arena, candidate_set_ty); @@ -18875,9 +18914,9 @@ fn resolvePeerTypes( const chosen_set_ty = err_set_ty orelse chosen_ty.errorUnionSet(); const candidate_set_ty = chosen_ty.errorUnionSet(); - if (.ok == try sema.coerceInMemoryAllowedErrorSets(chosen_set_ty, candidate_set_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, chosen_set_ty, candidate_set_ty, src, src)) { err_set_ty = chosen_set_ty; - } else if (.ok == try sema.coerceInMemoryAllowedErrorSets(candidate_set_ty, chosen_set_ty)) { + } else if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, candidate_set_ty, chosen_set_ty, src, src)) { err_set_ty = candidate_set_ty; } else { err_set_ty = try chosen_set_ty.errorSetMerge(sema.arena, candidate_set_ty); @@ -18889,9 +18928,9 @@ fn resolvePeerTypes( else => { if (err_set_ty) |chosen_set_ty| { const candidate_set_ty = candidate_ty.errorUnionSet(); - if (.ok == try sema.coerceInMemoryAllowedErrorSets(chosen_set_ty, candidate_set_ty)) { + if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, chosen_set_ty, candidate_set_ty, src, src)) { err_set_ty = chosen_set_ty; - } else if (.ok == try sema.coerceInMemoryAllowedErrorSets(candidate_set_ty, chosen_set_ty)) { + } else if (.ok == try sema.coerceInMemoryAllowedErrorSets(block, candidate_set_ty, chosen_set_ty, src, src)) { err_set_ty = null; } else { err_set_ty = try chosen_set_ty.errorSetMerge(sema.arena, candidate_set_ty); @@ -19458,43 +19497,44 @@ fn resolveBuiltinTypeFields( return sema.resolveTypeFields(block, src, resolved_ty); } -fn resolveInferredErrorSet(sema: *Sema, inferred_error_set: *Module.Fn.InferredErrorSet) CompileError!void { - // Ensuring that a particular decl is analyzed does not neccesarily mean that - // it's error set is inferred, so traverse all of them to get the complete - // picture. - // Note: We want to skip re-resolving the current function, as recursion - // doesn't change the error set. We can just check for state == .in_progress for this. - // TODO: Is that correct? +fn resolveInferredErrorSet( + sema: *Sema, + block: *Block, + src: LazySrcLoc, + ies: *Module.Fn.InferredErrorSet, +) CompileError!void { + if (ies.is_resolved) return; - if (inferred_error_set.is_resolved) { - return; + if (ies.func.state == .in_progress) { + return sema.fail(block, src, "unable to resolve inferred error set", .{}); } - inferred_error_set.is_resolved = true; - var it = inferred_error_set.inferred_error_sets.keyIterator(); + // To ensure that all dependencies are properly added to the set. + try sema.ensureFuncBodyAnalyzed(ies.func); + + ies.is_resolved = true; + + var it = ies.inferred_error_sets.keyIterator(); while (it.next()) |other_error_set_ptr| { - const func = other_error_set_ptr.*.func; - const decl = func.*.owner_decl; + const other_ies: *Module.Fn.InferredErrorSet = other_error_set_ptr.*; + try sema.resolveInferredErrorSet(block, src, other_ies); - if (func.*.state == .in_progress) { - // Recursion, doesn't alter current error set, keep going. - continue; + for (other_ies.errors.keys()) |key| { + try ies.errors.put(sema.gpa, key, {}); } - - try sema.ensureDeclAnalyzed(decl); // To ensure that all dependencies are properly added to the set. - try sema.resolveInferredErrorSet(other_error_set_ptr.*); - - for (other_error_set_ptr.*.errors.keys()) |key| { - try inferred_error_set.errors.put(sema.gpa, key, {}); - } - if (other_error_set_ptr.*.is_anyerror) - inferred_error_set.is_anyerror = true; + if (other_ies.is_anyerror) + ies.is_anyerror = true; } } -fn resolveInferredErrorSetTy(sema: *Sema, ty: Type) CompileError!void { +fn resolveInferredErrorSetTy( + sema: *Sema, + block: *Block, + src: LazySrcLoc, + ty: Type, +) CompileError!void { if (ty.castTag(.error_set_inferred)) |inferred| { - try sema.resolveInferredErrorSet(inferred.data); + try sema.resolveInferredErrorSet(block, src, inferred.data); } } diff --git a/src/type.zig b/src/type.zig index ae9ca31e9f..04bd015bcc 100644 --- a/src/type.zig +++ b/src/type.zig @@ -559,9 +559,9 @@ pub const Type = extern union { .error_set_inferred => { // Inferred error sets are only equal if both are inferred // and they originate from the exact same function. - const a_set = a.castTag(.error_set_inferred).?.data; - const b_set = (b.castTag(.error_set_inferred) orelse return false).data; - return a_set.func == b_set.func; + const a_ies = a.castTag(.error_set_inferred).?.data; + const b_ies = (b.castTag(.error_set_inferred) orelse return false).data; + return a_ies == b_ies; }, .anyerror => { @@ -983,10 +983,10 @@ pub const Type = extern union { .error_set_inferred => { // inferred error sets are compared using their data pointer - const set = ty.castTag(.error_set_inferred).?.data; + const ies: *Module.Fn.InferredErrorSet = ty.castTag(.error_set_inferred).?.data; std.hash.autoHash(hasher, std.builtin.TypeId.ErrorSet); std.hash.autoHash(hasher, Tag.error_set_inferred); - std.hash.autoHash(hasher, set.func); + std.hash.autoHash(hasher, ies); }, .@"opaque" => { diff --git a/test/behavior.zig b/test/behavior.zig index 0b008409fd..7efe687c75 100644 --- a/test/behavior.zig +++ b/test/behavior.zig @@ -61,6 +61,7 @@ test { _ = @import("behavior/bugs/7250.zig"); _ = @import("behavior/bugs/11100.zig"); _ = @import("behavior/bugs/10970.zig"); + _ = @import("behavior/bugs/11046.zig"); _ = @import("behavior/call.zig"); _ = @import("behavior/cast.zig"); _ = @import("behavior/comptime_memory.zig"); diff --git a/test/behavior/array.zig b/test/behavior/array.zig index eb8ae8972d..3d39942e3d 100644 --- a/test/behavior/array.zig +++ b/test/behavior/array.zig @@ -538,9 +538,6 @@ test "type coercion of anon struct literal to array" { try expect(arr1[1] == 56); try expect(arr1[2] == 54); - if (builtin.zig_backend == .stage2_llvm) return error.SkipZigTest; // TODO - if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO - var x2: U = .{ .a = 42 }; const t2 = .{ x2, .{ .b = true }, .{ .c = "hello" } }; var arr2: [3]U = t2; diff --git a/test/behavior/bugs/11046.zig b/test/behavior/bugs/11046.zig new file mode 100644 index 0000000000..4fcd33deb4 --- /dev/null +++ b/test/behavior/bugs/11046.zig @@ -0,0 +1,21 @@ +const builtin = @import("builtin"); + +fn foo() !void { + var a = true; + if (a) return error.Foo; + return error.Bar; +} +fn bar() !void { + try foo(); +} + +test "fixed" { + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + bar() catch |err| switch (err) { + error.Foo => {}, // error: expected (inferred error set of bar), found error{Foo} + error.Bar => {}, + }; +} diff --git a/test/behavior/error.zig b/test/behavior/error.zig index 0a55b34121..b3503051fb 100644 --- a/test/behavior/error.zig +++ b/test/behavior/error.zig @@ -476,7 +476,11 @@ test "function pointer with return type that is error union with payload which i } test "return result loc as peer result loc in inferred error set function" { - if (builtin.zig_backend != .stage1) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO const S = struct { fn doTheTest() !void {