Skip to content
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

decl literal cannot be used with catch operator #21289

Closed
paperclover opened this issue Sep 3, 2024 · 5 comments
Closed

decl literal cannot be used with catch operator #21289

paperclover opened this issue Sep 3, 2024 · 5 comments
Labels
question No questions on the issue tracker, please.

Comments

@paperclover
Copy link
Contributor

paperclover commented Sep 3, 2024

Zig Version

0.14.0-dev.1409+6d2945f1f

Steps to Reproduce and Observed Behavior

const A = struct {
    fn get() !A {
        return .{};
    }
};

pub fn main() !void {
    const a: A = try .get(); // works
    _ = a;

    const b: A = .get() catch unreachable; // does not
    _ = b;
}

note: extracted from this attempted change in my project when trying out decl literals

    var t: Transform = .{
        // ...
-       .id_decode_buf = std.ArrayList(u8).initCapacity(sfa.get(), 4096) catch
+       .id_decode_buf = .initCapacity(sfa.get(), 4096) catch
            unreachable, // stack-fallback has enough space
        // ...
    };

Expected Behavior

successful compilation, where .get is resolved to A.get

@paperclover paperclover added the bug Observed behavior contradicts documented or intended behavior label Sep 3, 2024
@MatthiasPortzel
Copy link

MatthiasPortzel commented Sep 3, 2024

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.

From the proposal where try was added as a special case.

#19777

@mlugg
Copy link
Member

mlugg commented Sep 3, 2024

Not a bug per the above. Note that try is not a "special case" in the sense that it is special-cased to work with decl literals; it just is applicable to RLS where catch is not due to being a more semantically constrained syntax form.

@mlugg mlugg closed this as not planned Won't fix, can't repro, duplicate, stale Sep 3, 2024
@andrewrk andrewrk added question No questions on the issue tracker, please. and removed bug Observed behavior contradicts documented or intended behavior labels Sep 26, 2024
@scottredig
Copy link
Contributor

scottredig commented Dec 14, 2024

So I just tried out master for reasons unrelated to decl literals, but they were the first thing I went to try as I'm very eager to start using them. Unfortunately, literally my first attempted usage hit this limitation. I hope you don't mind me necro-bumping this question; I have a clarification question on the limitations stated in the try proposal and this seems like a good place to ask a question that may add further context for further people who search about this issue.

Specifically, the proposal states that try works because the error set is known, and catch wouldn't work because the error set is not known. However, it then also states that this still works with IES because the errors discovered in the statement can be added to the IES. These statements seem contradictory in nature, and only is reasonable in the context that IES as a mechanism is currently limited only to return types.

Main question of my post: Is there some aspect of IES makes it inherently work for return types but not for the lhs expression of catch?

Let's say that the lhs takes the result type of the outer scope and creates a new "IES context", combining the result type with a to be defined set of errors. Then as the lhs's actual errors are determined (something that's clearly possible because it's done with try), they're added to this error set. Then this (now) concrete error set is passed as the error type to the rhs of the catch.

One note is that even if the rls context outside of the catch expression is an error union, the payload type of that error union should be used to form a new ies error union context. This would be required as the catch expression may change the set of errors, eg:

fn wrappedFoo() !SomeStruct {
	return .foo() catch |err| switch (err) {
		error.Foo => error.Bar,
	};
}

Actually declaring a local variable/constant with IES syntax is irrelevant to all of this. It may be a reasonable, but I don't have any motivating example so I don't care either way.

I do feel that if this is reasonable from a compiler implementation perspective, then it's fairly important from a language usability perspective. The language seems to be relying on RLS more and more which is smoothing over bumps, except it's making catch even bumpier. I don't think there's any reason that try should ever be more ergonomic than catch, since handling errors locally if able when you have maximum context should always be preferred to passing the problem up to your caller.

Doing things this way would make try <expr> actually functionally equivalent to <expr> catch |err| return err. It's not currently because of this rls limitation.


This has some implications for other proposals:

I'm not intending to press this point, but I would like to note that this also solves my concern with removing peer type resolution.

Additionally, this seems important if @Result is accepted. Eg if fmt.parseInt is changed to use rls, a catch in that same statement to handle parsing errors immediately would be common.

@mlugg
Copy link
Member

mlugg commented Dec 14, 2024

However, it then also states that this still works with IES because the errors discovered in the statement can be added to the IES. These statements seem contradictory in nature, and only is reasonable in the context that IES as a mechanism is currently limited only to return types

