Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustc compiles function that is never used #70333

Open
WaffleLapkin opened this issue Mar 23, 2020 · 7 comments
Open

rustc compiles function that is never used #70333

WaffleLapkin opened this issue Mar 23, 2020 · 7 comments
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@WaffleLapkin
Copy link
Member

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.

@WaffleLapkin WaffleLapkin added the C-bug Category: This is a bug. label Mar 23, 2020
@Mark-Simulacrum Mark-Simulacrum added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 23, 2020
@Mark-Simulacrum
Copy link
Member

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

@eddyb
Copy link
Member

eddyb commented Mar 23, 2020

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
Copy link
Member

eddyb commented Mar 23, 2020

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
Copy link
Member Author

@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
Copy link
Member

eddyb commented Mar 23, 2020

@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]:

// If panic_immediate_abort, inline the abort call,
// otherwise avoid inlining because of it is cold path.
#[cfg_attr(not(feature = "panic_immediate_abort"), track_caller)]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[cfg_attr(feature = "panic_immediate_abort", inline)]

@michaelwoerister
Copy link
Member

It might have to do with the reachability pass being rather conservative around methods in trait impls?

|| self.tcx.is_reachable_non_generic(def_id)

@Enselic
Copy link
Member

Enselic commented Dec 10, 2023

Probably because the logic to find unused trait functions are not in place. Also see #50927.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants