From e66190025ffab39527da601980b7e3211069b6f5 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 26 Jul 2023 22:51:16 -0700 Subject: [PATCH] frontend: make fn calls byval; fix false positive isNonErr This commit does two things which seem unrelated at first, but, together, solve a miscompilation, and potentially slightly speed up compiler perf, at the expense of making #2765 trickier to implement in the future. Sema: avoid returning a false positive for whether an inferred error set is comptime-known to be empty. AstGen: mark function calls as not being interested in a result location. This prevents the test case "ret_ptr doesn't cause own inferred error set to be resolved" from being regressed. If we want to accept and implement #2765 in the future, it will require solving this problem a different way, but the principle of YAGNI tells us to go ahead with this change. Old ZIR looks like this: %97 = ret_ptr() %101 = store_node(%97, %100) %102 = load(%97) %103 = ret_is_non_err(%102) New ZIR looks like this: %97 = ret_type() %101 = as_node(%97, %100) %102 = ret_is_non_err(%101) closes #15669 --- src/AstGen.zig | 11 +++++++---- src/Sema.zig | 32 ++++++++------------------------ test/behavior/error.zig | 25 +++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/AstGen.zig b/src/AstGen.zig index e6a4f6085f..667137bd61 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -9536,16 +9536,19 @@ fn nodeMayNeedMemoryLocation(tree: *const Ast, start_node: Ast.Node.Index, have_ .@"for", // This variant always has an else expression. .@"switch", .switch_comma, - .call_one, - .call_one_comma, .async_call_one, .async_call_one_comma, - .call, - .call_comma, .async_call, .async_call_comma, => return true, + // https://github.com/ziglang/zig/issues/2765 would change this. + .call_one, + .call_one_comma, + .call, + .call_comma, + => return false, + .block_two, .block_two_semicolon, .block, diff --git a/src/Sema.zig b/src/Sema.zig index 3ae2ab96bd..4a4823df2d 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -30732,18 +30732,10 @@ fn analyzeIsNonErrComptimeOnly( else => return .none, }, } - for (ies.inferred_error_sets.keys()) |other_ies_index| { - if (set_ty == other_ies_index) continue; - const other_resolved = - try sema.resolveInferredErrorSet(block, src, other_ies_index); - if (other_resolved == .anyerror_type) { - ies.resolved = .anyerror_type; - return .none; - } - if (ip.indexToKey(other_resolved).error_set_type.names.len != 0) - return .none; - } - return .bool_true; + // We do not have a comptime answer because this inferred error + // set is not resolved, and an instruction later in this function + // body may or may not cause an error to be added to this set. + return .none; }, else => switch (ip.indexToKey(set_ty)) { .error_set_type => |error_set_type| { @@ -30771,18 +30763,10 @@ fn analyzeIsNonErrComptimeOnly( else => return .none, }, } - for (ies.inferred_error_sets.keys()) |other_ies_index| { - if (set_ty == other_ies_index) continue; - const other_resolved = - try sema.resolveInferredErrorSet(block, src, other_ies_index); - if (other_resolved == .anyerror_type) { - ies.resolved = .anyerror_type; - return .none; - } - if (ip.indexToKey(other_resolved).error_set_type.names.len != 0) - return .none; - } - return .bool_true; + // We do not have a comptime answer because this inferred error + // set is not resolved, and an instruction later in this function + // body may or may not cause an error to be added to this set. + return .none; } } const resolved_ty = try sema.resolveInferredErrorSet(block, src, set_ty); diff --git a/test/behavior/error.zig b/test/behavior/error.zig index 1c4450a07f..4f26c9b18c 100644 --- a/test/behavior/error.zig +++ b/test/behavior/error.zig @@ -938,3 +938,28 @@ test "returning an error union containing a type with no runtime bits" { var zero_byte: ZeroByteType = undefined; (&zero_byte).* = try ZeroByteType.init(); } + +test "try used in recursive function with inferred error set" { + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + const Value = union(enum) { + values: []const @This(), + b, + + fn x(value: @This()) !void { + switch (value.values[0]) { + .values => return try x(value.values[0]), + .b => return error.a, + } + } + }; + const a = Value{ + .values = &[1]Value{ + .{ + .values = &[1]Value{.{ .b = {} }}, + }, + }, + }; + try expectError(error.a, Value.x(a)); +}