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

Proposal: extend simple else prong special case when switching on errors #21885

Open
dweiller opened this issue Nov 2, 2024 · 6 comments
Open

Comments

@dweiller
Copy link
Contributor

dweiller commented Nov 2, 2024

note: I'm not sure how to title this issue - suggestions welcome

Status quo

Currently, there is a special case for else prongs of switches on error sets that allows simple else prongs even if all cases have already been handled, in order to support generic code. This is the comment from the source that explains it:

// In order to enable common patterns for generic code allow simple else bodies
// else => unreachable,
// else => return,
// else => |e| return e,
// even if all the possible errors were already handled.

The existing special cases only allows correct code to return when an error that is not specifically handled is encountered, but there are cases a different error should not result in returning.

I haven't been able to find an issue where this special case was discussed (and searching in the language reference didn't find a mention of it), so I'm not sure if there was some discussion about what should be included in this feature in the past.

Proposal

I propose that we extend the special cases in some way to allow the else case to allow some default handling that is not a return. Concretely I would suggest to allow an empty block and a break statement.

This would mean that the list in the comment above would become:

// else => unreachable,
// else => return,
// else => |e| return e,
// else => break :blk,
// else => break :blk v,
// else => {},

This would allow some default handling to cover errors that hit the else or to continue execution of the function in cases where other errors can be ignored. Note that break wouldn't be strictly needed for this as the same effect could be achieved with an empty block, a break after the switch and a break in all the explicit switch cases, but the control flow would start becoming hard to read.

Why not?

The main reason I could see to not make this (or similar) extensions to the feature is to keep things simple. The more kinds of 'simple' prong bodies that are allowed, the longer the list of things you need to remember. I'm not sure there would be an obvious set of 'simple' prong bodies to allow (if break is allowed, maybe continue should be?), which would be a reason to keep it to an easy-to-remember rule like 'return statements' or 'branching statements'.

In addition this special case may be confusing to readers since (as far as I recall) it's the only case where a switch expression/statement can have more cases than there are values, so more use-cases for this feature increases the opportunity for this feature to confuse readers. The issue I see when reading code is that more context is required to determine whether there actually are any errors that can hit the else prong.

These objections could also apply to the feature as currently implemented - I think it comes down to making a judgement call on where to draw the line between easier to write generic code and easier to read/reason about code. I would expect that we'd decide on either something pretty restrictive, or something with an easy to remember rule. I see some tension between this proposal (and to a lesser extent the current feature) and

  • Favor reading code over writing code.
  • Avoid local maximums.
  • Reduce the amount one must remember.

from Zig's zen.

@gooncreeper
Copy link
Contributor

I'm not sure what the issue is, doesn't this work already?

const std = @import("std");

pub fn main() !void {
	std.io.getStdOut().writer().writeAll("Hello world!") catch |err| switch (err) {
		error.DiskQuota,
		error.FileTooBig,
		error.NoSpaceLeft,
		error.NotOpenForWriting => return error.Unwritable,
		else => |e| std.log.err("{s}\n", .{@errorName(e)}),
	};
}

@dweiller
Copy link
Contributor Author

dweiller commented Nov 2, 2024

I'm not sure what the issue is, doesn't this work already?

const std = @import("std");

pub fn main() !void {
	std.io.getStdOut().writer().writeAll("Hello world!") catch |err| switch (err) {
		error.DiskQuota,
		error.FileTooBig,
		error.NoSpaceLeft,
		error.NotOpenForWriting => return error.Unwritable,
		else => |e| std.log.err("{s}\n", .{@errorName(e)}),
	};
}

No - that code only compiles if writeAll() can return errors other than the four explicitly handled ones but will not compile if there are no other errors. The feature I'm proposing to extend is a special case that means it is not a compile error when all possible errors have been explicitly handled by switch prongs but there is still and else prong.

@gooncreeper
Copy link
Contributor

I see. I think instead of just extending it, the else prong should allow anything. For instance right now, it doesn't allow something simple like else => return error.Unexpected, and continuously expanding what is allowed as each useful case pops up is not the way to go.

@nektro
Copy link
Contributor

nektro commented Nov 2, 2024

No - that code only compiles if writeAll() can return errors other than the four explicitly handled ones but will not compile if there are no other errors.

i believe an exception to the exhaustiveness check was added explicitly for error switches using else since it very common for generic code to use catch switch. if you have code that reproduces your description i'd be very curious to see.

@rohlem
Copy link
Contributor

rohlem commented Nov 3, 2024

See also #20610 (rejected)

if you have code that reproduces your description i'd be very curious to see.

fn handle(x: anytype) void {
    _ = x catch |err| switch (err) {
        error.A => {},
        //error.B => {}, //compile error for a and b: expected type 'error{A}', found 'error{B}'
        //else => {}, //compile error for a and b: unreachable else prong; all cases already handled
    };
}
fn b() !void {
    if (true) return error.A;
}
fn c() !void {}
test {
    const a: error{A}!void = error.A;
    handle(a);
    handle(b());
    handle(c()); //only this variant does not trigger compile errors, which IMO is inconsistent and may be a bug / unintentional.
}

@dweiller
Copy link
Contributor Author

dweiller commented Nov 3, 2024

if you have code that reproduces your description i'd be very curious to see.

Code that for what description? In that message I was just describing the status quo behaviour for the else prong in the quoted code snippet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants