Skip to content

Bare Switch Capture #20249

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
marler8997 opened this issue Jun 10, 2024 · 3 comments
Closed

Bare Switch Capture #20249

marler8997 opened this issue Jun 10, 2024 · 3 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@marler8997
Copy link
Contributor

marler8997 commented Jun 10, 2024

Abstract

Introduce a "bare switch" construct to reduce unnecessary symbol aliasing via captures. This enhancement aims to improve code readability and maintainability by eliminating redundant captures.

Proposal

When an expression:

  1. Captures a variable by value,
  2. Is immediately followed by switch (captured_value),
  3. Does not use captured_value elsewhere.

The capture MUST be omitted and a bare switch must be used.

Examples

The following 4 examples would become compile errors if BODY does not use the captured value:

EXPRESSION catch |err| switch (err) { BODY }
errdefer |err| switch (err) { BODY }
if (expr) |value| switch (value) { BODY }
if (...) { } else |err| switch (err) { BODY }

Here's the proposed syntax to fix these examples:

EXPRESSION catch switch { BODY }
errdefer switch { BODY }
if (expr) switch { BODY }
if (...) { } else switch { BODY }

Rationale

The bare switch allows payloads to be switched on without introducing a symbol into the scope. This avoids awkward situations where the same value is aliased multiple times.

A common example of this is a catch clause like this one:

// Current Syntax
const file = std.fs.cwd().openFile(filename, .{}) catch |err| switch (err) {
    error.FileNotFound => return,
    else => |e| return e,
};

// Proposed Syntax
const file = std.fs.cwd().openFile(filename, .{}) catch switch {
    error.FileNotFound => return,
    else => |err| return err,
};

There's around 800 of instances of this pattern in the ziglang github repository. The proposed change would simplify these cases eliminating the need to manage conventions to avoid conflicts with error symbols. This improvement becomes more significant with nested clauses, such as:

// Current Syntax
foo() catch |err1| switch (err1) {
    else => |err2| switch (err2) {
        else => |err3| handleError(err3),
    },
}

// Proposed Syntax
foo() catch switch {
    else => switch {
        else => |err| handleError(err),
    },
}

Note that there are also dozens of examples with other constructs like for/if in the ziglang repository that would also be affected, but this is a minority number of examples in comparison to catch.

Limitations

This proposal only applies to payloads captured by value. Payloads captured by reference require a symbol to write back to them and therefore wouldn't benefit.

@xdBronch
Copy link
Contributor

im not really a fan of this kind of shorthand syntax for things because if you ever need access to the captured variable/error you need to rewrite the surrounding code so the feature ends up becoming a burden.
the general case of a single catch |err| switch (err) is perfectly fine imo and it conflicting with symbol names seems pretty rare so the benefit seems quite minimal to me.
the nested example you gave seems kinda superficial, how often is code really written like that? is it worth adding a somewhat confusing language feature just to make something like that a little nicer?

@rohlem
Copy link
Contributor

rohlem commented Jun 10, 2024

I've also considered a shorthand like this before. I think I'm neutral on it.
While it is shorter to read, and can avoid errors of referencing the wrong temporary,
it makes the code less regular to learn/read. The version without capture is
less clear to someone encountering the construct for the first time.

To note in the if example, currently if (bool) and if (optional) |capture| are syntactically differenti(at)able.
With this proposal implemented, if (x) switch (y) {...} is the bool kind, while if (x) switch {...} is the optional kind -
it becomes a little easier to get wrong in refactoring, though that's not a huge issue IMO.

if you ever need access to the captured variable/error you need to rewrite the surrounding code

Fwiw, this actually isn't the case for switch, where you can always re-capture the switched-on value in the prong itself.
In light of #12863 , this would actually always be preferred / more idiomatic.

the nested example you gave seems kinda superficial, how often is code really written like that?

At least in my personal codebase there are at least 5 instances of catch and switch nested; while it's not a big deal to name them err and e, or e0 through eN, I wouldn't say it never happens.

@dweiller
Copy link
Contributor

Examples

The following 4 examples would become compile errors if BODY does not use the captured value:

EXPRESSION catch |err| switch (err) { BODY }
errdefer |err| switch (err) { BODY }
if (expr) |value| { switch (value) { BODY } }
if (...) { } else |err| { switch (err) { BODY } }

If accepted, I'm not sure the third and fourth ones should be errors - they seem inconsistent with the first two exampples and with the special-case error union switching where the switch is the token following the capture instead of the first statement of a following block. Rather I think the third and fourth examples should be replaced with

if (expr) |value| switch (value) { BODY }
if (...) else |err| switch (err) { BODY }

Or we also change error union switching to also work for the first statement in a block, but I think that would complicate recognition somewhat.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Jul 26, 2024
@andrewrk andrewrk closed this as not planned Won't fix, can't repro, duplicate, stale Jul 26, 2024
@andrewrk andrewrk added this to the 0.14.0 milestone Jul 26, 2024
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

5 participants