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: disallow comptime return in all functions #22549

Open
mlugg opened this issue Jan 20, 2025 · 0 comments
Open

Proposal: disallow comptime return in all functions #22549

mlugg opened this issue Jan 20, 2025 · 0 comments
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@mlugg
Copy link
Member

mlugg commented Jan 20, 2025

A while ago, I made some changes to the compiler regarding comptime { ... } syntax which led to comptime return being disallowed in runtime functions. This makes logical sense, because you can't return from a runtime function at comptime; that's runtime control flow!

However, this syntax remained allowed for inline functions/calls. At the time, I justified this by the fact that the comptime return value is comptime-known to the caller, so you effectively can do this control flow at comptime.

I have come to realise that my logic was flawed: while the comptime-known result does propagate to the caller, it is not valid to actually perform the control flow at comptime (at least as we implement it today), because this may cause already-analyzed runtime operations to be "skipped", i.e. not actually added to the caller despite having been analyzed. This bug, it turns out, exists in the compiler today:

$ cat repro.zig
pub fn main() void {
    std.debug.print("foo\n", .{});
    inner();
    std.debug.print("qux\n", .{});
}
inline fn inner() void {
    std.debug.print("bar\n", .{});
    comptime return; // the comptime control flow here causes us to lose the runtime operation above
}
const std = @import("std");
$ zig run repro.zig
foo
qux

We could work around this in the compiler; introducing special handling for comptime control flow which pauses to first add instructions to the parent context. However, I believe it would make for a simpler and more logical language specification to simply ban this, as is already the case for non-inline calls.

Note that inline calls are highly analagous to labeled blocks. The equivalent of the above code would be this:

pub fn main() void {
    std.debug.print("foo\n", .{});
    inner: {
        std.debug.print("bar\n", .{});
        comptime break :inner;
    }
    std.debug.print("qux\n", .{});
}
const std = @import("std");

This code actually works today (and prints all three lines of output) due to a quirk in the compiler implementation, but very similar code can easily trigger compiler crashes; this code being accepted is a bug and will eventually become a compile error. I think that's a good demonstration of why the analogous inline fn version should also be a compile error.

@mlugg mlugg added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Jan 20, 2025
@mlugg mlugg added this to the 0.15.0 milestone Jan 20, 2025
@mlugg mlugg changed the title Proposal: disallow comptime return in inline functions Proposal: disallow comptime return in all functions Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

1 participant