Unimplemented!() should cause an error on compiling for release

language design
1
  • Running cargo build with the release flag (or similar) should throw an error and refuse to compile if there is an unimplemented!()/todo!()/similar in the code.
  • It should be possible to override with something similar to #![ignore_unimpl] (useful for shipping to beta testers)
  • For debug builds, it only throws an error if it reaches the line in which the unimplemtned!() / todo!() was thrown.

Alternate option:

Introduce another macro/separate syntax that forces the compiler to error out when compiling for release (Could potentially collide with pre-existing code that has the keyword as a variable name?)

This feature would be really useful to me as somebody who likes to write libraries (not seriously, for fun... but I need to make sure that all of my release code is implemented fully)

1 Like
2

I disagree that “compiling for release” should have any effects on errors. Often times, you don’t “compile for release” when you release code and also often time you do “compile for release” when you don’t release code. There’s no concept of actually telling cargo something like “now I’m done, please check more things because I want to release this code as it it”. cargo build --release mostly just controls the optimization level.

So the right approach to help avoid unimplemented or todo in releases might be a warning. You wouldn’t be the first one to suggest something like this, but AFAICT there isn’t necessarily 100% consensus yet either on if warnings on this are wanted.

In the meantime, you can use clippy to help you. It features optional allow-by-default restriction lints against todo! and against unimplemented! that you can enable with something like #![warn(clippy::todo, clippy::unimplemented)] at the top level of your crate.

31 Likes
3

We certainly shouldn't change the behavior of the existing macro.

You can make your own though, something like this:

#[cfg(debug_assertions)]
macro_rules! new_unimplemented {
    () => { unimplemented!() }
}

#[cfg(not(debug_assertions))]
macro_rules! new_unimplemented {
    () => { compile_error!("unimplemented") }
}

fn main() {
    new_unimplemented!();
}

$ cargo run
[...]
thread 'main' panicked at 'not implemented', src/main.rs:12:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

$ cargo run --release
[...]
error: unimplemented
  --> src/main.rs:8:13
   |
8  |     () => { compile_error!("unimplemented") }
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
12 |     new_unimplemented!();
   |     --------------------- in this macro invocation
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

error: could not compile `tmp`

To learn more, run the command again with --verbose.
15 Likes
4

BTW, Rust really needs to get a proper cfg for the debug profile, because debug_assertions can be enabled in the --release mode.

15 Likes
5

I assume the lack of detecting profiles in code is purposeful, there's no reason code should change based on it when there are more purposeful cfgs to use like debug_assertions, which I do have globally enabled in my release profile because I'm happy to pay the additional runtime cost on any rust utilities I install.

2 Likes
6

If you don't want to abuse debug_assertions, you could use a build script that writes your own cargo:rustc-cfg=something based on the PROFILE environment variable.

1 Like
7

@steffahn that is why I said that it should be easy to override (#![ignore_unimpl]), but good points on the warnings! I think that is a much less intrusive way to go about it, but a little counterproductive maybe if Rust's whole idea is to create code that is stable as possible? :thinking:

But I do agree with @cuviper, that is something that I completely missed :man_facepalming:. There should be a new macro or (preferably, maybe?) special syntax that stops compilation when using --release. The flag's name itself implies that the code is being compiled for a production environment instead of a development environment.

8

I personally tend to think that calling cargo clippy is the way (or at least a way) to tell the rust compiler / tooling “I might want to release this code soon, please tell me everything that might be wrong with it right now”.

Just the --release flag doesn’t mean “releasing” IMO. You’ll use it for benchmarks, too, and users of your application use it implicitly when running cargo install, and when I push code to github or crates.io, especially a simple library, I might do cargo test and call clippy and rustfmt perhaps, but I don’t necessarily do a --release build, because – in my mind – that’s not supposed to make a difference.

Thinking about this some more, there might be some merit in being able to set up different “warning profiles” or something along the lines so that you can easily switch between different sets of enabled or disabled warnings. This would provide an easier way to get distracting warnings, particularly about unused code, out of the way during development or refactors while guarding the possibility to accidentally leave those warnings disabled forever (because you forgot about some #![allow(…)] somewhere in your code).

6 Likes
9

Not necessarily. We deploy with debug builds because we want useful backtraces and performance is decent enough without release as well. In my mind, debug vs. release is more about optimization levels rather than target use cases.

1 Like
10

unimplemented!() is intended for leaving things unimplemented. todo!() is intended for things you have yet to implement. by this definition, build-erroring on unimplemented!() is wrong.

(you could use unreachable!() but, that's not meant to be exposed at API boundaries. so please don't.)

2 Likes
11

@mathstuf You can set enable debuginfo for the release profile too. debug=1 would be enough for useful backtraces. It basically includes lineinfo and inlining info. debug=2/debug=true also includes debuginfo to access locals, but those aren't included in the backtraces anyway. It is also possible to enable optimizations for the dev profile. The release and dev profiles have defaults that make sense for development cq releasing software. You can change them however you want or (on nightly) create custom profiles from scratch.

12

I think the issue is just that "release" is a bad name, because it suggests something about the software deployment process, when as others have said the actual intent is to control optimization level. Some real-time software like games run so slow in debug builds that they are completely unusable so even in development people will always run with release (to be clear this is not a Rust specific problem this happens in C++ as well). Going the other way sometimes you might want simpler assembly to be deployed for debugging purposes the next time an impossible to reproduce segfault occurs in production so you deploy an unoptimized build on purpose. In my experience any setting that affects codegen can change whether a crash is reproducible, so theoretically you can want any compiler setting in a build you "release."

Also in general enabling debug information in a optimized build is not equivalent to enabling debug information in an unoptimized build. Things like -Og try to tow the line but still end up erasing some information compared to an unoptimized build, and if you are unlucky that might be the information you need to diagnose your bug.

3 Likes
13

as an example of where unimplemented!() would be useful in release: Feature request: std::fmt::Write::write_line - #3 by Soni

1 Like
14

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.