Skip to content

Commit

Permalink
frontend: make fn calls byval; fix false positive isNonErr
Browse files Browse the repository at this point in the history
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
  • Loading branch information
andrewrk committed Jul 27, 2023
1 parent 7201e69 commit 7d8cdf6
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 28 deletions.
11 changes: 7 additions & 4 deletions src/AstGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
32 changes: 8 additions & 24 deletions src/Sema.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down Expand Up @@ -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);
Expand Down
25 changes: 25 additions & 0 deletions test/behavior/error.zig
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

0 comments on commit 7d8cdf6

Please sign in to comment.