I'm aware you have a related comment on #22182 related to this which I have yet to respond to -- sorry, I'll get to that soon! But, essentially, inferred error sets in Zig are much more fundamentally tied to function return types than I think you realise. In particular, IES types are distinct from their resolved variants, i.e.:

fn foo() !void {
    return error.Foo;
}
test {
    const foo_val = foo();
    const FooTy = @TypeOf(foo_val);
    const FooErr = @typeInfo(FooTy).error_union.error_set;

    comptime assert(FooErr != error{Foo});
    // equivalently:
    comptime assert(FooErr != @Type(@typeInfo(FooErr)));
    // this is because `FooErr` is not `error{Foo}`, it's "the inferred error set of `Foo`", which is a
    // completely distinct type! `@typeInfo` drops this information, hence the second `assert`.
}

In other words, "adding to the IES" is fine here because isn't changing the type; the error set "before" adding an error is <IES of foo>, and the error set "after" adding an error is <IES of foo>. (Specifically, this is written @typeInfo(@typeInfo(foo).@"fn".return_type.?).error_union.error_set, but that's a bit of a mouthful.)

That said, I'm actually planning to change try so as to not propagate a result type, because this decision has turned out to allow some weird code; see #21991. When I do that, I intend to allow try .foo(...) as a special case. I don't intend to allow .foo(...) catch ... as another special case, because it introduces a lot of compiler complexity and kind of opens the floodgates -- if we allow catch, why not orelse? And why not the RHS of these operators?

I'm not completely satisfied with where decl literals are not allowed today, but solving this will require something of a rehaul to RLS, which I do plan to propose at some point soon.

Also, note that #22182 will make it much easier to propagate result types into catch and orelse, because it eliminates the following awkward case:

const my_opt_u16: ?u16 = ...;
const my_u32: u32 = ...;
const x: ?u32 = a: {
    // The obvious thing is for this block to have a result type of `?u32`.
    // However, `?u16` doesn't coerce to `?u32`, so that fails.
    // But it works today without the result type, because we unwrap the optional and apply PTR, and `u16` *can* coerce to `?u32`!
    break :a my_opt_u16;
} orelse my_u32;

Rest assured that I'm aware of this issue. I've hit it myself, and acknowledge that status quo can feel awkward and inconsistent. I would like to find a language-level solution which is easy to understand and implement whilst allowing use of decl literals in more places; it's just a somewhat hard problem, and my focuses are elsewhere right now.

@scottredig
Copy link
Contributor

I'm aware you have a related comment on #22182 related to this which I have yet to respond to -- sorry, I'll get to that soon!

No worries. I appreciate your engagement, as it lets me improve my feedback to the project. However I also view my comments as just that - feedback to the project, to be valued or discarded by the Zig core team as they see fit.

But, essentially, inferred error sets in Zig are much more fundamentally tied to function return types than I think you realise.

Ah so that's how they work, interesting.

the error set "before" adding an error is <IES of foo>, and the error set "after" adding an error is <IES of foo>.

Right. Now, I'm not saying this would be an easy change to make in the compiler, but this still doesn't seem to be something that could only ever be implemented for return types and nothing else. Eg, could you not have a <IES of lhs of catch at line y column x>?

That said, I'm actually planning to change try so as to not propagate a result type, because this decision has turned out to allow some weird code; see #21991

Ah, hm, I see the awkwardness here. The balance between "pure orthogonal interactions of the system providing power", and "just be practical and special case it" is often a delicate balancing act which invites all sorts of bike shedding. But, I'd probably prefer leaving the weirdness in over special casing it. try .{ .a = 0 }, seems pretty obviously a bit funky and not likely to produce a bug. There's a lot of valid code that fits that description, and it's not a compiler's job to try and box you in and stop every single one, imo.

And why not the RHS of these operators?

Rhs of catch seems to work with rls already? I was going to bring it up in my comment above, but I tried it and a decl literal worked there, so it didn't seem worth bringing up. The type is much more easily well defined anyways.

Typing that sentence made me realize it seems the fundamental tension here is that many operations, such as pointer de-referencing, have well defined inverse operations that can be applied at the type level. However try and catch lacks this clear inverse, creating problems. I think I like ies as a solution because it can discover the missing information in the inverse operation.

Rest assured that I'm aware of this issue.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question No questions on the issue tracker, please.
Projects
None yet
Development

No branches or pull requests

5 participants