Warning-free builds?


#1

tl;dr: Is it currently possible to get a warning-free build, including cargo test, without introducing potentially sketchy annotations? Who’s doing it?

I love compiler warnings. I love static analysis and anything I can do to catch errors quicker. So, when working in seriously foot-shooty languages like C++, I tend to crank the warnings really high, set the equivalent of -Werror, and eliminate all the warnings. (I even do this in Haskell, which has some notoriously promiscuous warnings.)

Occasionally, I have to profane my source code with alterations to dodge such warnings, but I try to avoid disabling warnings outright using #pragma and similar mechanisms, because they’re rarely precise: they always feel like they open the door to future rot, undetected by the compiler.

So, that being said:

I’m learning Rust, and I’m surprised by the number of warnings I’m getting that seem… spurious, particularly in test builds. These are almost always dead_code. I wonder if there are tricks/idioms I’m missing that I could use to get a warning-free build?

main is reported as dead code in a test build, as is everything only directly called by main. (This is the subject of an open bug).

Imports used only by tests are reported as redundant in non-test builds, but I can work around this by by declaring a tests module marked with #[cfg(test)] and putting the imports inside it. (IMHO this could use a louder shout-out in the testing chapter of the book.)

Any functions not called from tests are also reported as dead, which I guess is the compiler suggesting I increase my test coverage. :slight_smile: I don’t want to slap them with #[allow(dead_code)], because if they go dead in the non-test build I do want to hear about it. I was surprised that this does not seem to work:

#[cfg_attr(test, allow(dead_code))]

…which seems like the most precise way of saying to the compiler, “tolerate this being unused in a test build.”

What else should I be doing?

Thanks in advance.


#2

That cfg_attr should work (it’s a bug if it doesn’t), could you post the code that fails?


#3

If you want to turn off the warning for your whole crate or module, you might need to add a !:

#![cfg_attr(test, allow(dead_code))]

Or instead of just disabling the warning, you can disable compilation of any item that’s not used in your tests, by marking it with:

#[cfg(not(test))]

#4

Played with it more – it’s specifically not working for functions defined in non-public modules that have no uses outside their module.

Definitely dead code… likely to not happen in a fully constructed system… except I really expected cfg_attr to work here.

Simplified version of my code in a gist

So in this case I’m willing to accept fault.

What about in general, though? Are people expecting warning-free builds? Are typical projects annotating e.g. main with #[cfg(not(test))] or #[allow(dead_code)]?


#5

If you mix library code and test code together in the same source file, then yes, you need to annotate things with #[cfg(not(test))] and #[cfg(test]).

