Asserting non-error

I need to check that a clock is available before I use it:

pub fn run(parsed: cli.Parsed(command)) !void {
    var gpa = std.heap.DebugAllocator(.{}){};
    defer _ = gpa.deinit();
    const allocator = gpa.allocator();

    var io_impl = std.Io.Threaded.init_single_threaded;
    const io = io_impl.io();

    const clock = std.Io.Clock.boot;
    const resolution = clock.resolution(io) catch |err| switch (err) {
        error.ClockUnavailable => {
            std.log.err(".boot clock unavailable but is required", .{});
            return error.NonRecoverable;
        },
        error.Unexpected => {
            std.log.err("unexpected error when querying clock resolution", .{});
            return error.NonRecoverable;
        },
    };
    std.log.warn("clock({s}) resolution: {} ns", .{ @tagName(clock), resolution.toNanoseconds() });

    _ = clock.resolution(io) catch unreachable;
    _ = clock.now(io);
    // clock must work beyond this point.
    ....
}

How can I document my requirement that the clock must be available (via non-error return of clock.resolution) with assert? catch unreachable does say this but I would prefer to use assert if possible to be extra explicit.

I’m hesitant to use an equality comparison because tigerbeetle bans them: tigerbeetle/src/tidy.zig at 6b5a2e9016a02587e929699d84a0ab585b2b46c8 · tigerbeetle/tigerbeetle · GitHub
Secondary question: why does tigerbeetle ban them? What is silent anyerror upcast?

I don’t recommend asserting at all, but to answer the question directly foo() catch assert(false)

In this specific case, you don’t. assert and unreachable are for internal logic. Whenever you’re dealing with an error possibly introduced by something external to your code, you should properly handle that error. Why? Assert indicates to the compiler that such a condition never happens. As such, if another system misbehaves in an unexpected way, your program (if compiled in ReleaseFast or ReleaseSmall) will not notice the problem and continue happily with an invalid program state.

As a simple example, if your program always expects a file to be installed alongside it, your call could still get an error if a user decides to delete that file, or there’s some disk corruption. In the case with your clock, there’s a reason that it can return an error, so you should respect it. (Anytime an Io object is used, this is pretty much the case.)

What proper error handling means depends highly on your program, but it can also be really simple. For “this should never happen” type errors, immediately crashing is my preferred method. For console applications, print and error and exit; For desktop applications, open up an “ok” popup then exit.

Here are a couple of functions that exist in my current project. I’ve changed them a few times, so there’s definitely some options here. I like this version because it’s easy to use (no need to write fancy error messages for things that shouldn’t happen), provides me with stack tracing when I do stupid things in development, and gives at least a little bit of something to go on if a user files a bug report because they’re getting a fatal error. It could probably use a bit more of a wordy error message, eg instructions on how to report a bug, but I haven’t gotten to that point yet.

pub fn check(v: anytype) @typeInfo(@TypeOf(v)).error_union.payload {
    return v catch |err| fatal(err);
}

pub fn fatal(err: anyerror) noreturn {
    if (@import("builtin").mode == .Debug) @panic(@errorName(err));
    std.debug.print("Fatal error {t}.\n", .{err});
    std.process.exit(1);
}

check would be used here like

const resolution = check(clock.resolution(io));

When is catch unreachable the right choice? Only when you know significantly more than the code you’re calling about why “no, that error can’t happen”. An example is if you’re formatting a number into a buffer. If you know for a fact that the buffer is large enough to hold all of the numbers you’re formatting, catch unreachable is reasonable. Overall, catch unreachable should be rarely used, if ever.

And yes, catch unreachable is the correct form here, over using an assert. unreachable is used generally when a codepath can never be hit. For example,

const E = enum { a, b, c };

const e: E = getSomeE();
if (e == .a) {
  return;
}
doSomethingCommon();
switch (e) {
    .a => unreachable,
    .b => handleB(),
    .c => handleC(),
}

switch here is like catch in your question, it’s just control flow.

8 Likes

Oh, and to answer the Tiggerbeetle question: Can you spot what’s wrong with this code? It happily compiles:

pub fn main() void {
    myFunc() catch |e| {
        if (e == error.Foo) {
            // handle foo
        } else if (e == error.Baz) {
            // handle baz
        }
    };
}

fn myFunc() error{Foo, Bar}!void {
   // ...
}

Using a switch instead of using if comparisons,

pub fn main() void {
    myFunc() catch |e| switch (e) {
        error.Foo => {}, // handle foo
        error.Baz => {}, // handle baz
    };
}

fn myFunc() error{Foo, Bar}!void {
    // ...
}

We get an error:

deleteme.zig:7:9: error: expected type 'error{Bar,Foo}', found 'error{Baz}'
        error.Baz => {}, // handle baz
        ^~~~~~~~~
deleteme.zig:7:9: note: 'error.Baz' not a member of destination error set

e== error.Baz is bad, because e is treated as anyerror in the comparison, even if the case is always false. Additionally switch is good because it forces you to handle all of the errors in the set, and only the errors in the set.

8 Likes

This snippet doesn’t do anything besides besides discarding the error.

Yes, they were asking how to assert that no error was returned. catch unreachable works, but they wanted to use assert as they find that more explicit.

But it’s not. Whether there’s an error or not it’ll behave the same.

Edit: Oh! I believe you meant assert(false) instead of assert(true)?

Oooh, oops! :sweat_smile:

fixed it!

I mean it works, but ngl I get strong condition == true vibes. :joy:

I’m confused by your code: after const resolution = clock.resolution(io) catch |err| ... your code now has a non-error value stored in resolution, so what’s the point of running _ = clock.resolution(io) catch unreachable; right afterwards? Why run that function twice if you already have the first return value and know that it didn’t error? It feels similar to typing

if (foo==15) return;
assert(foo!=15);

which I can only imagine being helpful if there was a lot of code in between those two lines, maybe.

I also don’t think you can be sure that clock will work “beyond this point”. resolution will “work” obviously, it’s just a constant, but I doubt that clock.resolution() working once guarantees that all operations on clock will always work from that point on. You have to try it (or catch switch etc) every time.

(as a side note, you might want to guard against resolution==0 - the doc comment on clock.resolution() says “May be zero, indicating unsupported clock”. (It seems odd, why isn’t that an error instead? idk much about Io internals))

nevermind, I’m wrong: zulip. In fact it seems like the clock maybe will work always if it worked once? See also #30171 (not yet merged) which adds Clock.isSupported. (To be clear, I still don’t see a reason for you to assert anything)