std lib: add std.debug.assertDebug

[hazeycode]

TIL that std.debug.assert generates panics in ReleaseSafe and generates undefined behaviour in ReleaseFast and ReleaseSmall, causing the optimiser to eagerly eliminate entire branches. Until now I have been assuming that it behaved like C's assert macro. This is a very bad assumption to make!

This PR adds a new function std.debug.assertDebug which is functionally equivalent to C's assert macro when used in conjunction with NDEBUG.

It can be used in cases where the user wants to quickly catch mistakes or broken assumptions during development but doesn't want to affect execution in any way in release modes. This is very useful in some domains, such as live music performance systems and video games (which happen to be the 2 domains I am currently involved with).

[nektro]

the assert(x) macro in C uses the noreturn attribute and likely achieves a very similar behavior given that @TypeOf(unreachable) == noreturn in Zig.

musl:
https://git.musl-libc.org/cgit/musl/tree/include/assert.h

glibc:
https://sourceware.org/git/?p=glibc.git;a=blob;f=include/assert.h
https://sourceware.org/git/?p=glibc.git;a=blob;f=assert/assert.h

[hazeycode]

the assert(x) macro in C uses the noreturn attribute and likely achieves a very similar behavior given that @TypeOf(unreachable) == noreturn in Zig.

Not when NDEBUG is defined

/* void assert (int expression);

    If NDEBUG is defined, do nothing.
    If not, and EXPRESSION is zero, print an error message and abort.  */

#ifdef  NDEBUG

# define assert(expr)           (__ASSERT_VOID_CAST (0))

...

[Vexu]

TIL that std.debug.assert can generate UB and cause the optimiser to eliminate successive code in release modes.

Can you give an example? At most the optimizer should remove the branch completely.

[hazeycode]

TIL that std.debug.assert can generate UB and cause the optimiser to eliminate successive code in release modes.

Can you give an example? At most the optimizer should remove the branch completely.

Here is a contrived example: https://godbolt.org/z/oYo4741Pd

Aside, I think I have done a poor job of describing the behaviour, sorry. I will update the PR description.

[Vexu]

Relevant issue #8901

[xdBronch]

that godbolt link isnt actually related to the assert, its because log.info is a no-op in ReleaseFast, the optimizer sees that there are no side effects and just removes the call to foo https://godbolt.org/z/aTzWeTKKz, here is an actual problem that is essentially what vexu linked https://godbolt.org/z/GY6TeYea4, it doesnt even return

[hazeycode]

Thanks guys!

I think the more generally reaching UB optimisation problem is somewhat orthogonal to what I'm trying to address here, which is that there ought to be a clear, standard way to make Debug-mode-only assertions that simply evaporate and have no effect what-so-ever in any release mode.

I might go a step further to suggest that debug.assert should be renamed to debug.assertStrong or something.

[Vexu]

Adding a new function to workaround a bug will result in a redundant function once the bug is fixed. Instead this should replace the current assert with a link to #8901 in a comment.

[Vexu]

Suggested change

	if (builtin.mode == .Debug) {
	if (runtime_safety) {

[hazeycode]

Debug only is intentional. No code should be generated in ReleaseSafe

[hazeycode]

Adding a new function to workaround a bug will result in a redundant function once the bug is fixed. Instead this should replace the current assert with a link to #8901 in a comment.

I don't think this would become redundant. Even when the big is fixed, the existing assert will panic in ReleaseSafe by design right? The use case here requires no code to be generated in any mode that is not debug. Or did I misunderstand?

[InKryption]

As is the case with C's assert macro, one should not rely on the expression being asserted to have been evaluated.

Why do you say this? There are no macros in zig, the function will evaluate its arguments just as all other functions do. One should expect for any expression control flow reaches to be evaluated, including expressions passed directly as arguments to function calls, even if the function is inlined and verifiably doesn't use the result.
The compiler may optimize away expressions which have no side effects, but they will be evaluated - if it optimizes something away which had side effects, that would be a bug.

[hazeycode]

As is the case with C's assert macro, one should not rely on the expression being asserted to have been evaluated.

Why do you say this? There are no macros in zig, the function will evaluate its arguments just as all other functions do. One should expect for any expression control flow reaches to be evaluated, including expressions passed directly as arguments to function calls, even if the function is inlined and verifiably doesn't use the result.

The compiler may optimize away expressions which have no side effects, but they will be evaluated - if it optimizes something away which had side effects, that would be a bug.

My mistake. You're right, zig shouldn't optimise away the expression if it has side effects. I will remove this statement from the description. Thanks for pointing this out!

[andrewrk]

I respectfully disagree with this being an advisable way to create software, and so this will not go into zig's standard library. I suggest for you to put this function into your own application.

[hazeycode]

I respectfully disagree with this being an advisable way to create software, and so this will not go into zig's standard library. I suggest for you to put this function into your own application.

From a purist POV, I agree. But as this is a fairly common practise in certain domains in the wild, I think this decision will empirically result in worse software being created, if others also assume debug.assert works in a similar way to C's assert macro.

Perhaps moving assert to a namespace other than debug would nullify this concern.

[andrewrk]

Here is a contrived example: https://godbolt.org/z/oYo4741Pd

I don't think your example demonstrates what you think it does. Here is a fixed example: https://godbolt.org/z/zjd4cWdda

As you can see, commenting out the assert does nothing. Also, commenting out the assert in your example does nothing. What you are observing is that ReleaseFast defaults to not printing info messages. In retrospect that's probably not a good default, but it has nothing to do with assert.

Edit: I see that others addressed this already - apologies for not reading the whole thread before commenting.

Anyway, I'm not budging on this one. If your asserts are not rock solid, then compile those asserts with ReleaseSafe.

[hazeycode]

Anyway, I'm not budging on this one. If your asserts are not rock solid, then compile those asserts with ReleaseSafe.

Fair enough. But I should point out that compiling the asserts with ReleaseSafe does not satisfy the use-case.

Having thought about it more, I don't think I really want to panic at all. I think the use case would be better served by some portable equivalent of MSVCs __debugbreak() if some expression is false in Debug mode. Does something like this exist?

What you are observing is that ReleaseFast defaults to not printing info messages. In retrospect that's probably not a good default

Agree that's not a good default

[nektro]

@breakpoint(); note that zig's unreachable also trip debugger symbols when attached

https://ziglang.org/documentation/master/#breakpoint
https://ziglang.org/documentation/master/#trap

[hazeycode]

@breakpoint()

Thanks!