new ast-check compile error: control flow implicitly returns from non-void function

[andrewrk]

Functions end with an implicit return; which implicitly returns a void value. Therefore, functions which return void have no requirement to have an explicit return expression:

fn foo(c: bool) void {
    if (c) {
        bar();
    }
}

However functions which do not return void require a return statement or there will be an error such as this:

fn foo(c: bool) i32 {
    if (c) {
        return 1234;
    }
}

./test.zig:1:21: error: expected type 'i32', found 'void'
fn foo(c: bool) i32 {
                    ^

Currently this error occurs during type checking, and so it won't trigger if the function is not referenced. It also will allow something like this:

const MyVoid = void;
fn foo(c: bool) MyVoid {
    if (c) {
        bar();
    }
}

Again, no error on this one.

This proposal is to add an additional check into zig ast-check (also known as AstGen). It makes the rules a bit stricter; functions which do not have a trivial void return type (literally the identifier void as the return type; no detection of aliasing) are required to terminate the function body block with an expression of type noreturn. This could be return or it could be, for example, unreachable, or it could be a while(true) {} loop.

The above example (with MyVoid) would become:

./test.zig:6:1: error: control flow implicitly returns from non-void function
}
^

This compile error is directly related to all the unreachable code compile errors done by ast-check.

---

Additionally accepted amendment to the proposal

[ifreund]

I think we should amend this proposal to allow implicit returns in functions with return type noreturn. For example:

fn fatal() noreturn {
    std.log.err("something bad happened, can't recover\n", .{});
    std.os.exit(1);
}

[andrewrk]

Ah, the existence of noreturn functions complicates this a bit, will remove the accepted label to think about it some more. Without type-checking, zig must assume that any function might be a noreturn function.

fn foo(c: bool) i32 {
    if (c) {
        return 1234;
    }
    exit(1);
}

This code should be allowed, for example.

[rohlem]

the existence of noreturn functions complicates this a bit

Here's a proposal an idea: Require the user to explicitly mark noreturn function calls by following them with unreachable:

fn foo(c: bool) i32 {
    if (c) {
        return 1234;
    }
    exit(1);
    unreachable; // this clearly communicates intent, to both early compiler stages, and readers!
}

I think that adequately solves this problem?
Though honestly, I would really like to advocate for this (if proposals were still allowed) from a stylistic viewpoint alone.

[ifreund]

@rohlem then you don't get a compile error at the call site if, for example, the return type of exit() is changed to void.

[rohlem]

@ifreund Maybe we could introduce such a compile error?
To me unreachable sounds like the natural tool for signaling unreachable code.
IIUC, you would suggest something to the effect of @compileErrorIfReachable() (obviously with a better name)? - EDIT: Here I'm naively hoping that evaluating its reachability analysis could be sufficiently postponed, don't know about the hurdles of implementing that.

[ifreund]

IIUC, you would suggest something to the effect of @compileErrorIfReachable() (obviously with a better name)?

We have this, it's called @compileError() :P

[ifreund]

I'm naively hoping that evaluating its reachability analysis could be sufficiently postponed, don't know about the hurdles of implementing that.

Yeah, that's not how things work in practice. Unreachable code errors MUST happen in AstGen (i.e. before semantic analysis) as branching at comptime allows arbitrary code to be only reachable when targeting a certain OS for example.

[rohlem]

Unreachable code errors MUST happen in AstGen

@ifreund Why can't we emit a simple "unreachable poison" AstNode (-> ZIR -> AIR)? (Special-case it if normal unreachable is not fit for this use case.)
Later in AIR, we know types and can determine whether to discard it (because we're in a dead code path) or error at this later stage.

I realize that this means the problem isn't isolated to be fully-verified during AstGen.
However, it would still allow the proposed ast-check to be implemented, with later validation whether the user-provided unreachable turned out true.
If it didn't, our function's missing a return value, so that's another problem we already need to deal with.
(If the user wanted no return value, that should be written as return undefined; anyway.)

---

EDIT: If what I'm suggesting here is basically already implemented or planned in later stages anyway, then you can ignore these details and I'll yield to my original suggestion that I'd prefer noreturn statements (except for explicit return) to be explicitly followed by unreachable; statements.

[rohlem]

Ah, the existence of noreturn functions complicates this a bit, will remove the accepted label to think about it some more. Without type-checking, zig must assume that any function might be a noreturn function.

I believe this would be unblocked by #17234 .