conditionally compile contents of debug_assert!

[Emerentius]

The question came up in IRC why you cannot use a function annotated with #[cfg(debug_assertions)] inside debug_assert!() without compile error in release mode. This is because the macro is defined via if cfg!(debug_assertions) { assert!(..) } so it's always compiled but only called in debug builds. It could however just as well be defined as #[cfg(debug_assertions)] { assert!(..) }

If I am not missing anything this would be strictly more powerful. An unfortunate side-effect is that switching this now may (will) cause a flood of unused_assignments warnings in large projects. At the same time it's useful for readability when variables needed only for debugging are clearly marked as such.

Should the change be done? Would this need an RFC? I've started patching up all the warnings in rustc that crop up due to the change but with the amount of them I'd like to know in advance whether this is even desired.

[jonas-schievink]

A problem with this is that code inside debug_assert! will no longer be type-checked when assertions are off, which is a nice reason to prefer if cfg! in user code.

[bluss]

Here's a link to previous discussion, just to find out where in compilation the dead branch is removed. It should be removed in the mir passes, which is before monomorphization and codegen, IIUC, so we already save most of the compilation effort correctly.

https://botbot.me/mozilla/rustc/2017-11-25/?msg=93905257&page=3

[Emerentius]

I don't think compilation effort was a big issue in the first place. debug_assert!() is mostly used with little code snippets. This is more about meeting expectations. It is somewhat surprising that code that is guaranteed to be available in a debug build will still cause an error in release mode when used inside a debug_assert!().

A problem with this is that code inside debug_assert! will no longer be type-checked when assertions are off, which is a nice reason to prefer if cfg! in user code.

cargo check does typecheck with debug_assertions by default both with and without --release as will rls. In general this affects all other code that is conditionally compiled, too.

Edit: Actually it seems that a conditionally compiled function #[cfg(debug_assertions] fn foo(..) {} is typechecked by rls but a code block #[cfg(debug_assertions] { /* code block */ } isn't which would include this.

[hanna-kruppe]

It seems strange to have some definitions available or not depending on debug_assertions (as opposed to, say, target OS or architecture).

[Emerentius]

Some good reasons for keeping status quo came up and in the few cases where you would need conditional compilation you can #[cfg(debug_assertions)] { debug_assert!() } it yourself (or write the changed macro). It would probably be for the best that changes such as this would undergo an RFC anyway purely because of the flood of warnings it would unleash on the ecosystem.

I'm going to close this to reduce issue clutter and the next time the question comes up we can link to this issue.