ast: Add comptime reason for error set merge

[marekmaskarinec]

This PR adds a better error message if a non-comptime operand is used with ||. This would often happen when accidentally using || instead of or. Previously the error looked like this:

repro.zig:5:12: error: comptime call of extern function
    _ = foo() || b;
        ~~~^~
repro.zig:5:12: note: types must be comptime-known

Now a new notes is added:

repro.zig:5:12: error: comptime call of extern function
    _ = foo() || b;
        ~~~^~
repro.zig:5:12: note: operand to error set merge (||) must be an error set

[silversquirl]

Suggested change

	.merge_error_set => "operand to error set merge (||) must be an error set",
	.merge_error_set => "operand to error set merge operator (||) must be a type",

although, really, it would probably be better to use the types must be comptime-known error message, with an additional note explaining why it expects a type.

[andrewrk]

Thank you for your efforts, however, this error message assumes too much. Consider for instance, if in your example, foo was a function returning an error set. In this case your error message is incorrect. Compile errors must never state or imply incorrect things, even if doing so is usually helpful.

EDIT: Hmm, on the other hand, it may be possible to prove that the error message could never be displayed in the case that it is an error set. For instance if it is the return type of a function it will force the function to be compile-time. If that's the case, then the property is upheld that the compile error would never state or imply an incorrect thing, making this change good.

[marekmaskarinec]

Hmm, on the other hand, it may be possible to prove that the error message could never be displayed in the case that it is an error set. For instance if it is the return type of a function it will force the function to be compile-time.

Yes, my assumption was comes from the fact that there isn't a way to create a non-comptime error set (but I might be wrong on this). That means a value being non-comptime implies it isn't an error set.

A more conservative approach could be changing the note to what @silversquirl suggested.

[andrewrk]

I think there could still be a problem if the comptime reason ends up being something else inside a function that returns an error set. Then the comptime reason trace would include the line "operand to error set merge (||) must be an error set" while pointing to an operand that is, indeed, an error set.

[andrewrk]

Example:

test "example" {
    const A = error{A};
    const B = A || foo();
    _ = B;
}
fn foo() type {
    bar();
    return error{B};
}
extern fn bar() void;

test.zig:7:8: error: comptime call of extern function
    bar();
    ~~~^~
test.zig:3:23: note: called at comptime from here
    const B = A || foo();
                   ~~~^~
test.zig:3:23: note: types must be comptime-known

[silversquirl]

the main reason i suggested changing "an error set" to "a type" is because "error set" is confusing - it somewhat implies a value of type error { ... }, not an error set type itself (which is of type type).

@andrewrk i think this confusion is demonstrated quite well by the fact that your example actually makes exactly this mistake! foo() should return type, not error{B} :)

[andrewrk]

Oops! Good catch. I think both of our points stand.