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

Error sets inferred to be empty still need to be unwrapped with try/catch #5226

Open
alexnask opened this issue Apr 30, 2020 · 6 comments
Open
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@alexnask
Copy link
Contributor

alexnask commented Apr 30, 2020

Example:

fn foo(arg: var) !void {
    // Do stuff, conditionally return an error based on @TypeOf(arg)
    // No error for this instantiation after all!
}

pub fn main() void {
    foo(42); // error: expression value is ignored
}
@alexnask alexnask changed the title Error sets inferred to be empty still need to be called with ty to have their values unwrapped Error sets inferred to be empty still need to be unwrapped with try/catch Apr 30, 2020
@MasterQ32
Copy link
Contributor

I think that's the wanted use case. The function says it will probably return an error in future implementations, but cannot fail atm. So all invocations of foo will require try, even if it's a no-op

@Sobeston
Copy link
Sponsor Contributor

I think that this is useful for future api compatibility. A library may have a function which may not error now, but may error in future versions. I've seen this pattern used often in go codebases where a function always returns a "nil" error.

@daurnimator
Copy link
Collaborator

Yes this is intentional: without it you couldn't do a pattern like:

fn foo(comptime: Error: type, func: fn() Error!void) !void {
  return try func();
}

which is pretty common in zig (see e.g. streams).

I don't know why you would want the inconsistency of the empty error set not needing try.

@BarabasGitHub
Copy link
Contributor

I was going to make an issue that it doesn't complain when an function that says it can return an error doesn't return an error. In other words, this is allowed and compiles without error:

fn function() !void {}

but I guess that is as intended due to the above mentioned use case of maybe returning an error in the future?

@tadeokondrak
Copy link
Contributor

@BarabasGitHub, that was the old behaviour, but was recently changed: #4564

@SpexGuy
Copy link
Contributor

SpexGuy commented Apr 30, 2020

The catch clause doesn't get compiled when the error set is empty, so you can safely do something like this as a workaround:

pub fn wrapper(x: var) void {
    impl(x) catch @compileError(@typeName(@TypeOf(x)) ++ " may cause an error, call impl instead.");
}

That said, I agree with the OP that maybe this should work. Changing the error set of a function is a change to it's interface, and it's expected to have to update all call sites when a function's interface changes. So adding an error in the future isn't a valid reason to require catch now.

The real question in my mind is whether error{}!T should be equivalent to T. If they are the same, then no try should be required (but this has other weird consequences). If they are different, then error{}!T is an error set type and should follow the rules of error set types for simplicity, similar to how ?void still requires a capture when unwrapped with if or while.

That said, we have precedent for the opposite too. Expressions returning void are allowed to discard their value or put it into a variable. If error{} is analogous to void in this case, then it makes sense that handling and discarding the error should both be valid.

Another option would be to redefine inferred error sets to say that if no error set is inferred it returns T, not error{}!T. But this would be kind of a discontinuity in the language. Still maybe worth thinking about though.

@Vexu Vexu added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label May 9, 2020
@Vexu Vexu added this to the 0.7.0 milestone May 9, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 27, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 23, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

9 participants