From aef1e35edc11b3e0ecf7f77bc70c197062023476 Mon Sep 17 00:00:00 2001 From: est31 Date: Tue, 30 Jun 2020 18:58:15 +0200 Subject: [PATCH 1/7] Emit unused externs --- compiler/rustc_errors/src/emitter.rs | 3 +++ compiler/rustc_errors/src/json.rs | 19 +++++++++++++++++++ compiler/rustc_errors/src/lib.rs | 8 ++++++++ compiler/rustc_metadata/src/creader.rs | 4 ++++ 4 files changed, 34 insertions(+) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 9b6f67166bd..dbb71d52e49 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -195,6 +195,9 @@ pub trait Emitter { fn emit_future_breakage_report(&mut self, _diags: Vec<(FutureBreakage, Diagnostic)>) {} + /// Emit list of unused externs + fn emit_unused_externs(&mut self, _unused_externs: &[&str]) {} + /// Checks if should show explanations about "rustc --explain" fn should_show_explain(&self) -> bool { true diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index c27b39a9d62..a1ab98f766e 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -159,6 +159,19 @@ impl Emitter for JsonEmitter { } } + fn emit_unused_externs(&mut self, unused_externs: &[&str]) { + let data = UnusedExterns { unused_extern_names: unused_externs }; + let result = if self.pretty { + writeln!(&mut self.dst, "{}", as_pretty_json(&data)) + } else { + writeln!(&mut self.dst, "{}", as_json(&data)) + } + .and_then(|_| self.dst.flush()); + if let Err(e) = result { + panic!("failed to print unused externs: {:?}", e); + } + } + fn source_map(&self) -> Option<&Lrc> { Some(&self.sm) } @@ -322,6 +335,12 @@ struct FutureIncompatReport { future_incompat_report: Vec, } +#[derive(Encodable)] +struct UnusedExterns<'a, 'b> { + /// List of unused externs by their names. + unused_extern_names: &'a [&'b str], +} + impl Diagnostic { fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { let sugg = diag.suggestions.iter().map(|sugg| Diagnostic { diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index a0be7442d59..5720e98abc8 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -767,6 +767,10 @@ impl Handler { self.inner.borrow_mut().emitter.emit_future_breakage_report(diags) } + pub fn emit_unused_externs(&self, unused_externs: &[&str]) { + self.inner.borrow_mut().emit_unused_externs(unused_externs) + } + pub fn delay_as_bug(&self, diagnostic: Diagnostic) { self.inner.borrow_mut().delay_as_bug(diagnostic) } @@ -841,6 +845,10 @@ impl HandlerInner { self.emitter.emit_artifact_notification(path, artifact_type); } + fn emit_unused_externs(&mut self, unused_externs: &[&str]) { + self.emitter.emit_unused_externs(unused_externs); + } + fn treat_err_as_bug(&self) -> bool { self.flags.treat_err_as_bug.map_or(false, |c| self.err_count() >= c.get()) } diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index b5506acf735..3f2d312e61b 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -893,6 +893,7 @@ impl<'a> CrateLoader<'a> { fn report_unused_deps(&mut self, krate: &ast::Crate) { // Make a point span rather than covering the whole file let span = krate.span.shrink_to_lo(); + let mut unused_externs = Vec::new(); // Complain about anything left over for (name, entry) in self.sess.opts.externs.iter() { if let ExternLocation::FoundInLibrarySearchDirectories = entry.location { @@ -917,6 +918,7 @@ impl<'a> CrateLoader<'a> { ) } }; + unused_externs.push(name as &str); self.sess.parse_sess.buffer_lint_with_diagnostic( lint::builtin::UNUSED_CRATE_DEPENDENCIES, span, @@ -929,6 +931,8 @@ impl<'a> CrateLoader<'a> { diag, ); } + // FIXME: add gating + self.sess.parse_sess.span_diagnostic.emit_unused_externs(&unused_externs); } pub fn postprocess(&mut self, krate: &ast::Crate) { From 3f2ca47a79911d422fd47ee2a23dd08a9fa42aa9 Mon Sep 17 00:00:00 2001 From: est31 Date: Tue, 30 Jun 2020 20:17:07 +0200 Subject: [PATCH 2/7] Gate the printing on --json=unused-externs --- compiler/rustc_metadata/src/creader.rs | 5 +++-- compiler/rustc_session/src/config.rs | 18 +++++++++++++++--- compiler/rustc_session/src/options.rs | 3 +++ src/librustdoc/config.rs | 2 +- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 3f2d312e61b..7b34374032c 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -931,8 +931,9 @@ impl<'a> CrateLoader<'a> { diag, ); } - // FIXME: add gating - self.sess.parse_sess.span_diagnostic.emit_unused_externs(&unused_externs); + if self.sess.opts.json_unused_externs { + self.sess.parse_sess.span_diagnostic.emit_unused_externs(&unused_externs); + } } pub fn postprocess(&mut self, krate: &ast::Crate) { diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index f25828e2161..a66201953d6 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -734,6 +734,7 @@ impl Default for Options { remap_path_prefix: Vec::new(), edition: DEFAULT_EDITION, json_artifact_notifications: false, + json_unused_externs: false, pretty: None, } } @@ -1254,11 +1255,12 @@ pub fn parse_color(matches: &getopts::Matches) -> ColorConfig { /// /// The first value returned is how to render JSON diagnostics, and the second /// is whether or not artifact notifications are enabled. -pub fn parse_json(matches: &getopts::Matches) -> (HumanReadableErrorType, bool) { +pub fn parse_json(matches: &getopts::Matches) -> (HumanReadableErrorType, bool, bool) { let mut json_rendered: fn(ColorConfig) -> HumanReadableErrorType = HumanReadableErrorType::Default; let mut json_color = ColorConfig::Never; let mut json_artifact_notifications = false; + let mut json_unused_externs = false; for option in matches.opt_strs("json") { // For now conservatively forbid `--color` with `--json` since `--json` // won't actually be emitting any colors and anything colorized is @@ -1275,6 +1277,7 @@ pub fn parse_json(matches: &getopts::Matches) -> (HumanReadableErrorType, bool) "diagnostic-short" => json_rendered = HumanReadableErrorType::Short, "diagnostic-rendered-ansi" => json_color = ColorConfig::Always, "artifacts" => json_artifact_notifications = true, + "unused-externs" => json_unused_externs = true, s => early_error( ErrorOutputType::default(), &format!("unknown `--json` option `{}`", s), @@ -1282,7 +1285,7 @@ pub fn parse_json(matches: &getopts::Matches) -> (HumanReadableErrorType, bool) } } } - (json_rendered(json_color), json_artifact_notifications) + (json_rendered(json_color), json_artifact_notifications, json_unused_externs) } /// Parses the `--error-format` flag. @@ -1860,7 +1863,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { let edition = parse_crate_edition(matches); - let (json_rendered, json_artifact_notifications) = parse_json(matches); + let (json_rendered, json_artifact_notifications, json_unused_externs) = parse_json(matches); let error_format = parse_error_format(matches, color, json_rendered); @@ -1873,6 +1876,14 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { let mut debugging_opts = build_debugging_options(matches, error_format); check_debug_option_stability(&debugging_opts, error_format, json_rendered); + if !debugging_opts.unstable_options && json_unused_externs { + early_error( + error_format, + "the `-Z unstable-options` flag must also be passed to enable \ + the flag `--json=unused-externs`", + ); + } + let output_types = parse_output_types(&debugging_opts, matches, error_format); let mut cg = build_codegen_options(matches, error_format); @@ -2050,6 +2061,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { remap_path_prefix, edition, json_artifact_notifications, + json_unused_externs, pretty, } } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 79bbad8307b..cfe29d40b74 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -147,6 +147,9 @@ top_level_options!( // by the compiler. json_artifact_notifications: bool [TRACKED], + // `true` if we're emitting a JSON blob containing the unused externs + json_unused_externs: bool [UNTRACKED], + pretty: Option [UNTRACKED], } ); diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index ecb6378f31f..5d6d5aaec14 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -323,7 +323,7 @@ impl Options { } let color = config::parse_color(&matches); - let (json_rendered, _artifacts) = config::parse_json(&matches); + let (json_rendered, ..) = config::parse_json(&matches); let error_format = config::parse_error_format(&matches, color, json_rendered); let codegen_options = build_codegen_options(matches, error_format); From 2d5200605f18717efcb5483cfd2aece167cab7ce Mon Sep 17 00:00:00 2001 From: est31 Date: Sun, 5 Jul 2020 19:35:46 +0200 Subject: [PATCH 3/7] Make parse_json return JsonConfig --- compiler/rustc_session/src/config.rs | 19 ++++++++++++++++--- src/librustdoc/config.rs | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index a66201953d6..433b87aa3c6 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1251,11 +1251,18 @@ pub fn parse_color(matches: &getopts::Matches) -> ColorConfig { } } +/// Possible json config files +pub struct JsonConfig { + pub json_rendered: HumanReadableErrorType, + pub json_artifact_notifications: bool, + pub json_unused_externs: bool, +} + /// Parse the `--json` flag. /// /// The first value returned is how to render JSON diagnostics, and the second /// is whether or not artifact notifications are enabled. -pub fn parse_json(matches: &getopts::Matches) -> (HumanReadableErrorType, bool, bool) { +pub fn parse_json(matches: &getopts::Matches) -> JsonConfig { let mut json_rendered: fn(ColorConfig) -> HumanReadableErrorType = HumanReadableErrorType::Default; let mut json_color = ColorConfig::Never; @@ -1285,7 +1292,12 @@ pub fn parse_json(matches: &getopts::Matches) -> (HumanReadableErrorType, bool, } } } - (json_rendered(json_color), json_artifact_notifications, json_unused_externs) + + JsonConfig { + json_rendered: json_rendered(json_color), + json_artifact_notifications, + json_unused_externs, + } } /// Parses the `--error-format` flag. @@ -1863,7 +1875,8 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { let edition = parse_crate_edition(matches); - let (json_rendered, json_artifact_notifications, json_unused_externs) = parse_json(matches); + let JsonConfig { json_rendered, json_artifact_notifications, json_unused_externs } = + parse_json(matches); let error_format = parse_error_format(matches, color, json_rendered); diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 5d6d5aaec14..112fe230916 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -323,7 +323,7 @@ impl Options { } let color = config::parse_color(&matches); - let (json_rendered, ..) = config::parse_json(&matches); + let config::JsonConfig { json_rendered, .. } = config::parse_json(&matches); let error_format = config::parse_error_format(&matches, color, json_rendered); let codegen_options = build_codegen_options(matches, error_format); From 13371b59ee918445ede03cebb741539db807e0e7 Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 1 Aug 2020 12:57:35 +0200 Subject: [PATCH 4/7] Make doctests collect and emit the unused externs --- compiler/rustc_session/src/config.rs | 4 ++ src/librustdoc/config.rs | 6 +- src/librustdoc/doctest.rs | 85 ++++++++++++++++++++++++++-- 3 files changed, 88 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 433b87aa3c6..2965fe8e1e8 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -485,6 +485,10 @@ impl Externs { pub fn iter(&self) -> BTreeMapIter<'_, String, ExternEntry> { self.0.iter() } + + pub fn len(&self) -> usize { + self.0.len() + } } impl ExternEntry { diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 112fe230916..61035684ef3 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -153,6 +153,8 @@ crate struct Options { /// If this option is set to `true`, rustdoc will only run checks and not generate /// documentation. crate run_check: bool, + /// Whether doctests should emit unused externs + crate json_unused_externs: bool, } impl fmt::Debug for Options { @@ -323,7 +325,8 @@ impl Options { } let color = config::parse_color(&matches); - let config::JsonConfig { json_rendered, .. } = config::parse_json(&matches); + let config::JsonConfig { json_rendered, json_unused_externs, .. } = + config::parse_json(&matches); let error_format = config::parse_error_format(&matches, color, json_rendered); let codegen_options = build_codegen_options(matches, error_format); @@ -644,6 +647,7 @@ impl Options { }, crate_name, output_format, + json_unused_externs, }) } diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 27ce064669d..50cdf46ce4f 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -1,5 +1,5 @@ use rustc_ast as ast; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::Lrc; use rustc_errors::{ColorConfig, ErrorReported}; use rustc_hir as hir; @@ -23,6 +23,8 @@ use std::panic; use std::path::PathBuf; use std::process::{self, Command, Stdio}; use std::str; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex}; use crate::clean::Attributes; use crate::config::Options; @@ -103,8 +105,10 @@ crate fn run(options: Options) -> Result<(), ErrorReported> { let mut test_args = options.test_args.clone(); let display_warnings = options.display_warnings; + let externs = options.externs.clone(); + let json_unused_externs = options.json_unused_externs; - let tests = interface::run_compiler(config, |compiler| { + let res = interface::run_compiler(config, |compiler| { compiler.enter(|queries| { let lower_to_hir = queries.lower_to_hir()?; @@ -147,12 +151,15 @@ crate fn run(options: Options) -> Result<(), ErrorReported> { }); compiler.session().abort_if_errors(); - let ret: Result<_, ErrorReported> = Ok(collector.tests); + let unused_extern_reports = collector.unused_extern_reports.clone(); + let compiling_test_count = collector.compiling_test_count.load(Ordering::SeqCst); + let ret: Result<_, ErrorReported> = + Ok((collector.tests, unused_extern_reports, compiling_test_count)); ret }) }); - let tests = match tests { - Ok(tests) => tests, + let (tests, unused_extern_reports, compiling_test_count) = match res { + Ok(res) => res, Err(ErrorReported) => return Err(ErrorReported), }; @@ -164,6 +171,29 @@ crate fn run(options: Options) -> Result<(), ErrorReported> { Some(testing::Options::new().display_output(display_warnings)), ); + // Collect and warn about unused externs, but only if we've gotten + // reports for each doctest + if json_unused_externs { + let unused_extern_reports: Vec<_> = + std::mem::take(&mut unused_extern_reports.lock().unwrap()); + if unused_extern_reports.len() == compiling_test_count { + let extern_names = externs.iter().map(|(name, _)| name).collect::>(); + let mut unused_extern_names = unused_extern_reports + .iter() + .map(|uexts| uexts.unused_extern_names.iter().collect::>()) + .fold(extern_names, |uextsa, uextsb| { + uextsa.intersection(&uextsb).map(|v| *v).collect::>() + }) + .iter() + .map(|v| (*v).clone()) + .collect::>(); + unused_extern_names.sort(); + let unused_extern_json = + serde_json::to_string(&UnusedExterns { unused_extern_names }).unwrap(); + eprintln!("{}", unused_extern_json); + } + } + Ok(()) } @@ -233,6 +263,12 @@ impl DirState { } } +#[derive(serde::Serialize, serde::Deserialize)] +struct UnusedExterns { + /// List of unused externs by their names. + unused_extern_names: Vec, +} + fn run_test( test: &str, cratename: &str, @@ -251,6 +287,7 @@ fn run_test( outdir: DirState, path: PathBuf, test_id: &str, + report_unused_externs: impl Fn(UnusedExterns), ) -> Result<(), TestFailure> { let (test, line_offset, supports_color) = make_test(test, Some(cratename), as_test_harness, opts, edition, Some(test_id)); @@ -276,6 +313,11 @@ fn run_test( if as_test_harness { compiler.arg("--test"); } + if options.json_unused_externs && !compile_fail { + compiler.arg("--error-format=json"); + compiler.arg("--json").arg("unused-externs"); + compiler.arg("-Z").arg("unstable-options"); + } for lib_str in &options.lib_strs { compiler.arg("-L").arg(&lib_str); } @@ -335,7 +377,26 @@ fn run_test( eprint!("{}", self.0); } } - let out = str::from_utf8(&output.stderr).unwrap(); + let mut out_lines = str::from_utf8(&output.stderr) + .unwrap() + .lines() + .filter(|l| { + if let Ok(uext) = serde_json::from_str::(l) { + report_unused_externs(uext); + false + } else { + true + } + }) + .collect::>(); + + // Add a \n to the end to properly terminate the last line, + // but only if there was output to be printed + if out_lines.len() > 0 { + out_lines.push(""); + } + + let out = out_lines.join("\n"); let _bomb = Bomb(&out); match (output.status.success(), compile_fail) { (true, true) => { @@ -719,6 +780,8 @@ crate struct Collector { source_map: Option>, filename: Option, visited_tests: FxHashMap<(String, usize), usize>, + unused_extern_reports: Arc>>, + compiling_test_count: AtomicUsize, } impl Collector { @@ -743,6 +806,8 @@ impl Collector { source_map, filename, visited_tests: FxHashMap::default(), + unused_extern_reports: Default::default(), + compiling_test_count: AtomicUsize::new(0), } } @@ -789,6 +854,10 @@ impl Tester for Collector { let runtool_args = self.options.runtool_args.clone(); let target = self.options.target.clone(); let target_str = target.to_string(); + let unused_externs = self.unused_extern_reports.clone(); + if !config.compile_fail { + self.compiling_test_count.fetch_add(1, Ordering::SeqCst); + } // FIXME(#44940): if doctests ever support path remapping, then this filename // needs to be the result of `SourceMap::span_to_unmapped_path`. @@ -844,6 +913,9 @@ impl Tester for Collector { test_type: testing::TestType::DocTest, }, testfn: testing::DynTestFn(box move || { + let report_unused_externs = |uext| { + unused_externs.lock().unwrap().push(uext); + }; let res = run_test( &test, &cratename, @@ -862,6 +934,7 @@ impl Tester for Collector { outdir, path, &test_id, + report_unused_externs, ); if let Err(err) = res { From 3a62eb74db63f1b49d5e00c32192498abaf1640f Mon Sep 17 00:00:00 2001 From: est31 Date: Mon, 10 Aug 2020 01:57:35 +0200 Subject: [PATCH 5/7] Emit the lint level of the unused-crate-dependencies Also, turn off the lint when the unused dependencies json flag is specified so that cargo doesn't have to supress the lint --- compiler/rustc_errors/src/emitter.rs | 2 +- compiler/rustc_errors/src/json.rs | 10 ++++--- compiler/rustc_errors/src/lib.rs | 8 +++--- compiler/rustc_interface/src/passes.rs | 7 +++++ compiler/rustc_metadata/src/creader.rs | 36 ++++++++++++++++++++------ src/librustdoc/doctest.rs | 22 ++++++++++++++-- 6 files changed, 66 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index dbb71d52e49..2b6dec905f1 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -196,7 +196,7 @@ pub trait Emitter { fn emit_future_breakage_report(&mut self, _diags: Vec<(FutureBreakage, Diagnostic)>) {} /// Emit list of unused externs - fn emit_unused_externs(&mut self, _unused_externs: &[&str]) {} + fn emit_unused_externs(&mut self, _lint_level: &str, _unused_externs: &[&str]) {} /// Checks if should show explanations about "rustc --explain" fn should_show_explain(&self) -> bool { diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index a1ab98f766e..8511b51e3bf 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -159,8 +159,8 @@ impl Emitter for JsonEmitter { } } - fn emit_unused_externs(&mut self, unused_externs: &[&str]) { - let data = UnusedExterns { unused_extern_names: unused_externs }; + fn emit_unused_externs(&mut self, lint_level: &str, unused_externs: &[&str]) { + let data = UnusedExterns { lint_level, unused_extern_names: unused_externs }; let result = if self.pretty { writeln!(&mut self.dst, "{}", as_pretty_json(&data)) } else { @@ -336,9 +336,11 @@ struct FutureIncompatReport { } #[derive(Encodable)] -struct UnusedExterns<'a, 'b> { +struct UnusedExterns<'a, 'b, 'c> { + /// The severity level of the unused dependencies lint + lint_level: &'a str, /// List of unused externs by their names. - unused_extern_names: &'a [&'b str], + unused_extern_names: &'b [&'c str], } impl Diagnostic { diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 5720e98abc8..533c32b32c6 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -767,8 +767,8 @@ impl Handler { self.inner.borrow_mut().emitter.emit_future_breakage_report(diags) } - pub fn emit_unused_externs(&self, unused_externs: &[&str]) { - self.inner.borrow_mut().emit_unused_externs(unused_externs) + pub fn emit_unused_externs(&self, lint_level: &str, unused_externs: &[&str]) { + self.inner.borrow_mut().emit_unused_externs(lint_level, unused_externs) } pub fn delay_as_bug(&self, diagnostic: Diagnostic) { @@ -845,8 +845,8 @@ impl HandlerInner { self.emitter.emit_artifact_notification(path, artifact_type); } - fn emit_unused_externs(&mut self, unused_externs: &[&str]) { - self.emitter.emit_unused_externs(unused_externs); + fn emit_unused_externs(&mut self, lint_level: &str, unused_externs: &[&str]) { + self.emitter.emit_unused_externs(lint_level, unused_externs); } fn treat_err_as_bug(&self) -> bool { diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 5217066bbef..717f4d5a3ba 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -17,6 +17,7 @@ use rustc_hir::definitions::Definitions; use rustc_hir::Crate; use rustc_index::vec::IndexVec; use rustc_lint::LintStore; +use rustc_metadata::creader::CStore; use rustc_middle::arena::Arena; use rustc_middle::dep_graph::DepGraph; use rustc_middle::middle; @@ -836,6 +837,12 @@ fn analysis(tcx: TyCtxt<'_>, cnum: CrateNum) -> Result<()> { }); sess.time("looking_for_derive_registrar", || proc_macro_decls::find(tcx)); + + let cstore = tcx + .cstore_as_any() + .downcast_ref::() + .expect("`tcx.cstore` is not a `CStore`"); + cstore.report_unused_deps(tcx); }, { par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 7b34374032c..9e3f121378c 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -46,6 +46,9 @@ pub struct CStore { /// This map is used to verify we get no hash conflicts between /// `StableCrateId` values. stable_crate_ids: FxHashMap, + + /// Unused externs of the crate + unused_externs: Vec, } pub struct CrateLoader<'a> { @@ -190,6 +193,21 @@ impl CStore { crate fn has_global_allocator(&self) -> bool { self.has_global_allocator } + + pub fn report_unused_deps(&self, tcx: TyCtxt<'_>) { + let level = tcx + .lint_level_at_node(lint::builtin::UNUSED_CRATE_DEPENDENCIES, rustc_hir::CRATE_HIR_ID) + .0; + if level != lint::Level::Allow && tcx.sess.opts.json_unused_externs { + let unused_externs = + self.unused_externs.iter().map(|ident| ident.to_ident_string()).collect::>(); + let unused_externs = unused_externs.iter().map(String::as_str).collect::>(); + tcx.sess + .parse_sess + .span_diagnostic + .emit_unused_externs(level.as_str(), &unused_externs); + } + } } impl<'a> CrateLoader<'a> { @@ -217,6 +235,7 @@ impl<'a> CrateLoader<'a> { allocator_kind: None, has_global_allocator: false, stable_crate_ids, + unused_externs: Vec::new(), }, used_extern_options: Default::default(), } @@ -893,18 +912,23 @@ impl<'a> CrateLoader<'a> { fn report_unused_deps(&mut self, krate: &ast::Crate) { // Make a point span rather than covering the whole file let span = krate.span.shrink_to_lo(); - let mut unused_externs = Vec::new(); // Complain about anything left over for (name, entry) in self.sess.opts.externs.iter() { if let ExternLocation::FoundInLibrarySearchDirectories = entry.location { // Don't worry about pathless `--extern foo` sysroot references continue; } - if self.used_extern_options.contains(&Symbol::intern(name)) { + let name_interned = Symbol::intern(name); + if self.used_extern_options.contains(&name_interned) { continue; } // Got a real unused --extern + if self.sess.opts.json_unused_externs { + self.cstore.unused_externs.push(name_interned); + continue; + } + let diag = match self.sess.opts.extern_dep_specs.get(name) { Some(loc) => BuiltinLintDiagnostics::ExternDepSpec(name.clone(), loc.into()), None => { @@ -918,7 +942,6 @@ impl<'a> CrateLoader<'a> { ) } }; - unused_externs.push(name as &str); self.sess.parse_sess.buffer_lint_with_diagnostic( lint::builtin::UNUSED_CRATE_DEPENDENCIES, span, @@ -931,9 +954,6 @@ impl<'a> CrateLoader<'a> { diag, ); } - if self.sess.opts.json_unused_externs { - self.sess.parse_sess.span_diagnostic.emit_unused_externs(&unused_externs); - } } pub fn postprocess(&mut self, krate: &ast::Crate) { @@ -941,9 +961,9 @@ impl<'a> CrateLoader<'a> { self.inject_allocator_crate(krate); self.inject_panic_runtime(krate); - info!("{:?}", CrateDump(&self.cstore)); - self.report_unused_deps(krate); + + info!("{:?}", CrateDump(&self.cstore)); } pub fn process_extern_crate( diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 50cdf46ce4f..116b3aad61e 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -188,8 +188,23 @@ crate fn run(options: Options) -> Result<(), ErrorReported> { .map(|v| (*v).clone()) .collect::>(); unused_extern_names.sort(); - let unused_extern_json = - serde_json::to_string(&UnusedExterns { unused_extern_names }).unwrap(); + // Take the most severe lint level + let lint_level = unused_extern_reports + .iter() + .map(|uexts| uexts.lint_level.as_str()) + .max_by_key(|v| match *v { + "warn" => 1, + "deny" => 2, + "forbid" => 3, + // The allow lint level is not expected, + // as if allow is specified, no message + // is to be emitted. + v => unreachable!("Invalid lint level '{}'", v), + }) + .unwrap_or("warn") + .to_string(); + let uext = UnusedExterns { lint_level, unused_extern_names }; + let unused_extern_json = serde_json::to_string(&uext).unwrap(); eprintln!("{}", unused_extern_json); } } @@ -265,6 +280,8 @@ impl DirState { #[derive(serde::Serialize, serde::Deserialize)] struct UnusedExterns { + /// Lint level of the unused_crate_dependencies lint + lint_level: String, /// List of unused externs by their names. unused_extern_names: Vec, } @@ -317,6 +334,7 @@ fn run_test( compiler.arg("--error-format=json"); compiler.arg("--json").arg("unused-externs"); compiler.arg("-Z").arg("unstable-options"); + compiler.arg("-W").arg("unused_crate_dependencies"); } for lib_str in &options.lib_strs { compiler.arg("-L").arg(&lib_str); From d8c9a287036f72bf078f868f8fe635b7a7fde32c Mon Sep 17 00:00:00 2001 From: est31 Date: Wed, 12 Aug 2020 03:45:16 +0200 Subject: [PATCH 6/7] Fix the tests --- compiler/rustc_metadata/src/creader.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 9e3f121378c..5634467eeb7 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -195,10 +195,16 @@ impl CStore { } pub fn report_unused_deps(&self, tcx: TyCtxt<'_>) { + // We put the check for the option before the lint_level_at_node call + // because the call mutates internal state and introducing it + // leads to some ui tests failing. + if !tcx.sess.opts.json_unused_externs { + return; + } let level = tcx .lint_level_at_node(lint::builtin::UNUSED_CRATE_DEPENDENCIES, rustc_hir::CRATE_HIR_ID) .0; - if level != lint::Level::Allow && tcx.sess.opts.json_unused_externs { + if level != lint::Level::Allow { let unused_externs = self.unused_externs.iter().map(|ident| ident.to_ident_string()).collect::>(); let unused_externs = unused_externs.iter().map(String::as_str).collect::>(); From d018ef180d9bc4e852457fd83fda0bec8452baf6 Mon Sep 17 00:00:00 2001 From: est31 Date: Tue, 2 Mar 2021 03:07:13 +0100 Subject: [PATCH 7/7] Add notes to keep the UnusedExterns structs synced up --- compiler/rustc_errors/src/json.rs | 4 ++++ src/librustdoc/doctest.rs | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 8511b51e3bf..3753ca580e7 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -335,6 +335,10 @@ struct FutureIncompatReport { future_incompat_report: Vec, } +// NOTE: Keep this in sync with the equivalent structs in rustdoc's +// doctest component (as well as cargo). +// We could unify this struct the one in rustdoc but they have different +// ownership semantics, so doing so would create wasteful allocations. #[derive(Encodable)] struct UnusedExterns<'a, 'b, 'c> { /// The severity level of the unused dependencies lint diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 116b3aad61e..f7a6ccfc7d1 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -278,6 +278,10 @@ impl DirState { } } +// NOTE: Keep this in sync with the equivalent structs in rustc +// and cargo. +// We could unify this struct the one in rustc but they have different +// ownership semantics, so doing so would create wasteful allocations. #[derive(serde::Serialize, serde::Deserialize)] struct UnusedExterns { /// Lint level of the unused_crate_dependencies lint