If you follow the standard Cargo convention, of putting tests in tests and library source in src/lib.rs and other modules within there, and binary code in src/main.rs or src/bin/*.rs, then it will build the appropriate files for the appropriate task, and you won’t need to explicitly annotate.


#6

Ah, there’s a slight trick there: cargo test will build a lib target as a library (to use in doc tests and external tests/examples) and as a test runner, in parallel by default. So those warnings are actually coming from a plain build i.e. without cfg(test), running cargo build (after a cargo clean if necessary) will display them too.

Just to emphasise the phases, running cargo test -j1 --verbose to run sequentially and noisily gives:

   Compiling warn v0.1.0 (file:///home/huon/projects/test-rust/warn)
     Running `rustc src/lib.rs --crate-name warn --crate-type lib -g --out-dir /home/huon/projects/test-rust/warn/target/debug --emit=dep-info,link -L dependency=/home/huon/projects/test-rust/warn/target/debug -L dependency=/home/huon/projects/test-rust/warn/target/debug/deps`
src/lib.rs:2:3: 2:31 warning: function is never used: `tested`, #[warn(dead_code)] on by default
src/lib.rs:2   pub fn tested() -> i32 { 0 }
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/lib.rs:5:3: 5:33 warning: function is never used: `untested`, #[warn(dead_code)] on by default
src/lib.rs:5   pub fn untested() -> i32 { 0 }
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     Running `rustc src/lib.rs --crate-name warn --crate-type lib -g --test -C metadata=5f9e59bcd7228ea2 -C extra-filename=-5f9e59bcd7228ea2 --out-dir /home/huon/projects/test-rust/warn/target/debug --emit=dep-info,link -L dependency=/home/huon/projects/test-rust/warn/target/debug -L dependency=/home/huon/projects/test-rust/warn/target/debug/deps`
     Running `/home/huon/projects/test-rust/warn/target/debug/warn-5f9e59bcd7228ea2`

running 1 test
test sub::tests::test ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured

   Doc-tests warn
     Running `rustdoc --test /home/huon/projects/test-rust/warn/src/lib.rs --crate-name warn -L dependency=/home/huon/projects/test-rust/warn/target/debug/deps --extern warn=/home/huon/projects/test-rust/warn/target/debug/libwarn.rlib`

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured

In particular there’s two Running rustc ...s, the first one is a normal build of the lib (with the warnings), the second is for the test runner (there’s a --test flag).

As you noted, the dead_code warnings are occurring because of the non-pub mod: no users of that library can call those functions since they’re in a private module. Changing mod sub to pub mod sub stops all the warnings (alternative fixes include reexporting the functions individually, or calling them in public functions).


#7

Thanks, Huon! Nice diagnosis.


#8

@lambda, I don’t think that’s true.

Try this:

$ cargo new --bin warn
$ cd warn
$ cargo test

It warns that main is unused. So (I assume) some explicit annotation must be necessary to get a warning-free test build.


#9

@cbiffle

Using #[cfg_attr(test, allow(dead_code))] directly before fn main() should remove the warning for the main function specifically. That seems to work for me with binaries with tests. If that isn’t working for you, could you possibly pastebin the file?

Note that #[cfg_attr(test, allow(dead_code))] will only have an affect if used directly on the code with the warning - it won’t work at the top of the file (for that, use #! instead of #).


#10

I’d suggest to use #[cfg(test)] instead of #[cfg_attr(test, allow(dead_code))].


#11

Thanks @daboross. My question was whether people are annotating their main functions in practice, and it sounds like you are. So that’s what I’ll do.


#12

So in a case where…

  • A subset of items in a module are covered by Rust/Cargo #[test] unit tests,
  • The rest are actively used by the non-test build of the system, but
  • That code is only tested in integration tests not visible to the test build,

you’d all agree that what I’m doing here is typical for Rust, and I haven’t missed something obvious?

Sorry for belaboring the point here, I’m still adjusting to how things are done here. Having to annotate individual struct fields that are used but not by the tests particularly surprised me.


#13

Wouldn’t putting #![cfg_attr(test, allow(dead_code))] in the topmost module and then (optionally) #![warn(dead_code)] in the tests modules achieve roughly the same?


#14

Interesting point! I don’t see why not.

I’m surprised by the number of suggestions I’m getting – I guess we’re too early in the language’s life to have a best practice here?


#15

@tbu

The only reason I suggest #[cfg_attr(test, allow(dead_code))] is because using #[cfg(not(test))] is that #[cfg(not(test))] requires annotations on all now-unused use statements as well.

@cbiffle

Just looked at the commit/code base you linked.

I think you are solving the wrong problem at the moment - I would suggest a bit of refactoring. In your current setup, you have everything as a sub-module of main.rs - this is a bit of an anti-pattern in rust.

I would suggest that instead you move most if not all of your modules to submodules of a separate lib.rs file, and expose them as public things in that crate. Then, in main.rs, you can access all public items from your lib.rs with extern crate httpd.

This different way of organizing is the current idiomatic way of solving the problem you have. It doesn’t require any changes to Cargo.toml or other configuration files, Cargo will automatically figure out you have a library and a binary and link them together.

Using this approach, none of the warnings you are suppressing right now will actually occur, because anything you use in main.rs will need to be a public item in your library file, and thus is assumed used by rust.


#16

@daboross As mentioned before, separating out components to library doesn’t (necessarily) work. cargo test still builds the binary in addition to the lib.

Try the following and you’ll still get a “warning: function is never used: `main`”

cargo new proj
cd proj
mkdir src/bin
echo 'extern crate proj;' >> src/bin/main.rs
echo 'fn main() { println!("hello"); }' >> src/bin/main.rs
cargo test

#17

@daboross - actually, I only recently stopped using lib.rs – it initially seemed like the right way to go, because I tend to factor programs into reusable pieces by default. But it felt like unnecessary code to maintain, and weakened the dead_code warnings for the actual binary (not test) build, because (as you note) rustc assumed that pub items are used.

I suppose I need to decide which of these two problems I want to solve. :smile:


#18

@gkoz, I like the way your suggestion came out. It will warn me if I forget a #[test] annotation in the tests module, but not hassle me about the other bits (which, if dead, will warn in the primary build – because I haven’t split the code into a lib crate).


#19

It seems that marking fn main with #[cfg_attr(test, allow(dead_code))] also exempts anything transitively referenced from main (at least within the same binary) as alive. Placing #![warn(dead_code)] or #![cfg_attr(test, warn(dead_code))] within the submodules doesn’t turn the warning back on.

So I think I have the following options:

  1. Keep everything in a single build, mark main as allow(dead_code), lose dead code warnings everywhere in test builds but retain them in normal builds (this may be preferable).

  2. Simply ignore warnings from test builds. This unnerves me, but there might be something to it.

  3. Tolerate one dead code warning (on main) in test builds, wait for corresponding bug to be fixed.

  4. Move most of the code back into a separate crate with a lib.rs, mark functions as pub, lose dead code warnings for anything they transitively reference in both normal and test builds (my least favorite).


#20

I dislike compiler warnings in final/stable release. In development process (before commit), i do not mind. I currently write rather smaller (few hundred lines + helping mods) agent simulations in Rust. But i find it easy to write warning-free projects of any size in Rust.

I find dead code warnings very interesting. They inform you that there are no tests and this probably should be enforced. Maybe the warning message could be changed slightly. :wink: