| author |  Andrew Kelley <andrew@ziglang.org> 2023-07-26 22:51:16-07:00 |
| committer | Andrew Kelley <andrew@ziglang.org> 2023-07-27 10:12:08-07:00 |
| log | e66190025ffab39527da601980b7e3211069b6f5 |
| tree | a5ab77ea514c9993edcbd9335ad3904cebefd7bd |
| parent | 9a3adeea6ef0eb30e75e732148e0a2b93d0d0c99 |
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 #156693 files changed, 40 insertions(+), 28 deletions(-)
test- behavior
src/AstGen.zig+7-4
| ... | ... | @@ -9536,16 +9536,19 @@ fn nodeMayNeedMemoryLocation(tree: *const Ast, start_node: Ast.Node.Index, have_ |
| 9536 | 9536 | .@"for", // This variant always has an else expression. |
| 9537 | 9537 | .@"switch", |
| 9538 | 9538 | .switch_comma, |
| 9539 | .call_one, | |
| 9540 | .call_one_comma, | |
| 9541 | 9539 | .async_call_one, |
| 9542 | 9540 | .async_call_one_comma, |
| 9543 | .call, | |
| 9544 | .call_comma, | |
| 9545 | 9541 | .async_call, |
| 9546 | 9542 | .async_call_comma, |
| 9547 | 9543 | => return true, |
| 9548 | 9544 | |
| 9545 | // https://github.com/ziglang/zig/issues/2765 would change this. | |
| 9546 | .call_one, | |
| 9547 | .call_one_comma, | |
| 9548 | .call, | |
| 9549 | .call_comma, | |
| 9550 | => return false, | |
| 9551 | ||
| 9549 | 9552 | .block_two, |
| 9550 | 9553 | .block_two_semicolon, |
| 9551 | 9554 | .block, |
src/Sema.zig+8-24
| ... | ... | @@ -30732,18 +30732,10 @@ fn analyzeIsNonErrComptimeOnly( |
| 30732 | 30732 | else => return .none, |
| 30733 | 30733 | }, |
| 30734 | 30734 | } |
| 30735 | for (ies.inferred_error_sets.keys()) |other_ies_index| { | |
| 30736 | if (set_ty == other_ies_index) continue; | |
| 30737 | const other_resolved = | |
| 30738 | try sema.resolveInferredErrorSet(block, src, other_ies_index); | |
| 30739 | if (other_resolved == .anyerror_type) { | |
| 30740 | ies.resolved = .anyerror_type; | |
| 30741 | return .none; | |
| 30742 | } | |
| 30743 | if (ip.indexToKey(other_resolved).error_set_type.names.len != 0) | |
| 30744 | return .none; | |
| 30745 | } | |
| 30746 | return .bool_true; | |
| 30735 | // We do not have a comptime answer because this inferred error | |
| 30736 | // set is not resolved, and an instruction later in this function | |
| 30737 | // body may or may not cause an error to be added to this set. | |
| 30738 | return .none; | |
| 30747 | 30739 | }, |
| 30748 | 30740 | else => switch (ip.indexToKey(set_ty)) { |
| 30749 | 30741 | .error_set_type => |error_set_type| { |
| ... | ... | @@ -30771,18 +30763,10 @@ fn analyzeIsNonErrComptimeOnly( |
| 30771 | 30763 | else => return .none, |
| 30772 | 30764 | }, |
| 30773 | 30765 | } |
| 30774 | for (ies.inferred_error_sets.keys()) |other_ies_index| { | |
| 30775 | if (set_ty == other_ies_index) continue; | |
| 30776 | const other_resolved = | |
| 30777 | try sema.resolveInferredErrorSet(block, src, other_ies_index); | |
| 30778 | if (other_resolved == .anyerror_type) { | |
| 30779 | ies.resolved = .anyerror_type; | |
| 30780 | return .none; | |
| 30781 | } | |
| 30782 | if (ip.indexToKey(other_resolved).error_set_type.names.len != 0) | |
| 30783 | return .none; | |
| 30784 | } | |
| 30785 | return .bool_true; | |
| 30766 | // We do not have a comptime answer because this inferred error | |
| 30767 | // set is not resolved, and an instruction later in this function | |
| 30768 | // body may or may not cause an error to be added to this set. | |
| 30769 | return .none; | |
| 30786 | 30770 | } |
| 30787 | 30771 | } |
| 30788 | 30772 | const resolved_ty = try sema.resolveInferredErrorSet(block, src, set_ty); |
test/behavior/error.zig+25
| ... | ... | @@ -938,3 +938,28 @@ test "returning an error union containing a type with no runtime bits" { |
| 938 | 938 | var zero_byte: ZeroByteType = undefined; |
| 939 | 939 | (&zero_byte).* = try ZeroByteType.init(); |
| 940 | 940 | } |
| 941 | ||
| 942 | test "try used in recursive function with inferred error set" { | |
| 943 | if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO | |
| 944 | if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO | |
| 945 | ||
| 946 | const Value = union(enum) { | |
| 947 | values: []const @This(), | |
| 948 | b, | |
| 949 | ||
| 950 | fn x(value: @This()) !void { | |
| 951 | switch (value.values[0]) { | |
| 952 | .values => return try x(value.values[0]), | |
| 953 | .b => return error.a, | |
| 954 | } | |
| 955 | } | |
| 956 | }; | |
| 957 | const a = Value{ | |
| 958 | .values = &[1]Value{ | |
| 959 | .{ | |
| 960 | .values = &[1]Value{.{ .b = {} }}, | |
| 961 | }, | |
| 962 | }, | |
| 963 | }; | |
| 964 | try expectError(error.a, Value.x(a)); | |
| 965 | } |