ziglang/zig

incorrect peer type resolution taking the address of `catch |err| switch (err)`

xdBronch opened this issue · 4 comments

Zig Version

0.12

Steps to Reproduce and Observed Behavior

test {
    @compileLog(@TypeOf(&(try @as(anyerror!usize, 0))));
    @compileLog(@TypeOf(&(@as(anyerror!usize, 0) catch {})));
    @compileLog(@TypeOf(&(@as(anyerror!usize, 0) catch |err| return err)));
    @compileLog(@TypeOf(&(@as(anyerror!usize, 0) catch |err| switch (err) {
        else => return err,
    })));
}
Compile Log Output:
@as(type, *const usize)
@as(type, *const usize)
@as(type, *const usize)
@as(type, usize)

this didnt happen in 0.11, im guessing its caused by #18173
interestingly if you take the address of the switch itself it becomes correct

@compileLog(@TypeOf((@as(anyerror!usize, 0) catch |err| &switch (err) {
    else => return err,
})));

@as(type, *const usize)

Expected Behavior

all logs should be *const usize

Compile Log Output:
@as(type, *const usize)
@as(type, *const usize)
@as(type, *const usize)
@as(type, *const usize)

Two notes:

  • Using a local const x = @as(anyerror!usize, 0) to shorten the test case leads to the same unexpected behavior.
  • I'm pretty sure the variant with &switch is broken too, as it should be doing the opposite:
    We are now (or should be) peer-type resolving @TypeOf(usize, noreturn), which should be usize, not *const usize.
    (note: @TypeOf(&{return;}) is still noreturn. Explicitly constructing *noreturn is disallowed by compile error.)

this didnt happen in 0.11, im guessing its caused by #18173 interestingly if you take the address of the switch itself it becomes correct

Yes, that PR is the cause. The reason that taking the address does not exhibit the new (buggy) behaviour is that it does not make use of the new switch_block_err_union Zir instruction. One workaround (which is probably preferable to taking the address of the switch) is to store the result of the catch-switch expression in a decl before taking the address.

This issue is not dependent on having the type of the switch be noreturn. Interestingly the (what should be) equivalent formulation using if (which also uses switch_block_err_union) does give the desired type:

test {
    const a: anyerror!usize = 0;
    const b = &(a catch |e| switch (e) {
        else => 3,
    });
    const c = &(if (a) |v| v else |e| switch (e) {
        else => 3,
    });
    @compileLog(@TypeOf(b));
    @compileLog(@TypeOf(c));
}

This results in

bug.zig:9:5: error: found compile log statement
    @compileLog(@TypeOf(b));
    ^~~~~~~~~~~~~~~~~~~~~~~

Compile Log Output:
@as(type, usize)
@as(type, *const usize)

The Zir corresponding to these two error union switches is:

%12 = switch_block_err_union(%8,
  non_err => {
    %13 = dbg_block_begin()
    %14 = err_union_payload_unsafe(%8) 
    %15 = dbg_block_end()
    %16 = break(%12, %14)
  },
  else => {
    %18 = dbg_block_begin()
    %19 = int(3)
    %20 = ref(%19) 
    %21 = dbg_block_end()
    %22 = break(%12, %20)
  }) 
%23 = dbg_var_val(%12, "b")

and

%26 = switch_block_err_union(%8,
  non_err => {
    %27 = dbg_block_begin()
    %28 = err_union_payload_unsafe(%8) 
    %31 = ref(%28) 
    %29 = dbg_block_begin()
    %30 = dbg_var_val(%28, "v")
    %32 = dbg_block_end()
    %33 = break(%26, %31)
},
  else => {
    %35 = dbg_block_begin()
    %36 = int(3)
    %37 = ref(%36) 
    %38 = dbg_block_end()
    %39 = break(%26, %37)
  }) 
%40 = dbg_var_val(%26, "c")

So the issue seems to be that the catch version is not generating a ref(%14) in the non_err branch (or maybe it could be using err_union_payload_unsafe_ptr with the ref hoisted outside the switch_err_union_block).

mlugg commented

One workaround...

Do note that this proposed workaround has quite different semantics! Consider:

var eu: anyerror!u32 = ...;
var x: u32 = ...;
const ptr = &(eu catch |err| switch (err) {
    else => x,
});
// (Intended behavior:)
// The address-of is essentially forwarded on to the sub-expressions. So, either:
// * `eu` is non-error, and `ptr` points to its payload, or
// * `eu` is error, and `ptr` points to `x`
// So, `ptr` is a MUTABLE pointer to either `x` or the payload of `eu`.
ptr.* += 1;

One workaround...

Do note that this proposed workaround has quite different semantics! Consider:

var eu: anyerror!u32 = ...;
var x: u32 = ...;
const ptr = &(eu catch |err| switch (err) {
    else => x,
});
// (Intended behavior:)
// The address-of is essentially forwarded on to the sub-expressions. So, either:
// * `eu` is non-error, and `ptr` points to its payload, or
// * `eu` is error, and `ptr` points to `x`
// So, `ptr` is a MUTABLE pointer to either `x` or the payload of `eu`.
ptr.* += 1;

I guess I was mostly thinking about getting the type resolution right and not the desired semantics, which probably isn't helpful. The behaviour can in fact be quite different when using var instead of const - e.g. the examples I made will cause a compile error (both before and after switch_block_err_union was introduced) if a is declared as

var a: anyerror!usize = 0;
a = 1;