Rust: Spurious unused import warning with trait used transitively via glob

[jturner314]

Under certain circumstances involving pub(crate) use Trait, the compiler issues an "unused import" warning even though the import is actually being used.

This is an example (playground link):

mod foo {
    pub(crate) use std::ascii::AsciiExt;
}

mod bar {
    use foo::*;

    pub fn bar() -> bool {
        "bar".is_ascii()
    }
}

fn main() {
    println!("{}", bar::bar());
}

Since the body of bar::bar() uses the AsciiExt trait (AsciiExt provides .is_ascii()), there shouldn't be a warning.

Instead, this happened:

warning: unused import: `std::ascii::AsciiExt`
 --> src/main.rs:2:20
  |
2 |     pub(crate) use std::ascii::AsciiExt;
  |                    ^^^^^^^^^^^^^^^^^^^^
  |
  = note: #[warn(unused_imports)] on by default

Applying any of the following changes makes the warning go away:

• change pub(crate) use std::ascii::AsciiExt to pub use std::ascii::AsciiExt
• change use foo::* to use foo::AsciiExt
• change "bar".is_ascii() to AsciiExt::is_ascii("bar")

Meta

This occurs for all versions of the compiler on the playground:

• stable
• beta
• nightly

Also, on my machine, rustc --version --verbose:

rustc 1.21.0 (3b72af97e 2017-10-09)
binary: rustc
commit-hash: 3b72af97e42989b2fe104d8edbaee123cdf7c58f
commit-date: 2017-10-09
host: x86_64-unknown-linux-gnu
release: 1.21.0
LLVM version: 4.0

[ExpHP]

Just ran into this myself today. I'm surprised I haven't run into it earlier.

My own findings are aligned with the OP's:

// build with cargo test

mod tr {
    pub trait Trait: Sized { fn frob(self); }

    impl Trait for () { fn frob(self) {} }
}

#[cfg(test)]
mod c {
    use tr::Trait;
    mod c {
        use super::*;
        #[test] fn lol() { ().frob(); }
    }
}

• replacing use tr::Trait with a copy of contents of mod tr suppresses the warning (so the warning only applies to imports of traits, not traits themselves)
• unglobbing the glob import suppresses the warning

It has been around since the introduction of item-like imports. Or at least since 1.15, which is the first version in which the snippet compiles.

   Compiling unused v0.1.0 (file:///home/lampam/cpp/throwaway/unused)
warning: unused import: `tr::Trait`, #[warn(unused_imports)] on by default
  --> src/lib.rs:12:9
   |
12 |     use tr::Trait;
   |         ^^^^^^^^^

    Finished debug [unoptimized + debuginfo] target(s) in 0.38 secs
     Running target/debug/deps/unused-a116605afa74f946

[ExpHP]

Because I am interested in beginning to learn more about the compiler internals, I tried tracing through the relevant logic in the source code. Here's what I think happens:

• usages of a trait that are only through method resolution cannot be checked by rustc_resolve, so it defers them to be checked by rustc_typeck.
• Ideally, the lint would be silenced here in typeck, at pretty much the last moment before it is linted. This checks a table called "used_trait_imports".
• when method resolution identifies a trait method, it places a single import into the used_trait_imports table.
  
  
  
  • I tried enabling debug logs and got this:
    
    DEBUG:rustc_typeck::check::method: used_trait_import: DefId { krate: CrateNum(0), index: DefIndex(0:11) => lib[317d]::c[0]::c[0]::{{?}}[0] }
    
    I assume {{?}} refers to the glob import.
  
  

As a result, the glob import is marked as used, but not the trait import.

Does it sound like this "amateur analysis" is on the right track? I wonder which component ought to be changed to fix this... (could the glob import be followed by method resolution to insert more entries into the table?)

[pmarcelll]

This seems to be similar to #43970.

[ExpHP]

Status update: this bug is still present. I get hit by it increasingly often as I like to use miniature inline modules (with use super::*) for the purposes of ad-hoc namespacing, privacy barriers, and organization.

I still don't have the chops to fix it myself (at least, not without a fair investment of time learning compiler internals).

[ExpHP]

^--- I took a trip around the tracker and rounded up these four juicy, delicious duplicate bug reports, for anybody with an appetite

[Aeradriel]

I just ran into this kind of issue myself. It looks like if you import a trait and use it only in submodules you will face this false warning. Here is another playground that shows the bug (importing an existing trait from another crate) => Playground

[dbrgn]

Would be fantastic if this could get fixed.

@sanxiyn in #50537, you wrote that you know how to fix this. Maybe if you give some guidance someone could pick up this issue? (I don't feel qualified myself.)

[sanxiyn]

Sorry I dropped the ball, didn't I? @ExpHP's analysis above is completely correct, and "follow glob" suggestion is also what I thought. Source: I wrote the most of relevant code.

I probably don't have the time to do this myself in near future...

[ExpHP]

Is this a thing that can be fixed just by modifying one crate? Or is it much more complicated (due to e.g. incrementalization, query system, rainbow unicorn dust...)

If it is something a beginner could do, do you have any hints on the proper way to do it? Any method names you can easily recall? I guess basically what I need is to be able to get the DefId for a particular trait in the module referenced by the glob import (assuming that the module is crate-local), but the internal API surface of the compiler is huge :stuck_out_tongue: ...

[jespersm]

So, I've made a fix for this -- the meat of it is in Resolver::get_traits_in_module_containing_item: If a binding comes from an import, just follow the import's binding and collect all the import_id's, instead of stopping after the first (repeat until you reach the actual Def).

I've verified that it actually fixes the problem, and I'm running the full test suite now.

Only thing is that then I'd have to change

pub struct TraitCandidate {
    pub def_id: DefId,
    pub import_id: Option<NodeId>,
}

into

pub struct TraitCandidate {
    pub def_id: DefId,
    pub import_ids: Vec<NodeId>,
}

(and similar change to Pick). I'm just concerned this will lead to increased malloc usage. Is there a "SmallVec" or similar that could be used, because I guess it's going to be very rare that there should be more than one or levels of trait imports.

Please advice!

[ExpHP]

That's marvelous!

Once the test suite passes, I think you should go ahead and submit a PR. This will give it higher visibility to the people who are best equipped to answer your concern, and it will also enable them to queue up a rustc-perf comparison if deemed necessary.

[jespersm]

I'm looking into the test failures.

[jespersm]

When does a bug like this get merged to stable?

[sanxiyn]

It takes 3 months unless explicit backport is done.

[ExpHP]

Every three months, the current master becomes the new beta, and the current beta becomes the new stable.

Therefore, any change takes 3-6 months to get into stable, depending on its timing. Since a new version of rust was just released, there are 3 months remaining for this change.

[sanxiyn]

Eh no. master->beta and beta->stable happens every 1.5 months. So 3 months is correct.

[TonalidadeHidrica]

It's been nearly a year since then but the bug still remains.
I'm afraid I cannot reproduce a minimal example because I am a completely new to Rust...

[jespersm]

It's been nearly a year since then but the bug still remains.
I'm afraid I cannot reproduce a minimal example because I am a completely new to Rust...

The bug is closed, and all the examples given are fixes -- so why do you think the bug still exists?

A trick: If you get unused import warnings when building, but you code fails to compile if you remove them, it could be because they are used in conditionally imported code such as tests. If that is the case, you should move the imports into the conditional section. That was the problem I had as a beginner to begin with, which lead me to this bug...

[TonalidadeHidrica]

You are completely right, that was why I was getting such an error! Thanks for telling me, and sorry for every participants of this issue to have sent such a silly notification...