2018: fix lint on unused dependencies

[mark-i-m]

Quoting from #53128 (comment)

Previously warn(unused_extern_crates) was a way to detect unused dependencies. How can I:

1. declare dependencies in the root module
2. be warned when those dependencies are not used

As it is now, cargo builds all dependencies and does not warn if they are unused.

[cramertj]

IMO it should be based on the --extern args provided to rustc, and should not ignore "for-side-effects-only" imports now that we can do use some_crate as _;

[mark-i-m]

So as far as I can tell, the set of unused crates is computed here:

rust/src/librustc_typeck/check_unused.rs

Line 74 in ec19464

fn unused_crates_lint<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>) {

and then passed around through the tcx. Is this the code that needs to be updated?

[Nemo157]

This might have issues with adding dependencies-of-dependencies to force specific versions (I'm not sure how common that pattern is, and I guess they could be treated as "for-side-effects-only" imports as @cramertj mentions above).

[dekellum]

In a project, I was just adding such (originally-)transitive dependencies to get a -Z minimal-versions build working. At least before "unused_extern_crates" is changed to default to "warn", perhaps an allow=unused flag could be supported on the Cargo.toml dependency itself, like the #[allow(some_lint)] attribute in code?

[cramertj]

@dekellum Is there a reason you want to allow it in Cargo.toml rather than use <cratename> as _; as I suggested above?

[dekellum]

I agree both that use some_crate as _; should not trigger the unused_extern_crate lint (should not be treated as unused) and with the concern for the use-case raised by @Nemo157. Needing to add use some_crate as _; for each of these originally-transitive dependencies is more cumbersome, when just wanting to guide cargo's dependency resolution, where nothing really needs to be imported as far as rustc is concerned. Note also in my case, using the [patch] section of Cargo.toml didn't work for the such resolution adjustment.

[jonhoo]

We'd also have to make sure that if you have, say, two binaries in a crate, and Cargo.toml lists a dependency that is only used by one of them, compiling the other shouldn't generate a warning.

[cramertj]

@jonhoo IMO that's a bug in cargo that it doesn't allow specifying per-build-target dependencies (or maybe it does, in which case users should do that).

[jonhoo]

@cramertj yeah, I agree, the real solution would be to have per-target dependencies, but sadly that's not something we have yet. There's some discussion in rust-lang/cargo#1982 on how we may add support for dependencies for binary targets, which would go a long way, but that has been open for a while. Until we get support for that though, we'd have to make sure that the lint doesn't erroneously give a warning in those cases.

[cramertj]

@jonhoo It seems fine to me to silence those cases using use X as _;. They are unused external crates, and it's wrong for them to be passed in via --extern but not used in the crate. It so happens that there isn't a "correct" way to fix it right now aside from silencing the warning with use X as _;, but rustc is correct to warn in that case IMO.

What to do about this case practically is just sort of unfortunate, since rustc doesn't have enough information at any point to determine that this is just waiting on a missing cargo feature. The only clear options to me are to trigger this warning and require that users manually silence it (which is undoubtedly annoying) or not display the warning at all until the requisite feature is added to cargo (which seems similarly annoying).

A more complex but complete solution would be to have some way of telling rustc about all the targets you intend to compile in the future that should be considered in determining whether a crate is "used" without actually fully compiling each of those targets. Actually making this work well is a bit outside the reach of my imagination at the moment, and it seems of little value once we get cargo build-target-specific dependencies working.

[jonhoo]

Sounds like the best thing would be to push the effort to allow per-target depenencies in cargo forward. Sadly that's a process that's been slow going since 2015. There has been some recent effort from @pwoolcoc to write up an RFC recently though (helped by @alexreg), so maybe there's hope!