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: Inline Switch Cases #7224

Closed
SpexGuy opened this issue Nov 25, 2020 · 11 comments · Fixed by #12979
Closed

Proposal: Inline Switch Cases #7224

SpexGuy opened this issue Nov 25, 2020 · 11 comments · Fixed by #12979
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@SpexGuy
Copy link
Contributor

SpexGuy commented Nov 25, 2020

This proposal allows switch cases to be inlined and instantiated for multiple values, similar to inline for. It allows for limited conversion of runtime values to comptime-known values, by generating separate code blocks for each possible value.

For example:

const SliceTypeA = extern struct {
    len: usize,
    ptr: [*]u8,
};
const SliceTypeB = extern struct {
    ptr: [*]u8,
    len: usize,
};
const AnySlice = union(enum) {
    a: SliceTypeA,
    b: SliceTypeB,
};
pub fn getLen(slice: AnySlice) usize {
    return switch (slice) { inline else => |val| val.len; };
}

In this code, the else clause of the switch is instantiated twice, once for SliceTypeA and once for SliceTypeB. It generates two cases, which grab the length from different offsets into the union. The type of val is comptime known, but may be different in different instantiations of the case.

Some more examples:

switch (@as(usize, c)) {
    // this case is instantiated 16 times, value is comptime known
    inline 0..15 => |value| {
        const IndexType = comptime getTypeFromIndex(value);
        // do something
    },
    // this case is generated only once, value is runtime known
    else => |value| reportErrorValue(value),
}

switch (@as(NonExhaustiveEnum, x)) {
    .a => handleA(),
    .b => handleB(),
    // only allowed because _ is specified, val is comptime known
    inline else => |val| handleDynamicNamed(val),
    // cannot be inline, val is runtime known.
    _ => |val| handleUnnamed(val),
}

To keep things sane, inline else is only allowed for tagged unions and exhaustive enums, or non-exhausted enums if the _ case is specified. In the enum case, the tag is comptime known in the else clause. In the union case, the comptime-known tag may also be specified as a second capture (else => |val, tag|). The type of the payload is comptime known but its value may be runtime known.

For the purposes of eval branch quota, each instantiation of a case (except the first one) counts as a backwards branch (just like inline for).

@alexnask alexnask added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Nov 25, 2020
@zigazeljko
Copy link
Contributor

Another usecase: dynamic dispatch for enums and tagged unions.

For example, lib/std/zig/ast.zig currently does this:

pub fn iterate(base: *Node, index: usize) ?*Node {
    inline for (@typeInfo(Tag).Enum.fields) |field| {
        const tag = @intToEnum(Tag, field.value);
        if (base.tag == tag) {
            return @fieldParentPtr(tag.Type(), "base", base).iterate(index);
        }
    }
    unreachable;
}

With this proposal, it would simplify to this:

pub fn iterate(base: *Node, index: usize) ?*Node {
    return switch (base.tag) {
        inline else => |tag| @fieldParentPtr(tag.Type(), "base", base).iterate(index),
    };
}

@rohlem
Copy link
Contributor

rohlem commented Nov 26, 2020

I really like this idea, just not sure if I understand the point about _ requiring else.
I would still vote for supporting inline _ =>, so the limitation doesn't quite make sense to me. If you use a specific enum size like enum(i6) it might make perfect sense to generate all branches for some use cases.
Sure, for an enum(i32) it would be overkill, but even then the branch could contain something comptime-only like a @compileError, and allowing a trivial inline _ => {} also does no harm I think.

If we really want to straight-out disallow it for generating an absurd number of cases for _, we could just specify some limit, say 128.
But again, I don't think limiting edge cases just because they look unusual is necessary, that just forces them into using an uglier workaround.

@zigazeljko
Copy link
Contributor

If we really want to straight-out disallow it for generating an absurd number of cases for _, we could just specify some limit, say 128.

No need for that. Since each instantiation of a case (except the first one) counts as a backwards branch, the number of cases would already be limited by the eval branch quota.

