# Are these patterns OK?

**URL:** <https://ziggit.dev/t/are-these-patterns-ok/3007>\
**Category:** Brainstorming\
**Tags:** language\
**Created:** [January 26, 2024, 8:23am UTC](https://ziggit.dev/t/are-these-patterns-ok/3007 "2024-01-26T08:23:05Z")\
**Posts on this page:** 7\
**Page:** 1

<div class="post-metadata">

**Author:** ![Durobot](https://ziggit.dev/user_avatar/ziggit.dev/durobot/32/447_2.png) [@Durobot](https://ziggit.dev/u/Durobot)\
**Post date:** [January 26, 2024, 8:23am UTC](https://ziggit.dev/t/are-these-patterns-ok/3007/1 "2024-01-26T08:23:05Z")

</div>

I still consider myself new to Zig, so I would like to somebody with more experience to correct me if my line of thinking was wrong in the cases below.

1. In a (moderately) long function, which MUST return a value (or an error), and returns said value / error on more than one line (depending on various conditions), and which handles all the possible cases before it reaches the bottom (as far as you can tell), would you add some sort of `@panic` or `unreachable` statement at the bottom to indicate that your assumption was wrong, and none of the return statements fired?  
Something like

```zig
fn ham(...) !MyType
{
    Many conditions and returns, covering all the possible cases (or so you think);
    @panic("Bottom of fn ham reached!"); // Do you add this, or do you rely on the compiler to catch all the branches?
}

```

`unreachable` is not going to do much in unsafe modes though, so, `@panic`, I guess?

1. Your function should have a comptime parameter, of a generic type - you don’t know the type precisely, but you know its “category”. Say, a struct, a pointer to a struct, an integer, etc.  
Is it ok to do this, or are there better ways:

```zig
fn eggs(ptr_tgt: anytype) void // Expect a pointer to a mutable struct
{
    // Comptime check type of ptr_tgt
    const ptr_tgt_type_info = @typeInfo(@TypeOf(ptr_tgt));
    if (ptr_tgt_type_info != .Pointer or
        ptr_tgt_type_info.Pointer.is_const or
        @typeInfo(@TypeOf(ptr_tgt.*)) != .Struct)
        @compileError("fn eggs: 'ptr_tgt' must be a pointer to a mutable (non-const) struct, " ++
                      @typeName(@TypeOf(ptr_tgt)) ++ " provided");
    // ptr_tgt IS a ponter to mutable struct, we can work with it
    . . .
}

```

---

<div class="post-metadata">

**Author:** ![Sze](https://ziggit.dev/user_avatar/ziggit.dev/sze/32/496_2.png) [@Sze](https://ziggit.dev/u/Sze)\
**Post date:** [January 26, 2024, 12:25pm UTC](https://ziggit.dev/t/are-these-patterns-ok/3007/2 "2024-01-26T12:25:18Z")

</div>

Regarding 1. If you write a function that reaches the end without returning (when it isn’t void) zig complains at you, like this:

```zig
fn doSomething(x: i32) bool {
    if (x == 5) return true;
}

pub fn main() !void {
    _ = doSomething(5);
}

```

Output:

```zig
patternsquestion.zig:3:24: error: function with non-void return type 'bool' implicitly returns
fn doSomething(x: i32) bool {
                       ^~~~
patternsquestion.zig:5:1: note: control flow reaches end of body here
}
^

```

I think you shouldn’t pre-emptively add `@panic` or `unreachable` at the bottom, because those signal to the compiler “I want a runtime crash if something gets here” (instead of the compile time error which is better) and “I made sure this can’t happen” (which you didn’t if you are unsure) respectively, so in both of these cases it is better to get the compiler error that the control flow reaches the end of the body.

And when you get that error, then you can think about what the right thing to do is,  
whether you know enough, to make that decision, whether using `@panic` or `unreachable` is appropriate.

There are also cases, where you can just change the code to avoid needing `@panic` or `unreachable`. Instead of:

```zig
const Option = enum { A, B };
fn choose(o: Option) u32 {
    switch (o) {
        .A => return 4,
        .B => return 764,
    }
    unreachable;
}

```

Write this:

```zig
fn chooseBetter(o: Option) u32 {
    return switch (o) {
        .A => 4,
        .B => 764,
    };
}

```

Instead of only thinking about each case individually (hoping the unreachable is actually correct):  
_sidenote: this probably still would be better as a switch expression, imagine a situation where a switch doesn’t work well_

```zig
const Case = enum { A, B, C };
fn handleCases(c: Case, val: i32) i32 {
    if (c == .A and val == 15) return 25;
    if (c == .A) return 0;
    if (c == .B and val > 3) return 9;
    if (c == .B) return 0;
    if (c == .C and val < 0) return 100;
    if (c == .C) return 0;
    unreachable;
}

```

Think if there is some default case that always applies, when none of the early exit conditions apply. To get there sometimes it is helpful to reverse the way you are thinking about the problem. In this case maybe you could try writing the function in reverse starting from the last return statement (that is unconditional). _Also tangentially related: think about whether your problem is a dynamic programming problem that has a certain kind of base case._

```zig
fn handleCasesBetter(c: Case, val: i32) i32 {
    if (c == .A and val == 15) return 25;
    if (c == .B and val > 3) return 9;
    if (c == .C and val < 0) return 100;
    return 0;
}

```

I think you should consider all the things above before you consider using `@panic` or `unreachable` at the end of the function.

And if you decide to use them, it is probably still a good idea to use plenty of `std.debug.assert`s and writing lots of test cases, to make sure that you don’t have a logic error or faulty assumption somewhere.

For functions with a small number of combinations, you can exhaustively check them, for other functions, you can use fuzzing and try to isolate the function within a module, so that it is used by code in one particular manner that is well tested.

* * *

Regarding 2. I think it is ok, personally I tend to put these checks in local named functions and then write:

```zig
fn eggs(ptr_tgt: anytype) void // Expect a pointer to a mutable struct
{
    comptime checkMutableStructPointer(@TypeOf(ptr_tgt));
    ...
}

```

But that depends a bit whether the check is complex, used multiple times and what you find more readable.

---

<div class="post-metadata">

**Author:** ![Durobot](https://ziggit.dev/user_avatar/ziggit.dev/durobot/32/447_2.png) [@Durobot](https://ziggit.dev/u/Durobot)\
**Post date:** [January 26, 2024, 1:13pm UTC](https://ziggit.dev/t/are-these-patterns-ok/3007/3 "2024-01-26T13:13:50Z")

</div>

TL;DR - don’t add `unreachable` or `@panic`, Zig compiler knows its job.

I have tried intentionally “making holes” in the logic of `fn handleCases` above, and the compiler caught me red-handed:

```zig
fn handleCases(c: Case, val: i32) i32
{
    if (c == .A and val == 15) return 25;
    //if (c == .A) return 0;
    if (c == .B and val > 3) return 9;
    if (c == .B) return 0;
    if (c == .C and val < 0) return 100;
    if (c == .C) return 0;
    //unreachable;
}

```

Even if I call `handleCases(.A, 15)`, passing values that are handled by `handleCases`, I get this compilation error:

```zig
handle_cases.zig:18:35: error: function with non-void return type 'i32' implicitly returns
fn handleCases(c: Case, val: i32) i32
                                  ^~~
handle_cases.zig:27:1: note: control flow reaches end of body here                                                                            
}
^

```

If I add `unreachable` or `@panic` at the bottom of `handleCases`, it now compiles and runs. Which is much worse - I now have a trap in my code ready to spring.

So, my conclusion is that not having `unreachable` or `@panic` at the bottom of such functions is a much better approach, since you can catch holes in your logic at compile time, rather than runtime.

Oh, and if your function is not called, it doesn’t matter if it returns anything at all, you’re not going to get compile errors with or without `unreachable` or `@panic`.

---

<div class="post-metadata">

**Author:** ![squeek502](https://ziggit.dev/user_avatar/ziggit.dev/squeek502/32/409_2.png) [@squeek502](https://ziggit.dev/u/squeek502)\
**Post date:** [January 26, 2024, 6:17pm UTC](https://ziggit.dev/t/are-these-patterns-ok/3007/4 "2024-01-26T18:17:46Z")

</div>

For 1, you actually want `@compileError`. As pointed out, if the return type is non-`void`, then the compiler will give you an error anyway, but if the return type is `void`, then you can use `@compileError` to get the desired behavior:

```zig
pub fn main() void {
    foo(true);
    foo(false);
}

fn foo(bar: bool) void {
    if (bar) {
        return;
    } else {
        return;
    }
    @compileError("this should be unreachable");
}

```

If you remove the `else` branch or forget to `return` from one of the branches, then you’ll get the compile error:

```zig
comptime-unreachable.zig:12:5: error: this should be unreachable
    @compileError("this should be unreachable");
    ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

```

Note that this also works within blocks (this is a [simplified version of this code](https://github.com/squeek502/resinator/blob/b59b3a25b5ae3633a9d9437bc70db6384f04d483/src/res.zig#L646-L660)):

```zig
    if (first_codepoint == '^') {
        switch (c) {
            '^' => return '^', // special case
            'a'...'z', 'A'...'Z' => return std.ascii.toUpper(@intCast(c)) - 0x40,
            else => return error.ControlCharacterOutOfRange,
        }
        @compileError("this should be unreachable");
    }

```

If I forgot to return from one of the switch cases, then I’d get a compile error.

---

<div class="post-metadata">

**Author:** ![Durobot](https://ziggit.dev/user_avatar/ziggit.dev/durobot/32/447_2.png) [@Durobot](https://ziggit.dev/u/Durobot)\
**Post date:** [January 27, 2024, 6:33am UTC](https://ziggit.dev/t/are-these-patterns-ok/3007/5 "2024-01-27T06:33:18Z")

</div>

> [@squeek502](#):
>
> `@compileError`

Of course. `@compileError` should have been my first choice from the get-go.

(But my functions I had in mind do have to return value or error, so in this particular case I don’t really need `@compileError`, it turns out)

---

<div class="post-metadata">

**Author:** ![cancername](https://ziggit.dev/user_avatar/ziggit.dev/cancername/32/8752_2.png) [@cancername](https://ziggit.dev/u/cancername)\
**Post date:** [January 27, 2024, 2:32pm UTC](https://ziggit.dev/t/are-these-patterns-ok/3007/6 "2024-01-27T14:32:43Z")

</div>

> [@Sze](#):
>
> I think you shouldn’t pre-emptively add `@panic` or `unreachable` at the bottom, because those signal to the compiler “I want a runtime crash if something gets here”

PSA: [`unreachable`](https://ziglang.org/documentation/master/#unreachable) doesn’t signal “I want this to crash”, it signals “I don’t care what happens if code gets here”.

---

<div class="post-metadata">

**Author:** ![Sze](https://ziggit.dev/user_avatar/ziggit.dev/sze/32/496_2.png) [@Sze](https://ziggit.dev/u/Sze)\
**Post date:** [January 27, 2024, 2:37pm UTC](https://ziggit.dev/t/are-these-patterns-ok/3007/7 "2024-01-27T14:37:12Z")

</div>

I meant the crashing part for the panic and the “I made sure this can’t happen” part for the `unreachable`, meaning you should make sure that this doesn’t happen. But the trailing respectively may be confusing.
