Make `unreachable` panic in ReleaseFast, introduce explicit `@optimizeAway()` builtin about zig HOT 8 CLOSED

Outside of comptime, the developer rarely means "Compiler, optimize this code in a way that leverages the fact that this code will never be reached."

On the contrary, this is precisely what I mean when I write an assertion. When I write assert(xyz), I am stating that xyz will always be true. If it is not true in any case, I of course would like to know about that when safety is enabled; but otherwise, I expect the optimizer to utilize that fact when it makes sense. It's no different than any other safety check.

This proposal doesn't just lose the optimizations made possible by certain code paths being unreachable (although that is still itself a major loss); it also requires unnecessary code to be evaluated, even in ReleaseFast. It is not uncommon for me to write code like assert(nonTrivialFunction(foo)) -- I absolutely do not want that function to be called in unsafe optimized builds, because it wastes runtime to check for a bug which I ought to have already caught in debugging. I highly doubt you would propose to make other safety checks apply in ReleaseFast; why should unreachable be any different?

At worst, it's the start of a security vulnerability.

If you are not confident in the correctness of your assertions, you should not run the code in security-critical environments. Assertions are intended to be factual statements about how your code's logic works: an assertion failing indicates a bug which, in all likelihood, could manifest regardless of the assertion. We can't magically guard against incorrect code. It is not appropriate to complicate the language, and hugely limit optimization capabilities, just in order for certain incorrect code to manifest itself in a safer way in release builds.

If you are working in a complex codebase where performance is less critical and there are many trivial assertions, it may indeed be appropriate to create a "custom" assertion function which panics in all release modes. This should not be the default.

[...] the keyword unreachable does not make that clear enough to the developer.

As already mentioned by the commenters above me, the keyword is completely unambiguous. I very rarely see anyone misinterpret its meaning, and when I do, they are virtually always complete beginners to the language. The word "unreachable" in English literally describes "a location that cannot be reached", and that is precisely what the construct models.

There are enough commits in Zig's codebase where developers made this mistake to disprove this.

I highly doubt that many -- if any -- of the examples you brought up were people misunderstanding the keyword. unreachable is not a complex construct, and is widely understood by basically everyone with a reasonable understanding of Zig. Instead, placing unreachable here is a simple logic error: the author thought this code path was unreachable, but it was not. Zig, including its standard library, is v0 software which undergoes rapid change -- there are going to be bugs. That isn't a reason to hinder the optimizer for everyone on the off-chance that someone deploys software without adequate testing in a security-critical environment in a manner where the failure happens to be caught by this mechanism (rather than being a more subtle logic error, or other illegal behavior).

from zig.

Another solution to your use case would be implementing #13339 and compiling Bun with -OReleaseFast+unreachable.

from zig.

If you are not confident in the correctness of your assertions, you should not run the code in security-critical environments.

I'd like to add that if you care about security you should be a) exhaustively testing and fuzzing your code, with runtime safety enabled; and possibly even b) shipping in ReleaseSafe, so that when something does go wrong, you immediately crash.

from zig.

It should be extremely clear to the developer when the compiler will use this information in optimizations.

I would argue the name unreachable is very clear - it means "I guarantee this line will never be reached". If people are using it without understanding what it does, that's a documentation problem.

from zig.

theres also precedent for unreachable to mean this or something very similar, C has __builtin_unreachable() and rust has std::hint::unreachable_unchecked() which is functionally identical to zigs. i think the solution to this is stricter testing and/or using ReleaseSafe which is already planned for zig

from zig.

I would argue the name unreachable is very clear - it means "I guarantee this line will never be reached". If people are using it without understanding what it does, that's a documentation problem.

There are enough commits in Zig's codebase where developers made this mistake to disprove this

theres also precedent for unreachable to mean this or something very similar, C has __builtin_unreachable() and rust has std::hint::unreachable_unchecked() which is functionally identical to zigs. i think the solution to this is stricter testing and/or using ReleaseSafe which is already planned for zig

In both of those cases, unreachable is not a keyword. __builtin_unreachable() is a scary-sounding function. rust's std::hint__unreachable_unchecked() makes it extremely clear that it is unchecked, which is why unreachable! is the default people use instead (which panics)

Note that this is less about Zig's codebase and more about the general problem. We've run into many issues related to unreachable at Bun and now use it very rarely

from zig.

__builtin_unreachable() is a scary-sounding function

i dont really understand your point? i dont see how it being a function instead of a keyword all of a sudden makes it scary

rust's std::hint__unreachable_unchecked() makes it extremely clear that it is unchecked

thats fair but surely we shouldnt shape the language around things that people who are unfamiliar with zig assume about it? zigs unreachable is actually the safest of the 3 here, Cs is completely unchecked and rusts is unchecked in any release mode afaik, zig gives you the choice to keep these things checked while still getting optimizations

from zig.

Recent incorrect usages of unreachable in the Zig compiler

Your very first example is bogus. You're assuming the catch unreachable was an invalid assumption and the commit was to fix it but if you actually look at 0a3dff8 you'll see that's not the case.

Anyway it's moot. This proposal is never going to happen. It destroys the heart of the language.

It sounds like you're using ReleaseFast but you want to be using ReleaseSafe.

from zig.