Skip to content

addition of try prevents proper error unwrapping coercion #22633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nektro opened this issue Jan 28, 2025 · 2 comments · Fixed by #22707
Closed

addition of try prevents proper error unwrapping coercion #22633

nektro opened this issue Jan 28, 2025 · 2 comments · Fixed by #22707
Assignees
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness. regression It worked in a previous version of Zig, but stopped working.
Milestone

Comments

@nektro
Copy link
Contributor

nektro commented Jan 28, 2025

Zig Version

0.14.0-dev.2851+b074fb7dd

Steps to Reproduce and Observed Behavior

const std = @import("std");

test {
    _ = try foo();
}

fn foo() error{OutOfMemory}![]const u8 {
    return try switch (@as(enum { a, b, c }, .b)) {
        inline else => |v| std.fmt.allocPrint(std.heap.c_allocator, "{s}", .{@tagName(v)}),
    };
}
test.zig:9:46: error: expected type 'error{OutOfMemory}!error{OutOfMemory}![]const u8', found 'error{OutOfMemory}![]u8'
        inline else => |v| std.fmt.allocPrint(std.heap.c_allocator, "{s}", .{@tagName(v)}),
                           ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test.zig:9:46: note: error union payload '[]u8' cannot cast into error union payload 'error{OutOfMemory}![]const u8'

Expected Behavior

works
or tells me to remove the try

@nektro nektro added the bug Observed behavior contradicts documented or intended behavior label Jan 28, 2025
@nektro
Copy link
Contributor Author

nektro commented Jan 28, 2025

editing foo to use return switch works as expected and is arguably better code but wanted to submit this for consideration

@mlugg
Copy link
Member

mlugg commented Jan 31, 2025

Same cause as #21991; I'll try and get this sorted before the release of 0.14.0.

@mlugg mlugg added regression It worked in a previous version of Zig, but stopped working. frontend Tokenization, parsing, AstGen, Sema, and Liveness. labels Jan 31, 2025
@mlugg mlugg added this to the 0.14.0 milestone Jan 31, 2025
@mlugg mlugg self-assigned this Jan 31, 2025
mlugg added a commit to mlugg/zig that referenced this issue Feb 1, 2025
This commit effectively reverts 9e683f0, and hence un-accepts ziglang#19777.
While nice in theory, this proposal turned out to have a few problems.

Firstly, supplying a result type implicitly coerces the operand to this
type -- that's the main point of result types! But for `try`, this is
actually a bad idea; we want a redundant `try` to be a compile error,
not to silently coerce the non-error value to an error union. In
practice, this didn't always happen, because the implementation was
buggy anyway; but when it did, it was really quite silly. For instance,
`try try ... try .{ ... }` was an accepted expression, with the inner
initializer being initially coerced to `E!E!...E!T`.

Secondly, the result type inference here didn't play nicely with
`return`. If you write `return try`, the operand would actually receive
a result type of `E!E!T`, since the `return` gave a result type of `E!T`
and the `try` wrapped it in *another* error union. More generally, the
problem here is that `try` doesn't know when it should or shouldn't
nest error unions. This occasionally broke code which looked like it
should work.

So, this commit prevents `try` from propagating result types through to
its operand. A key motivation for the original proposal here was decl
literals; so, as a special case, `try .foo(...)` is still an allowed
syntax form, caught by AstGen and specially lowered. This does open the
doors to allowing other special cases for decl literals in future, such
as `.foo(...) catch ...`, but those proposals are for another time.

Resolves: ziglang#21991
Resolves: ziglang#22633
@mlugg mlugg closed this as completed in 3924f17 Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior frontend Tokenization, parsing, AstGen, Sema, and Liveness. regression It worked in a previous version of Zig, but stopped working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants