ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
35.12k stars 2.56k forks source link

no error on invalid `try` before anon struct #21991

Open travisstaloch opened 1 week ago

travisstaloch commented 1 week ago

Zig Version

0.14.0-dev.2126+e27b4647d

Steps to Reproduce and Observed Behavior

The compiler doesn't report an error message here and these 3 tests all pass.

const std = @import("std");

test {
    const S = struct { a: u8 };
    const s: S = try .{ .a = 0 }; // try makes no sense here
    try std.testing.expectEqual(0, s.a);
}

I originally noticed this while refactoring some code and forgot to remove try from a switch statement, similar to these

test {
    const S = struct { a: u8 };
    const s: S = switch (0) {
        0 => try .{ .a = 0 }, // try makes no sense here
        else => unreachable,
    };
    try std.testing.expectEqual(0, s.a);
}

test {
    const S = struct { a: u8 };
    const s: S = if (true)
        try .{ .a = 0 } // try makes no sense here
    else
        unreachable;

    try std.testing.expectEqual(0, s.a);
}

Expected Behavior

I would expect an error message like this:

test {
    const S = struct { a: u8 };
    const s = try S{ .a = 0 };
    try std.testing.expectEqual(0, s.a);
}
$ zig test /tmp/tmp.zig
/tmp/tmp.zig:5:23: error: expected error union type, found 'tmp.test_0.S'
    const s: S = try S{ .a = 0 };
                     ~^~~~~~~~~~
/tmp/tmp.zig:4:15: note: struct declared here
    const S = struct { a: u8 };
              ^~~~~~~~~~~~~~~~
pierrelgol commented 1 week ago

I can reproduce this with 0.14.0-dev.2245+4fc295dc0. In fact its more silly than that this compiles just fine.

const std = @import("std");

test {
    const S = struct { a: u8 };
    const s: S = try try try try try try try try try try .{ .a = 0 }; // try makes no sense here
    try std.testing.expectEqual(0, s.a);
}

pub fn main() void {}
mlugg commented 1 week ago

This is caused by a combination of two things:

I think this issue indicates that try should not provide a result type; that is, #19777 should be reverted. The concrete reasoning is that if the operand to try is not already an error union, it is incorrect to use try, but providing a result type to the operand masks this by applying a coercion which always turns the operand into an error union. (As a side note, this issue also shows that the current implementation of this is clearly incorrect, because this error union result type for the try operand isn't applied in simpler cases, e.g. const x: u8 = try 1, hence exposing an implementation detail.)

It's still useful to be able to use try in conjunction with Decl Literals, so that can be allowed as a syntactic special case.

mathieu-suen-sonarsource commented 6 days ago

Maybe a naive question but why not disallowing the syntax try <Literal value> ?