rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.96k stars 12.69k forks source link

rustc compiles function that is never used #70333

Open WaffleLapkin opened 4 years ago

WaffleLapkin commented 4 years ago

rustc seems to compile never used functions in traits:

// Note: both trait and fn are private and never used
trait NeverUsed {
    fn never_used();
}

impl NeverUsed for () {
    fn never_used() {
        size_expectation_failed()
    }
}

fn size_expectation_failed() -> ! {
    panic!("size of `slice` must not be less than size of `Self` to ref cast to success")
}

Generates a lot of "panic" asm (godbolt).

I expected not to see unused functions in asm, but they are there.

This also happens when using lib with a similar (never used) trait.

Steps to reproduce

  1. create lib and bin crates: cargo new --bin neverbin && cargo new --lib neverlib
  2. fill ./neverlib/src/lib.rs with:

    trait NeverUsed { 
       fn never_used(); 
    } 
    
    impl NeverUsed for () { 
       fn never_used() { 
           call_panic() 
       } 
    } 
    
    fn call_panic() { panic!(\"ooops\") }
  3. add dependency to bin crate:
    # Cargo.toml
    [dependencies]
    neverlib = { path = "../neverlib" }
  4. install & run cargo asm for bin crate, you'll see something like that:
    $ cargo asm
    <() as neverlib::NeverUsed>::never_used
    <T as core::any::Any>::type_id
    <std::panicking::begin_panic::PanicPayload<A> as core::panic::BoxMeUp>::get
    <std::panicking::begin_panic::PanicPayload<A> as core::panic::BoxMeUp>::take_box
    core::ops::function::FnOnce::call_once{{vtable.shim}}
    core::ptr::drop_in_place
    neverbin::main
    neverlib::call_panic
    std::panicking::begin_panic
    std::rt::lang_start
    std::rt::lang_start::{{closure}}
  5. Note never_used and call_panic functions, all the panic! stuff
  6. this behaviour also holds with lto

Meta

rustc --version --verbose:

rustc 1.43.0-nightly (564758c4c 2020-03-08)
binary: rustc
commit-hash: 564758c4c329e89722454dd2fbb35f1ac0b8b47c
commit-date: 2020-03-08
host: x86_64-unknown-linux-gnu
release: 1.43.0-nightly
LLVM version: 9.0

P.S.

I may misunderstand something, and maybe I've forgot to turn on some optimizations, so correct me if I'm wrong. But this behaviour seems strange.

Mark-Simulacrum commented 4 years ago

cc @rust-lang/wg-mir-opt perhaps?

eddyb commented 4 years ago

Here it is on playground - you can press the ASM button etc.

I forget why the fact that these are actually unused, doesn't factor into whether this should compile or not, I wonder if this, once upon a time, fixed impl Trait.

The problem with traits is it's almost impossible to figure out where they might be used from. You can write code that would have the effect of a downstream crate needing <() as NeverUsed>::never_used, and the MIR for that isn't serialized, so it has to exist in the original crate.

What's weird is adding #[inline] to the impl method doesn't fix this, even though it should IMO.

Sadly I don't know these aspects of monomorphization, cc @michaelwoerister @Zoxc

eddyb commented 4 years ago

Ah, for some reason both functions need #[inline].

@WaffleLapkin Out of curiosity, is that a suitable workaround? We should keep this issue open, probably, but a fix might be hard or effectively impossible, so maybe you could use the workaround.

WaffleLapkin commented 4 years ago

@eddyb well, I was trying to copy std's approach with #[inline(never)] panic-fns to reduce code bloat. But since it doesn't work well, I think I'll just move panic!() to the trait's fn itself. That is not so critical to me.

eddyb commented 4 years ago

@WaffleLapkin libstd's approach assumes you will need to panic, it's only a good idea to do that in other situations if you're using LTO (if it even works to remove the panic).

There's even code like this where it switches between #[inline(never)] and #[inline]: https://github.com/rust-lang/rust/blob/55299b2ba99432d10f925cd28ff52fe397577371/src/libstd/panicking.rs#L362-L366

michaelwoerister commented 4 years ago

It might have to do with the reachability pass being rather conservative around methods in trait impls? https://github.com/rust-lang/rust/blob/1add455ec6f81045e7651c6225902823f5d4fbfa/src/librustc_mir/monomorphize/collector.rs#L1008

Enselic commented 10 months ago

Probably because the logic to find unused trait functions are not in place. Also see https://github.com/rust-lang/rust/issues/50927.