Indicate `debug_assert` in output of panic

language design
1

We make extensive use of the variants of debug_assert in our codebase. Generally, we have the expectation that our software should never panic and all edge cases should be handled gracefully using Result and the likes.

So naturally, we are very concerned when we see a panic during development. It would greatly reduce our anxiety if debug_assert could add a hint to the resulting panic that it came from a debug assert.

Here is an example (that is also available as a playground):

fn main() {
    debug_assert!(false, "foobar");
    assert!(false, "foobar2");
}

For debug builds the output is the following (with RUST_BACKTRACE=1):

thread 'main' panicked at src/main.rs:2:5:
foobar
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/std/src/panicking.rs:697:5
   1: core::panicking::panic_fmt
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/core/src/panicking.rs:75:14
   2: playground::main
             at ./src/main.rs:2:5
   3: core::ops::function::FnOnce::call_once
             at ./.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Compared to hitting the regular assert! in debug mode:

thread 'main' panicked at src/main.rs:3:5:
foobar2
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/std/src/panicking.rs:697:5
   1: core::panicking::panic_fmt
             at /rustc/29483883eed69d5fb4db01964cdf2af4d86e9cb2/library/core/src/panicking.rs:75:14
   2: playground::main
             at ./src/main.rs:3:5
   3: core::ops::function::FnOnce::call_once
             at ./.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

At no point do we know that the first assert will only occur in debug mode.

We currently fix this by adding [DEBUG ASSERT] to the beginning of each assert message, but that requires discipline / tooling to enforce.

In an ideal world, the output from the panic would indicate that an assert was "just" a debug_assert. I'm imagining something like this:

thread 'main' panicked at src/main.rs:2:5:
triggered `debug_assert`: foobar

In general, this makes me wonder if all asserts (debug and non-debug) should have output that is separate from other panics. Even more ergonomic would be to change the thread 'main' panicked at part of the output, but that is probably difficult to implement, given that assertions are implemented as macros.

1 Like
2

If you add this at your crate root (before all modules), it should work. All you'll have to do is import the macro manually (use crate::internal_macros::debug_assert;).

mod internal_macros {
    macro_rules! debug_assert {
        ($e:expr) => {
            $crate::internal_macros::debug_assert!($e, "assertion failed: {}", stringify!($e))
        };
        ($e:expr $(, $msg:literal $($x:tt)* )?) => {
            ::core::debug_assert!($e $(, concat!("[DEBUG ASSERT] ", $msg) $($x)*)?)
        };
    }

    pub(crate) use debug_assert;
}
1 Like
3

That macro won't let you use shorthand formatting for variables in scope, because it builds the format string with concat!. You can resolve this by passing the formatting arguments to format_args! instead. Something like this:

mod internal_macros {
    macro_rules! debug_assert {
        ($e:expr $(,)?) => {
            $crate::internal_macros::debug_assert!($e, "assertion failed: {}", stringify!($e))
        };
        ($e:expr, $msg:literal $($args:tt)*) => {
            ::core::debug_assert!($e, "[DEBUG ASSERT] {}", format_args!($msg $($args)*))
        };
    }

    pub(crate) use debug_assert;
}

Playground here.

Also, you could couple this with Clippy's disallowed_macros lint to ban the use of regular debug_assert! in your codebase. I don't know if that'll cause problems with it appearing in a macro expansion; if so, you can stick an #[expect(clippy::disallowed_macros)] in @jhpratt's internal debug_assert!, although since you're in an expression context I think that has to look something like

... => {
    {
        #[expect(
            clippy::disallowed_macros,
            reason = "this is the one allowed wrapper of `debug_assert!`"
        )]
        let result = ::core::debug_assert!(...);
        result
    }
}
4

I figured the macro wasn't perfect, but it worked for my trivial cases in the playground. The general idea was there :sweat_smile:

1 Like
5

First of all, thank you both for the comments!

We thought about writing a macro, but we have multiple workspaces with up to 80 crates. Adding a global macro would mean adding a new crate to most of these crates, which is in large parts why we haven't done this yet.

I'd love to hear if other (bigger) projects are running into this as well, and if there is enough interest in this change from the community for a contribution via (pre-)RFC and/or implementation.

1 Like
6

Having "assertion failed:" or "debug assertion failed:" at the start of the message seems like a good idea.

10 Likes
7

That would be great! Maybe I can make some progress.

I briefly skimmed the code library/core/src/macros/mod.rs code. It seems that the match-like assertions (assert_eq, debug_assert_eq) are implemented as library macros.

For these there already is an assertion message: rust/library/core/src/panicking.rs at eec6bd9d69832f57341c6de6a93fa7b9f47e2111 · rust-lang/rust · GitHub. So for these it's just a matter of differentiating between debug and non-debug.

Regular assert! on the other hand is built into the compiler: rust/library/core/src/macros/mod.rs at eec6bd9d69832f57341c6de6a93fa7b9f47e2111 · rust-lang/rust · GitHub and needs changing there.

Because this touches rustc as well, I will cross-post on t-compiler on Zulip too.

1 Like
8

Additional text is not costless. Every piece of added unchangeable boilerplate text can hinder the message making sense in context or conveying the actionable information, and makes people more likely to skim and miss the words that matter.

To me, the difference between assert! and debug_assert! is just the pragmatic decision of whether the check is worth the cost “in production” or not, and it doesn’t change what the assertion failing means; either one equally indicates a bug in the program. Evidently @grtlr is following a different principle, but please don’t make the rest of us fit our programs into that shape.

2 Likes
9

This means larger binaries due to larger strings and more variants of the strings. Large binaries due to panic messages is already an issue for embedded. Please don't make those strings longer than absolutely necessary.

Also, the distinction between debug asserts and normal asserts is meaningless as @kpreid pointed out. Any assert failing is bad, it is just a matter of if you're willing to pay the cost for checking it.

And some of us are willing to pay the cost of a debug asserts in release builds. It is tradeoffs and depend on your threat model. In that scenario it doesn't make sense to print different output for the two types of asserts.

10

I would generally say no. The difference whether something came from specifically the assert! macro or a panic! in a conditional is not important.

If I phrase my "assertion" as

let (chunks, []) = some_slice.as_chunks::<4>() 
else {
    panic!("Input slice must have a length divisible by 4")
};

I don't want complaints that its output is different from

assert!(some_slice.len() % 4 == 0, "Input slice must have a length divisible by 4");

because there's no important difference between them.

(Really, I'd rather encourage the former form of "asserts" because it helps carry proofs in the types.)

8 Likes
11

Couldn't the standard library add trivial wrapper functions (debug_)assert_failed around panic_fmt which would then show up on the backtrace?

That way the error message remains unchanged, but the developer can figure out that it was a (debug) assert. I'd assume the impact on the code-size (or rather debug symbol size) would be small as well (at least if the functions aren't inlineable).

5 Likes
12

It could, but that's a defacto "please all libraries go think about whether your panic is an assert or not" request, which I don't think we want to do.

2 Likes