link-dead-code limitations (bad for code coverage)

[malbarbo]

Consider a cargo project (dead) with this src/lib.rs file:

pub fn fun0() {}

pub fn fun1<T>() {}

pub trait Trait {
    fn fun2(&self);
    fn fun3(&self) {}
}

impl Trait for () {
    fn fun2(&self) {}
}

Running

cargo clean && RUSTFLAGS='-C link-dead-code' cargo test && cargo sym -C target/debug/dead* | grep dead::

outputs

0000000000015e60 dead::fun0::he72896c4e971b738
0000000000015e70 <() as dead::Trait>::fun2::h21fa8b5ef4270ce1
0000000000015e90 dead::__test::main::h824aaf01f68ecd0a

We can observe that that only fun0 and <() as Trait>::fun2 is included (also checked with nm and running kcov). Not including fun1 and Trait::fun2 makes code coverage inflates (using kcov).

I can see that fun1 and Trait::fun2 is not included because they need to be monomorphised. One partial solution would be to monomorphise the default methods implementations for every type that implements the trait, but I think this could blow the binary size and compilation time.

Would it be possible to add this symbols without monomorphisation? Or maybe creating a monomorphised version that just panics? (Could all generic paramaters be set to !?)

[clarfon]

One improvement for this that I can see would be monomorphising all of the methods and trait impls for a type when it's instantiated, rather than just the necessary methods. This would solve most of these problems, I think.

[JelteF]

@clarcharr I think that would be a great intermediary solution, because it would catch all the cases I currently have. Even though only doing this for methods and trait impls of instansiated types would not fix this fully, because generic functions and generic types that are never instansiated would still have this issue (but I think those are quite rare).

@malbarbo I'm not sure about binary size, but because this would normally only be done for test builds I don't think it should be an issue for a first version. I have one possible optimization to avoid excessive blowing up of the binary though. There could be kept track of which methods are not monomorphised at all and only monomorphise them for one of the types that they could be monomorphised for.

[Mark-Simulacrum]

Looks like the problem here is (in general) that we don't use the -Clink-dead-code anywhere except for the direct arguments to the linker. We should probably also utilize it in the collection for trans so that we generate the LLVM IR for dead code. One potential problem is that, IIRC, MIR has a dead code elimination pass which would also need to read this, and might cause problems later down the line.

OTOH, dead code doesn't really need to be tested -- so perhaps this isn't really a problem.

[malbarbo]

OTOH, dead code doesn't really need to be tested -- so perhaps this isn't really a problem.

You maybe be writing a library and forget to test a generic function. It is dead code in the library but a client may use the (untested) function.