Skip to content

Supply result type to operand of try #19777

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
mlugg opened this issue Apr 26, 2024 · 5 comments
Closed

Supply result type to operand of try #19777

mlugg opened this issue Apr 26, 2024 · 5 comments
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@mlugg
Copy link
Member

mlugg commented Apr 26, 2024

Short lil proposal today!

In general, syntax forms which "extract" components of an operand cannot provide result types to said operands. For instance, a field access x.foo clearly cannot provide a result type to the expression x, as there may be arbitrarily many aggregate types with a field named foo. In general, the same is true of error handling: for instance, x catch ... cannot provide a result type to x even if the overall expression has a result type, because while this type may contain sufficient information to determine the payload type, we cannot determine the error type.

However, there is one interesting exception here! The try operator could, in theory, provide a correct result type to its operand. If an expression try x has result type T, then the sub-expression x can be assigned result type E!T, where E is the error set of the current function. Because the behavior of try is more limited than that of e.g. catch, we may not know the error set type that the expression will actually return, but we do know that it must be coercible to the error set of this function, and we lose no information from applying this rule (the result would have been coerced to this type by the ret_node ZIR in the error path regardless). This coercion will correctly handle growing an IES, both runtime and ad-hoc. The more I think about it, the more this seems to me like a no-brainer.

Many thanks to @silversquirl for suggesting this to me :D

@mlugg mlugg added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Apr 26, 2024
@mlugg mlugg added this to the 0.13.0 milestone Apr 26, 2024
@InKryption
Copy link
Contributor

Speaking as a bit of a layman to the compiler jargon, might I ask what type of code this would allow which currently is not allowed?

@silversquirl
Copy link
Contributor

@InKryption the usecase I was initially considering when suggesting this was #9938. With decl literals and this both implemented, you'd be able to do stuff like const foo: ArrayList = try .initCapacity(allocator, 10); without needing to special-case try in the decl literal logic.

Other usecases are a bit more niche, but one example is const foo: u32 = try if (cond) 3 else error.Foo;
That example is a bit contrived though haha

@mlugg
Copy link
Member Author

mlugg commented Apr 28, 2024

A slightly less esoteric use-case today is try @errorCast(xyz) in a function with a concrete error set.

@andrewrk
Copy link
Member

andrewrk commented Aug 30, 2024

Proposal seems fine, let's see if the implementation turns up any complications.

Related:

@andrewrk andrewrk added the accepted This proposal is planned. label Aug 30, 2024
@mlugg
Copy link
Member Author

mlugg commented Aug 30, 2024

There is a really esoteric annoying case, which you'd need something like this to repro:

const my_slice: []const u32 = &try (if (foo) error.Bar else .{@intCast(bar)});

We can't forward slice ref result locations through try because the operand needs to be a *const E![n]u32 and we don't know n; we usually lower this case by saying we need a type which when refd gives a []const u32, but there's no meaningful equivalent here ([]const E!u32 would be wrong, we only want one possible error).

This is ridiculously esoteric. We could solve it in the future by changing how we represent ref result locations in the compiler, but for now I'm just making this drop the result type. If someone actually hits this in the wild, I will visit a clothing store and ingest every available hat.

mlugg added a commit to mlugg/zig that referenced this issue Aug 31, 2024
This is mainly useful in conjunction with Decl Literals (ziglang#9938).

Resolves: ziglang#19777
@mlugg mlugg closed this as completed in 9e683f0 Sep 1, 2024
richerfu pushed a commit to richerfu/zig that referenced this issue Oct 28, 2024
This is mainly useful in conjunction with Decl Literals (ziglang#9938).

Resolves: ziglang#19777
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 added a commit that referenced this issue Feb 1, 2025
This commit effectively reverts 9e683f0, and hence un-accepts #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: #21991
Resolves: #22633
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

4 participants