I’ll take a shot at it. When I see std.debug.assert called, the intent that is conveyed is usually “panic if this condition is false in safe modes, do nothing in optimised modes”. The behavior of passing a side-effecting expression as the argument to assert is not immediately obvious, because it contradicts what I expect the intent of an assert call to be.
Really it seems like a matter of familiarity. If passing a side-effecting expression to assert or other functions that are similarly intended to be optimised away under certain conditions was more common, maybe it wouldn’t be an issue.
i think the status quo behavior is fine and should be kept. given that Zig has first class comptime support, what needs to be changed is messaging that what you should do is something like
const mode = @import("builtin").mode;
const assert = @import("std").debug.assert;
{
// ...
if (mode == .Debug) assert(heavyweightFunction() == .good_value);
// or if (mode != .ReleaseFast) or even switch(mode) ...
// ...
}
heck, you could even do one better and define for yourself a
const include_heavyweight_asserts = mode == .Debug; // and whatever logic you want to include
{
// ...
assert(include_heavyweight_asserts and heavyweightFunction() == .good_value);
// ...
}
I am definitely not suggesting the behavior should change! Just that the current means for expressing that behavior might not be conveying the intent clearly.
The problem with wrapping the assert with if here, is that we actually do want to evaluate the parameter always. Wrapping the assert with if like this only makes sense if you do not want to evaluate the argument in the case that the condition is false, and wish to clearly convey that intent.
Isn’t it common in some languages to never put function calls in asserts or logging. For example in C++ multiple libraries like SPD_LOG or Qt’s use macros to completely remove the asserts or debug messages, so that arguments aren’t evaluated in release mode / if they are under the logging level.
This usually leads to bugs maybe more often in OOP land when a function mutates some internal state of a class in debug builds but is optimized away in release builds, leading to different behaviour.
So it’s common to see the suggestion to never make function calls in asserts, since the assert might get removed
The TL;DR is that if the function has side-effects, so you can safely do this without worrying about it being optimized away if you are relying upon it performing some change of state. This is due to the nuance between C/C++ often using macros for assert, while Zig uses traditional function calls.
I think people coming to zig usually come with the idea that asserts will be optimized away due to their assumptions coming from previous languages.
I think the best way to correct this misassumption would be to have a simple canned response to explain why it is different in Zig. Maybe a super simple blog post
Zig’s assert behavior is indeed different from other languages. Using this pattern more often in the standard library has its benefits: it reminds readers of this unusual behavior of Zig and helps avoid misunderstandings in other, more confusing places.
The one big thing to still be aware of is that the unreachable that remains in Zig’s release-fast mode may cause additional breakage if the assert actually evaluates to false, compared to C where the assert would be entirely removed.
In release-fast mode the assert turns from a panic into manually injected undefined-behaviour which the compiler may ‘exploit’ for additional (but dangerous) optimizations, so the damage if the assert is hit may actually be greater and much harder to debug than as if the entire assert would simply be removed.
IMHO this makes release-fast mode more dangerous to use than it should be.
I definitely don’t share the excitement about the “yay, automatic optimization hints” in all places where assert is used, I’d prefer to decide that on a case-by-case basis via a manually inserted unreachable
Also happy it doesn’t work like C where bugs are introduced if compiled with NDEBUG and asserted code has side effects - which is unsurprisingly a source of bugs in the wild.
That’s why ‘proper’ C/C++ libraries usually don’t directly use the stdlib assert macro, but allow to override it with custom macros. Keeping asserts in release mode is also an entirely valid strategy in C code, just with a bit more nuance (like different macros for ‘should stay in release mode’ vs ‘should be removed in release mode’, adding text messages, hooking up the assert to logging or bug reporting services, or hint a static analyzer (although this seems to be standard nowaways in the stdlib assert macros) - or if you want to, even inject undefined behaviour in release mode
Unchecked illegal behavior just means your program is broken now, and nothing will crash it for you with a panic if that’s the case. Instead, some unspecified thing you did not want will occur.
“Safety checked” means that in safe modes, there will be a panic. So your assume is an assertion, and your assert is something stronger.
“Assume” should be for things which cannot possibly be asserted to uphold the invariant. Example: handing a pointer back to free or destroy. If that pointer doesn’t belong to the allocator, Bad Things will happen, and it isn’t practical to prevent this with an assertion.
No, your two functions are simply the difference between -OReleaseFast and -OReleaseSafe. unreachable specifies intent precisely, and optimization mode decides how to realize that intent.
-OReleaseSafe is precisely the option offered to those with a more conservative mindset about encountering bugs at runtime.
The thing @floooh is arguing in favor of would be fn assert(condition: bool) void { _ = condition;}. I see this as an awkward middle ground. Too cowardly to trust the assertion; too daring to panic. The Zig philosophy as demonstrated by std.debug.assert is to pick a lane.
Related:
So you can more precisely control which different parts of your application have which optimization mode applied.
Not quite. The intent conveyed is to communicate that the condition can never be false. Combined with Debug/ReleaseSafe optimization mode, the behavior is to panic if that assumption does not hold. Combined with ReleaseFast/ReleaseSafe optimization mode, the behavior is for the optimizer to assume that condition is never false, which can have optimization implications.
It is obvious, unless you suffer from trauma from another language and you overthink it. It’s obvious because assert is just a function, like every other function in your entire application, and it does not have different rules that govern it. Would you expect foo(bar()) to delete your call to bar for some reason? No!
The rest of your application uses unreachable all the time, for instance when doing .?, or using @intCast, or accessing a pointer. In these cases unreachable occurs when the value turns out to be null, the integer doesn’t fit in the destination type, or the pointer address is invalid, respectively.
If you want assertions that are more in line with your experience coming from another language, you can write them.
I’ve worked on serious commercial products that define a variety of “assert” functionality for different purposes and specific behaviors. (e.g. a debug break when the check fails, or log and continue). Of course these begin to fall into territory that are only in the ballpark of an assertion in a theoretic sense, so arguably shouldn’t be called a variation on “assert”,
My comment was taken out of context, and I feel like this reply is being made to my comment in isolation. I was not making an argument that the semantics of unreachable or the std.debug.assert function should change or that they’re hard to understand. My argument is that writing in this style:
const bar_result = bar();
foo(bar_result);
Has entirely obvious behavior, even when foo(...) may be a no-op due to comptime conditions like the build safety mode.
When foo(...) is a no-op due to the build mode, it is not immediately obvious what the behavior of
foo(bar());
is. As is evident by the existence of the original question in the other thread, asking if bar() (in this case) would be evaluated. It is not a difficult problem to reason about, but the first style in this comment would not have raised the question, and the second style prompted me to take a few seconds to reason through it.
I don’t honestly care all that much; I think it’s fine to have code written in the style of the second way in the standard library. I would prefer to write code in the first style though, where the behavior just seems unquestionably obvious.
