Skip to content

Commit

Permalink
Implement warning for unused dependencies. This will print a diagnostic
Browse files Browse the repository at this point in the history
for crates which are mentioned as `--extern` arguments on the command
line, but are never referenced from the source.

This diagnostic is controlled by `-Wunused-crate-deps` or
`#![warn(unused_crate_deps)]` and is "allow" by default.

There are cases where certain crates need to be linked in but are not
directly referenced - for example if they are providing symbols for C
linkage. In this case the warning can be suppressed with
`use needed_crate as _;`.

The diagnostics will name the crate and the source file of its
root module.  Ideally, however, it would point to the place where the
dependency is defined (in Cargo.toml, for example) to allow the problem
to be solved directly.  That still needs some design work and will be
in a later PR.

Resolves issue rust-lang#57274
  • Loading branch information
jsgf authored and petrochenkov committed May 23, 2020
1 parent 75b0a68 commit 65b5619
Show file tree
Hide file tree
Showing 23 changed files with 318 additions and 12 deletions.
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) {
UNUSED_ALLOCATION,
UNUSED_DOC_COMMENTS,
UNUSED_EXTERN_CRATES,
UNUSED_CRATE_DEPS,
UNUSED_FEATURES,
UNUSED_LABELS,
UNUSED_PARENS,
Expand Down
106 changes: 94 additions & 12 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use rustc_middle::middle::cstore::{
};
use rustc_middle::ty::TyCtxt;
use rustc_session::config::{self, CrateType};
use rustc_session::lint;
use rustc_session::output::validate_crate_name;
use rustc_session::search_paths::PathKind;
use rustc_session::{CrateDisambiguator, Session};
Expand All @@ -28,12 +29,20 @@ use rustc_target::spec::{PanicStrategy, TargetTriple};

use log::{debug, info, log_enabled};
use proc_macro::bridge::client::ProcMacro;
use std::collections::BTreeSet;
use std::path::Path;
use std::{cmp, fs};

#[derive(Clone)]
struct ResolvedCrateMetadata {
/// Track names by which this crate has been referenced in imports
names: BTreeSet<Symbol>,
meta: Lrc<CrateMetadata>,
}

#[derive(Clone)]
pub struct CStore {
metas: IndexVec<CrateNum, Option<Lrc<CrateMetadata>>>,
metas: IndexVec<CrateNum, Option<ResolvedCrateMetadata>>,
injected_panic_runtime: Option<CrateNum>,
/// This crate needs an allocator and either provides it itself, or finds it in a dependency.
/// If the above is true, then this field denotes the kind of the found allocator.
Expand Down Expand Up @@ -127,18 +136,38 @@ impl CStore {
let cdata = self.metas[cnum]
.as_ref()
.unwrap_or_else(|| panic!("Failed to get crate data for {:?}", cnum));
CrateMetadataRef { cdata, cstore: self }
CrateMetadataRef { cdata: &cdata.meta, cstore: self }
}

fn set_crate_data(&mut self, cnum: CrateNum, data: CrateMetadata) {
fn set_crate_data(&mut self, cnum: CrateNum, name: Symbol, data: CrateMetadata) {
assert!(self.metas[cnum].is_none(), "Overwriting crate metadata entry");
self.metas[cnum] = Some(Lrc::new(data));
let mut names = BTreeSet::default();
names.insert(name);
self.metas[cnum] = Some(ResolvedCrateMetadata { names, meta: Lrc::new(data) });
}

fn add_name(&mut self, cnum: CrateNum, name: Symbol) {
assert!(self.metas[cnum].is_some(), "Setting name on unset entry");
if let Some(data) = &mut self.metas[cnum] {
data.names.insert(name);
}
}

crate fn iter_crate_data(&self, mut f: impl FnMut(CrateNum, &CrateMetadata)) {
for (cnum, data) in self.metas.iter_enumerated() {
if let Some(data) = data {
f(cnum, data);
f(cnum, &data.meta);
}
}
}

crate fn iter_crate_data_names(
&self,
mut f: impl FnMut(CrateNum, &CrateMetadata, &BTreeSet<Symbol>),
) {
for (cnum, data) in self.metas.iter_enumerated() {
if let Some(data) = data {
f(cnum, &data.meta, &data.names);
}
}
}
Expand Down Expand Up @@ -331,10 +360,16 @@ impl<'a> CrateLoader<'a> {
let host_hash = host_lib.as_ref().map(|lib| lib.metadata.get_root().hash());
self.verify_no_symbol_conflicts(span, &crate_root);

let private_dep =
self.sess.opts.externs.get(&name.as_str()).map(|e| e.is_private_dep).unwrap_or(false);
let ext = self.sess.opts.externs.get(&name.as_str());
let private_dep = ext.map(|e| e.is_private_dep).unwrap_or(false);
let add_prelude = ext.map(|e| e.add_prelude).unwrap_or(false);

info!("register crate `{}` (private_dep = {})", crate_root.name(), private_dep);
info!(
"register crate `{}` (private_dep = {}, add_prelude = {})",
crate_root.name(),
private_dep,
add_prelude
);

// Claim this crate number and cache it
let cnum = self.cstore.alloc_new_crate_num();
Expand Down Expand Up @@ -368,6 +403,7 @@ impl<'a> CrateLoader<'a> {

self.cstore.set_crate_data(
cnum,
name,
CrateMetadata::new(
self.sess,
metadata,
Expand All @@ -386,7 +422,7 @@ impl<'a> CrateLoader<'a> {
}

fn load_proc_macro<'b>(
&self,
&mut self,
locator: &mut CrateLocator<'b>,
path_kind: PathKind,
) -> Option<(LoadResult, Option<Library>)>
Expand Down Expand Up @@ -455,7 +491,7 @@ impl<'a> CrateLoader<'a> {
mut dep_kind: DepKind,
dep: Option<(&'b CratePaths, &'b CrateDep)>,
) -> Result<CrateNum, LoadError<'b>> {
info!("resolving crate `{}`", name);
info!("resolving crate `{}` dep {}", name, dep.is_some());
let (root, hash, host_hash, extra_filename, path_kind) = match dep {
Some((root, dep)) => (
Some(root),
Expand All @@ -467,9 +503,10 @@ impl<'a> CrateLoader<'a> {
None => (None, None, None, None, PathKind::Crate),
};
let result = if let Some(cnum) = self.existing_match(name, hash, path_kind) {
self.cstore.add_name(cnum, name);
(LoadResult::Previous(cnum), None)
} else {
info!("falling back to a load");
info!("falling back to a load for {}", name);
let mut locator = CrateLocator::new(
self.sess,
self.metadata_loader,
Expand Down Expand Up @@ -509,7 +546,7 @@ impl<'a> CrateLoader<'a> {
}
}

fn load(&self, locator: &mut CrateLocator<'_>) -> Option<LoadResult> {
fn load(&mut self, locator: &mut CrateLocator<'_>) -> Option<LoadResult> {
let library = locator.maybe_load_library_crate()?;

// In the case that we're loading a crate, but not matching
Expand All @@ -530,6 +567,9 @@ impl<'a> CrateLoader<'a> {
result = LoadResult::Previous(cnum);
}
});
if let LoadResult::Previous(cnum) = result {
self.cstore.add_name(cnum, locator.crate_name());
}
Some(result)
} else {
Some(LoadResult::Loaded(library))
Expand Down Expand Up @@ -839,6 +879,46 @@ impl<'a> CrateLoader<'a> {
});
}

fn report_unused_deps(&mut self, krate: &ast::Crate) {
// Get set of specified crates on the command line
let mut externs = BTreeSet::default();
for (name, _ext) in self.sess.opts.externs.iter() {
externs.insert(Symbol::intern(name));
}

// Remove all resolved crates which this crate references directly
self.cstore.iter_crate_data_names(|_, crate_data, names| {
info!(
"resolved crate `{}` names {:?} direct_extern_crate {}",
crate_data.name(),
names,
crate_data.direct_extern_crate()
);
if crate_data.direct_extern_crate() {
for name in names {
externs.remove(name);
}
}
});

// Make a point span rather than covering the whole file
let spandata = krate.span.data();
let span = spandata.with_hi(spandata.lo);
// Complain about anything left over
for ext in externs {
self.sess.parse_sess.buffer_lint(
lint::builtin::UNUSED_CRATE_DEPS,
span,
ast::CRATE_NODE_ID,
&format!(
"External crate `{}` unused in `{}`. Remove the dependency or add `use {} as _;`.",
ext,
self.local_crate_name,
ext),
);
}
}

pub fn postprocess(&mut self, krate: &ast::Crate) {
self.inject_profiler_runtime();
self.inject_allocator_crate(krate);
Expand All @@ -847,6 +927,8 @@ impl<'a> CrateLoader<'a> {
if log_enabled!(log::Level::Info) {
dump_crates(&self.cstore);
}

self.report_unused_deps(krate);
}

pub fn process_extern_crate(
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_metadata/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,10 @@ impl<'a> CrateLocator<'a> {
// Extract the dylib/rlib/rmeta triple.
self.extract_lib(rlibs, rmetas, dylibs).map(|(_, lib)| lib)
}

crate fn crate_name(&self) -> Symbol {
self.crate_name
}
}

// Just a small wrapper to time how long reading metadata takes.
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_metadata/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1675,6 +1675,10 @@ impl CrateMetadata {
self.root.is_proc_macro_crate()
}

crate fn direct_extern_crate(&self) -> bool {
self.extern_crate.borrow().map_or(false, |ext| ext.is_direct())
}

crate fn name(&self) -> Symbol {
self.root.name
}
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_session/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,12 @@ declare_lint! {
"extern crates that are never used"
}

declare_lint! {
pub UNUSED_CRATE_DEPS,
Allow,
"crate dependencies that are never used"
}

declare_lint! {
pub UNUSED_QUALIFICATIONS,
Allow,
Expand Down Expand Up @@ -523,6 +529,7 @@ declare_lint_pass! {
UNCONDITIONAL_PANIC,
UNUSED_IMPORTS,
UNUSED_EXTERN_CRATES,
UNUSED_CRATE_DEPS,
UNUSED_QUALIFICATIONS,
UNKNOWN_LINTS,
UNUSED_VARIABLES,
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/unused-crate-deps/auxiliary/bar.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub const BAR: &str = "bar";
5 changes: 5 additions & 0 deletions src/test/ui/unused-crate-deps/auxiliary/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// edition:2018
// aux-crate:bar=bar.rs

pub const FOO: &str = "foo";
pub use bar::BAR;
19 changes: 19 additions & 0 deletions src/test/ui/unused-crate-deps/libfib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// check-pass
// aux-crate:bar=bar.rs
// compile-flags:--crate-type lib -Wunused-crate-deps

pub fn fib(n: u32) -> Vec<u32> {
//~^ External crate `bar` unused in `libfib`. Remove the dependency or add `use bar as _;`.
let mut prev = 0;
let mut cur = 1;
let mut v = vec![];

for _ in 0..n {
v.push(prev);
let n = prev + cur;
prev = cur;
cur = n;
}

v
}
10 changes: 10 additions & 0 deletions src/test/ui/unused-crate-deps/libfib.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
warning: External crate `bar` unused in `libfib`. Remove the dependency or add `use bar as _;`.
--> $DIR/libfib.rs:5:1
|
LL | pub fn fib(n: u32) -> Vec<u32> {
| ^
|
= note: requested on the command line with `-W unused-crate-deps`

warning: 1 warning emitted

11 changes: 11 additions & 0 deletions src/test/ui/unused-crate-deps/suppress.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Suppress by using crate

// edition:2018
// check-pass
// aux-crate:bar=bar.rs

#![warn(unused_crate_deps)]

use bar as _;

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/unused-crate-deps/unused-aliases-dylib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Warn about unused aliased for the crate

// edition:2018
// check-pass
// aux-crate:bar=bar.rs
// aux-crate:barbar=bar.rs
// ignore-tidy-linelength

#![warn(unused_crate_deps)]
//~^ WARNING External crate `barbar` unused in `unused_aliases_dylib`. Remove the dependency or add `use barbar as _;`.

use bar as _;

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/unused-crate-deps/unused-aliases-dylib.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
warning: External crate `barbar` unused in `unused_aliases_dylib`. Remove the dependency or add `use barbar as _;`.
--> $DIR/unused-aliases-dylib.rs:9:1
|
LL | #![warn(unused_crate_deps)]
| ^
|
note: the lint level is defined here
--> $DIR/unused-aliases-dylib.rs:9:9
|
LL | #![warn(unused_crate_deps)]
| ^^^^^^^^^^^^^^^^^

warning: 1 warning emitted

15 changes: 15 additions & 0 deletions src/test/ui/unused-crate-deps/unused-aliases.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Warn about unused aliased for the crate

// edition:2018
// check-pass
// aux-crate:bar=bar.rs
// aux-crate:barbar=bar.rs
// no-prefer-dynamic
// ignore-tidy-linelength

#![warn(unused_crate_deps)]
//~^ WARNING External crate `barbar` unused in `unused_aliases`. Remove the dependency or add `use barbar as _;`.

use bar as _;

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/unused-crate-deps/unused-aliases.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
warning: External crate `barbar` unused in `unused_aliases`. Remove the dependency or add `use barbar as _;`.
--> $DIR/unused-aliases.rs:10:1
|
LL | #![warn(unused_crate_deps)]
| ^
|
note: the lint level is defined here
--> $DIR/unused-aliases.rs:10:9
|
LL | #![warn(unused_crate_deps)]
| ^^^^^^^^^^^^^^^^^

warning: 1 warning emitted

13 changes: 13 additions & 0 deletions src/test/ui/unused-crate-deps/use_extern_crate_2015.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Suppress by using crate

// edition:2015
// check-pass
// aux-crate:bar=bar.rs

#![warn(unused_crate_deps)]

extern crate bar;

fn main() {
println!("bar {}", bar::BAR);
}
11 changes: 11 additions & 0 deletions src/test/ui/unused-crate-deps/warn-attr-dylib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Check for unused crate dep, no path

// edition:2018
// check-pass
// aux-crate:bar=bar.rs
// ignore-tidy-linelength

#![warn(unused_crate_deps)]
//~^ WARNING External crate `bar` unused in `warn_attr_dylib`. Remove the dependency or add `use bar as _;`.

fn main() {}
Loading

0 comments on commit 65b5619

Please sign in to comment.