From a7f3c2eab427397846f619dba7abb72378ac2ce9 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 23 Jul 2023 17:19:50 -0700 Subject: InternPool: fix coerced func hash/eql same as uncoerced Since the same Key.Func data structure is used for coerced function bodies as well as uncoerced function bodies, there is danger of them being hashed and equality-checked as the same. When that happens, the type of a function body value might be wrong, causing lots of problems. In this instance, it causes an assertion failure. This commit fixes it by introducing an `uncoerced_ty` field which is different than `ty` in the case of `func_coerced` and is used to differentiate when doing hashing and equality checking. I have a new behavior test to cover this bug, but it revealed *another* bug at the same time, so I will fix it in the next commit and and add the new test therein. --- src/InternPool.zig | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) (limited to 'src/InternPool.zig') diff --git a/src/InternPool.zig b/src/InternPool.zig index b8eaafceab..0e9e1d4559 100644 --- a/src/InternPool.zig +++ b/src/InternPool.zig @@ -594,6 +594,13 @@ pub const Key = union(enum) { /// In the case of a generic function, this type will potentially have fewer parameters /// than the generic owner's type, because the comptime parameters will be deleted. ty: Index, + /// If this is a function body that has been coerced to a different type, for example + /// ``` + /// fn f2() !void {} + /// const f: fn()anyerror!void = f2; + /// ``` + /// then it contains the original type of the function body. + uncoerced_ty: Index, /// Index into extra array of the `FuncAnalysis` corresponding to this function. /// Used for mutating that data. analysis_extra_index: u32, @@ -990,11 +997,15 @@ pub const Key = union(enum) { // otherwise we would get false negatives for interning generic // function instances which have inferred error sets. - if (func.generic_owner == .none and func.resolved_error_set_extra_index == 0) - return Hash.hash(seed, asBytes(&func.owner_decl) ++ asBytes(&func.ty)); + if (func.generic_owner == .none and func.resolved_error_set_extra_index == 0) { + const bytes = asBytes(&func.owner_decl) ++ asBytes(&func.ty) ++ + [1]u8{@intFromBool(func.uncoerced_ty == func.ty)}; + return Hash.hash(seed, bytes); + } var hasher = Hash.init(seed); std.hash.autoHash(&hasher, func.generic_owner); + std.hash.autoHash(&hasher, func.uncoerced_ty == func.ty); for (func.comptime_args.get(ip)) |arg| std.hash.autoHash(&hasher, arg); if (func.resolved_error_set_extra_index == 0) { std.hash.autoHash(&hasher, func.ty); @@ -1122,6 +1133,12 @@ pub const Key = union(enum) { )) return false; } + if ((a_info.ty == a_info.uncoerced_ty) != + (b_info.ty == b_info.uncoerced_ty)) + { + return false; + } + if (a_info.ty == b_info.ty) return true; @@ -3371,6 +3388,7 @@ fn extraFuncDecl(ip: *const InternPool, extra_index: u32) Key.Func { const func_decl = ip.extraDataTrail(P, extra_index); return .{ .ty = func_decl.data.ty, + .uncoerced_ty = func_decl.data.ty, .analysis_extra_index = extra_index + std.meta.fieldIndex(P, "analysis").?, .zir_body_inst_extra_index = extra_index + std.meta.fieldIndex(P, "zir_body_inst").?, .resolved_error_set_extra_index = if (func_decl.data.analysis.inferred_error_set) func_decl.end else 0, @@ -3392,6 +3410,7 @@ fn extraFuncInstance(ip: *const InternPool, extra_index: u32) Key.Func { const func_decl = ip.funcDeclInfo(fi.data.generic_owner); return .{ .ty = fi.data.ty, + .uncoerced_ty = fi.data.ty, .analysis_extra_index = extra_index + std.meta.fieldIndex(P, "analysis").?, .zir_body_inst_extra_index = func_decl.zir_body_inst_extra_index, .resolved_error_set_extra_index = if (fi.data.analysis.inferred_error_set) fi.end else 0, -- cgit v1.2.3 From cc6964c5dc7a4d4c3081db06697b8ba0d7900b85 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 23 Jul 2023 17:46:50 -0700 Subject: InternPool: add func_coerced handling to funcIesResolved --- src/InternPool.zig | 11 +++++++++++ test/behavior/generics.zig | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+) (limited to 'src/InternPool.zig') diff --git a/src/InternPool.zig b/src/InternPool.zig index 0e9e1d4559..8a160d9e39 100644 --- a/src/InternPool.zig +++ b/src/InternPool.zig @@ -7069,6 +7069,17 @@ pub fn funcIesResolved(ip: *const InternPool, func_index: Index) *Index { const extra_index = switch (tags[@intFromEnum(func_index)]) { .func_decl => func_start + @typeInfo(Tag.FuncDecl).Struct.fields.len, .func_instance => func_start + @typeInfo(Tag.FuncInstance).Struct.fields.len, + .func_coerced => i: { + const uncoerced_func_index: Index = @enumFromInt(ip.extra.items[ + func_start + std.meta.fieldIndex(Tag.FuncCoerced, "func").? + ]); + const uncoerced_func_start = datas[@intFromEnum(uncoerced_func_index)]; + break :i switch (tags[@intFromEnum(uncoerced_func_index)]) { + .func_decl => uncoerced_func_start + @typeInfo(Tag.FuncDecl).Struct.fields.len, + .func_instance => uncoerced_func_start + @typeInfo(Tag.FuncInstance).Struct.fields.len, + else => unreachable, + }; + }, else => unreachable, }; return @ptrCast(&ip.extra.items[extra_index]); diff --git a/test/behavior/generics.zig b/test/behavior/generics.zig index a8a7d3c0ed..0954615223 100644 --- a/test/behavior/generics.zig +++ b/test/behavior/generics.zig @@ -456,3 +456,23 @@ test "return type of generic function is function pointer" { try expect(null == S.b(void)); } + +test "coerced function body has inequal value with its uncoerced body" { + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; + if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest; + + const S = struct { + const A = B(i32, c); + fn c() !i32 { + return 1234; + } + fn B(comptime T: type, comptime d: ?fn () anyerror!T) type { + return struct { + fn do() T { + return d.?() catch @panic("fail"); + } + }; + } + }; + try expect(S.A.do() == 1234); +} -- cgit v1.2.3 From d92cca9324e71705af9134de70e1e995ba8aa57a Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 23 Jul 2023 18:39:58 -0700 Subject: InternPool: add func_coerced handling to getFuncInstanceIes Oops, there was a missing call to `unwrapCoercedFunc`. --- src/InternPool.zig | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'src/InternPool.zig') diff --git a/src/InternPool.zig b/src/InternPool.zig index 8a160d9e39..0c01c9988c 100644 --- a/src/InternPool.zig +++ b/src/InternPool.zig @@ -4819,6 +4819,8 @@ pub fn getFuncInstanceIes( assert(arg.bare_return_type != .none); for (arg.param_types) |param_type| assert(param_type != .none); + const generic_owner = unwrapCoercedFunc(ip, arg.generic_owner); + // The strategy here is to add the function decl unconditionally, then to // ask if it already exists, and if so, revert the lengths of the mutated // arrays. This is similar to what `getOrPutTrailingString` does. @@ -4854,7 +4856,7 @@ pub fn getFuncInstanceIes( .owner_decl = undefined, .ty = func_ty, .branch_quota = 0, - .generic_owner = arg.generic_owner, + .generic_owner = generic_owner, }); ip.extra.appendAssumeCapacity(@intFromEnum(Index.none)); // resolved error set ip.extra.appendSliceAssumeCapacity(@ptrCast(arg.comptime_args)); @@ -4927,7 +4929,7 @@ pub fn getFuncInstanceIes( return finishFuncInstance( ip, gpa, - arg.generic_owner, + generic_owner, func_index, func_extra_index, arg.generation, -- cgit v1.2.3