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.

Sign up for GitHub

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

langref: add example for errdefer |err| sytnax #20441

Merged
merged 1 commit into from
Jul 21, 2024
Merged

langref: add example for errdefer |err| sytnax #20441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/langref.html.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2988,6 +2988,10 @@ fn createFoo(param: i32) !Foo {
the verbosity and cognitive overhead of trying to make sure every exit path
is covered. The deallocation code is always directly following the allocation code.
</p>
<p>
The {#syntax#}errdefer{#endsyntax#} statement can optionally capture the error:
</p>
{#code|test_errdefer_capture.zig#}
{#header_close#}
{#header_open|Common errdefer Slip-Ups#}
<p>
Expand Down
19 changes: 19 additions & 0 deletions doc/langref/test_errdefer_capture.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
const std = @import("std");

fn captureError(captured: *?anyerror) !void {
errdefer |err| {
captured.* = err;
}
return error.GeneralFailure;
}

test "errdefer capture" {
var captured: ?anyerror = null;

if (captureError(&captured)) unreachable else |err| {
try std.testing.expectEqual(error.GeneralFailure, captured.?);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ if I remove .? and replace captured.? with just captured the test fails with expected error.GeneralFailure, found error.GeneralFailure. Seems like a bug in the language: either this code should not compile without ?, or it should return true.

Copy link
Contributor

@rohlem rohlem Jun 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could just be that std.testing.expectEqual doesn't look at the payload of optionals, but when printing the output is the same.
EDIT: Nope, looks like @TypeOf(a, b) should make both into optionals, which should be unwrapped and fail comparison here. Maybe some conversion from single-member error set to anyerror is screwy?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found the issue: @TypeOf of an optional error type with a non-optional error type leads to an error union with optional error payload,
in this case @TypeOf(error.GeneralError, ?anyerror) == error{GeneralError}!?anyerror.
Here the error.GeneralError value inhabits the error, while the ?anyerror inhabits the payload, which makes them unequal in the ErrorUnion case of expectEqual.
IMO this isn't a language defect but a deficiency in std.testing.expectEqual, filing an issue for discussion.

Copy link
Contributor

@Rexicon226 Rexicon226 Jun 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's happening is that the "found" error.GeneralFailure, is an instant of the anyerror, whereas the "expected" error.GeneralFailure is part of the IES. There is no well defined behaviour for coercing an error into an error{X}!?anyerror. Does the error go into the error set or into the anyerror? Here it just become two different things that doesn't equal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Rexicon226 I do think that there is well-defined behavior: Anything that isn't an undecorated error is a payload; that's the way type resolution for return works (which is the same mechanism behind multi-argument @TypeOf).
The reason one ?anyerror is a payload I think is that it's decorated by ?, I don't think it being part of an inferred error set matters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created #20442 for discussion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it! Just to restate it, the problem here is that expectEquals tries to find a common type between its arguments. The lhs type here is error{GeneralFailure}, the rhs type is ?anyerror. And there are three "reasonable" answers as to what's the common type here:

  1. common type is ?anyerror -> convert singleton error set to anyerror, that convert that to an optional.
  2. common type is an error union error{GeneralFailure} ! ?anyerror -> using the general rule that the common type for error{...} and T is error{...}!T
  3. no common type, as this is ambigious for the human.

Currently, Zig uses the option 2.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohlem I see, you’re right. Didn’t fully consider how it was working. Thanks for the issue :)

try std.testing.expectEqual(error.GeneralFailure, err);
}
}

// test