Are these patterns OK?

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
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:
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
    . . .
}

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

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

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

Output:

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:

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

Write this:

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

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.

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.asserts 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:

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.

4 Likes

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:

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:

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.

4 Likes

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:

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:

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):

    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.

6 Likes

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)

PSA: unreachable doesn’t signal “I want this to crash”, it signals “I don’t care what happens if code gets here”.

1 Like

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.

2 Likes