@Vexu Vexu added this to the 0.8.0 milestone Nov 27, 2020
@frmdstryr
Copy link
Contributor

frmdstryr commented Dec 2, 2020

Great idea!

Another thought would be to add special syntax for the single inline else since that will probably be pretty common. Eg:

pub fn iterate(base: *Node, index: usize) ?*Node {
    inline switch (base.tag) |tag| {
      return  @fieldParentPtr(tag.Type(), "base", base).iterate(index);
    };
}

Edit : updated to use the block style, the expression style below also would work.

Also instead of the _ => ... I think undefined => ... is more clear.

@zigazeljko
Copy link
Contributor

@frmdstryr I like the idea, but the syntax you are proposing looks a bit weird. Since this is the only case when a switch would have a payload capture, it could be directly followed by an expression. Eg:

pub fn iterate(base: *Node, index: usize) ?*Node {
    return inline switch (base.tag) |tag| @fieldParentPtr(tag.Type(), "base", base).iterate(index);
}

@SpexGuy
Copy link
Contributor Author

SpexGuy commented Jan 13, 2021

I really like that extension. Here are some real world cases where I would use it:
https://github.com/SpexGuy/Zig-GrailSort/blob/3e98eb224f004eb0dfd415ea700958d87c426939/src/grailsort.zig#L733-L737
https://github.com/SpexGuy/Zig-GrailSort/blob/3e98eb224f004eb0dfd415ea700958d87c426939/src/grailsort.zig#L804-L808
These cases are on the boundary of the design tradeoff between using runtime values to reduce code bloat and using comptime values to speed up inner loops, where I need to convert from one to the other.

@ghost
Copy link

ghost commented Jan 13, 2021

@andrewrk andrewrk added the accepted This proposal is planned. label Jan 14, 2021
@andrewrk
Copy link
Member

✔️ Main proposal
❌ No syntax sugar for inline else with no cases (example)
❌ No restrictions on using integer types, the user will have to use tools to help troubleshoot where binary bloat comes from, same as status quo

@SpexGuy
Copy link
Contributor Author

SpexGuy commented Jan 14, 2021

I'll expand on this a bit. While we agree that the sugared version looks cleaner for the exhaustive case, it makes it harder to special case specific items later. With the direct syntax,

switch (x) { inline else => |ct| foo(ct) }
// becomes
switch (x) {
    .special => doSpecial(),
    inline else => |ct| foo(ct),
}

But with the sugar, the natural modification is

inline switch (x) |ct| foo(ct);
// becomes
inline switch (x) |ct| {
    if (ct == .special) {
        doSpecial();
    } else {
        foo(ct);
    }
}
// or worse
inline switch (x) |ct| {
    switch (ct) {
        .special => doSpecial(),
        else => foo(ct),
    }
}

From a maintainability standpoint, it's not clear that the sugared version is better. So we decided for language simplicity instead.


For the integer types, we felt that the limit was arbitrary. The language usually doesn't care about instantiating things like this, so introducing a new limit here seemed unintuitive. If this does become a problem, we can address it then, or introduce something different (some sort of instantiation limit, similar to the eval branch quota?) to help.

@tau-dev
Copy link
Contributor

tau-dev commented Feb 13, 2021

This is great!
@SpexGuy Besides else and ranges, would inline also work on combined cases? For example, having

const Zag = enum { A, B, C, D };
fn foo(comptime a: Zag) void {}
fn bar(comptime a: Zag) void {}

fn someRuntimeDispatch(x: Zag) void {
    switch(x) {
        inline .A, .B => |aorb| foo(aorb),
        inline .C, .D => |cord| bar(cord),
    }
}

expand to

...
switch(x) {
    .A => foo(.A),
    .B => foo(.B),
    .C => bar(.C),
    .D => bar(.D)
}

would be quite nice.

@SpexGuy
Copy link
Contributor Author

SpexGuy commented Feb 13, 2021

Yes, all types of cases will support inline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants