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

std.debug.assert should not be special-cased in test mode #1304

Closed
andrewrk opened this Issue Jul 30, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@andrewrk
Copy link
Member

andrewrk commented Jul 30, 2018

Here's the current implementation of std.debug.assert:

/// This function invokes undefined behavior when `ok` is `false`.
/// In Debug and ReleaseSafe modes, calls to this function are always
/// generated, and the `unreachable` statement triggers a panic.
/// In ReleaseFast and ReleaseSmall modes, calls to this function are
/// optimized away, and in fact the `unreachable` gives extra information to
/// the optimizer to work with.
pub fn assert(ok: bool) void {
    if (!ok) {
        // In ReleaseFast test mode, we still want assert(false) to crash, so
        // we insert an explicit call to @panic instead of unreachable.
        // TODO we should use `assertOrPanic` in tests and remove this logic.
        if (builtin.is_test) {
            @panic("assertion failure");
        } else {
            unreachable; // assertion failure
        }
    }
}

We turn asserts into calls to @panic in test mode so that assert() in tests will be guaranteed to panic in ReleaseFast and ReleaseSmall modes.

@thejoshwolfe pointed out that the point of testing in ReleaseFast mode and ReleaseSmall mode is to test the actual code that will be run in these modes. So instead of changing the behavior of assert in these modes, we should have a different function to call during testing, and keep asserts in the implementation code.

@PavelVozenilek

This comment has been minimized.

Copy link

PavelVozenilek commented Jul 30, 2018

If tests are compiled in (regardless of debug/release mode), then make all asserts active. If they are not, make them no-op. No exceptions*.

Then there is one special case, when you intentionally stress-test the code in a way that triggers reasonably placed useful assert. For this I made special version of assert (called verify), which does nothing if it gets false inside a test. If it is triggered from a normal (non-test) code it fires as expected. If tests are not compiled in, it is no-op.


* If benchmarking support/PGO/etc is implemented via the testing mechanism (#1010 (comment)), then there will be exceptions, however this is advanced feature and whoever does it should be expected to understand its consequences.

@andrewrk

This comment has been minimized.

Copy link
Member Author

andrewrk commented Nov 21, 2018

We're going to have to edit a lot of tests to use std.debug.assertOrPanic before implementing this.

@andrewrk andrewrk closed this in c2db077 Feb 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.