Skip to content
23 changes: 23 additions & 0 deletions src/cargo/core/compiler/build_context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::core::compiler::CompileKind;
use crate::core::compiler::Unit;
use crate::core::compiler::UnitIndex;
use crate::core::compiler::unit_graph::UnitGraph;
use crate::core::dependency::DepKind;
use crate::core::profiles::Profiles;
use crate::util::Rustc;
use crate::util::context::GlobalContext;
Expand Down Expand Up @@ -64,6 +65,9 @@ pub struct BuildContext<'a, 'gctx> {
/// Configuration information for a rustc build.
pub build_config: &'a BuildConfig,

/// Associated [`DepKind`]s for root targets
pub selected_dep_kinds: DepKindSet,

/// Extra compiler args for either `rustc` or `rustdoc`.
pub extra_compiler_args: HashMap<Unit, Vec<String>>,

Expand Down Expand Up @@ -97,6 +101,7 @@ impl<'a, 'gctx> BuildContext<'a, 'gctx> {
logger: Option<&'a BuildLogger>,
packages: PackageSet<'gctx>,
build_config: &'a BuildConfig,
selected_dep_kinds: DepKindSet,
profiles: Profiles,
extra_compiler_args: HashMap<Unit, Vec<String>>,
target_data: RustcTargetData<'gctx>,
Expand All @@ -118,6 +123,7 @@ impl<'a, 'gctx> BuildContext<'a, 'gctx> {
logger,
packages,
build_config,
selected_dep_kinds,
profiles,
extra_compiler_args,
target_data,
Expand Down Expand Up @@ -157,3 +163,20 @@ impl<'a, 'gctx> BuildContext<'a, 'gctx> {
self.extra_compiler_args.get(unit)
}
}

#[derive(Copy, Clone, Default, Debug)]
pub struct DepKindSet {
pub build: bool,
pub normal: bool,
pub dev: bool,
}

impl DepKindSet {
pub fn contains(&self, kind: DepKind) -> bool {
match kind {
DepKind::Build => self.build,
DepKind::Normal => self.normal,
DepKind::Development => self.dev,
}
}
}
9 changes: 9 additions & 0 deletions src/cargo/core/compiler/job_queue/job_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,4 +212,13 @@ impl<'a, 'gctx> JobState<'a, 'gctx> {
self.messages
.push(Message::FutureIncompatReport(self.id, report));
}

/// The rustc emitted the list of unused `--extern` args.
///
/// This is useful for checking unused dependencies.
/// Should only be called once, as the compiler only emits it once per compilation.
pub fn unused_externs(&self, unused_externs: Vec<String>) {
self.messages
.push(Message::UnusedExterns(self.id, unused_externs));
}
}
44 changes: 32 additions & 12 deletions src/cargo/core/compiler/job_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ use super::UnitIndex;
use super::custom_build::Severity;
use super::timings::SectionTiming;
use super::timings::Timings;
use super::unused_deps::UnusedDepState;
use crate::core::compiler::descriptive_pkg_name;
use crate::core::compiler::future_incompat::{
self, FutureBreakageItem, FutureIncompatReportPackage,
Expand Down Expand Up @@ -186,6 +187,7 @@ struct DrainState<'gctx> {
progress: Progress<'gctx>,
next_id: u32,
timings: Timings<'gctx>,
unused_dep_state: UnusedDepState,

/// Map from unit index to unit, for looking up dependency information.
index_to_unit: HashMap<UnitIndex, Unit>,
Expand Down Expand Up @@ -384,6 +386,7 @@ enum Message {
Finish(JobId, Artifact, CargoResult<()>),
FutureIncompatReport(JobId, Vec<FutureBreakageItem>),
SectionTiming(JobId, SectionTiming),
UnusedExterns(JobId, Vec<String>),
}

impl<'gctx> JobQueue<'gctx> {
Expand Down Expand Up @@ -503,6 +506,7 @@ impl<'gctx> JobQueue<'gctx> {
progress,
next_id: 0,
timings: self.timings,
unused_dep_state: UnusedDepState::new(build_runner),
index_to_unit: build_runner
.bcx
.unit_to_index
Expand Down Expand Up @@ -543,12 +547,10 @@ impl<'gctx> JobQueue<'gctx> {
.take()
.map(move |srv| srv.start(move |msg| messages.push(Message::FixDiagnostic(msg))));

thread::scope(
move |scope| match state.drain_the_queue(build_runner, scope, &helper) {
Some(err) => Err(err),
None => Ok(()),
},
)
thread::scope(move |scope| {
let (result,) = state.drain_the_queue(build_runner, scope, &helper);
result
})
}
}

Expand Down Expand Up @@ -722,6 +724,11 @@ impl<'gctx> DrainState<'gctx> {
items,
});
}
Message::UnusedExterns(id, unused_externs) => {
let unit = &self.active[&id];
self.unused_dep_state
.record_unused_externs_for_unit(unit, unused_externs);
}
Message::Token(acquired_token) => {
let token = acquired_token.context("failed to acquire jobserver token")?;
self.tokens.push(token);
Expand Down Expand Up @@ -762,14 +769,15 @@ impl<'gctx> DrainState<'gctx> {
/// This is the "main" loop, where Cargo does all work to run the
/// compiler.
///
/// This returns an Option to prevent the use of `?` on `Result` types
/// because it is important for the loop to carefully handle errors.
/// This returns a tuple of `Result` to prevent the use of `?` on
/// `Result` types because it is important for the loop to
/// carefully handle errors.
fn drain_the_queue<'s>(
mut self,
build_runner: &mut BuildRunner<'_, '_>,
scope: &'s Scope<'s, '_>,
jobserver_helper: &HelperThread,
) -> Option<anyhow::Error> {
) -> (Result<(), anyhow::Error>,) {
trace!("queue: {:#?}", self.queue);

// Iteratively execute the entire dependency graph. Each turn of the
Expand Down Expand Up @@ -813,6 +821,18 @@ impl<'gctx> DrainState<'gctx> {
}
self.progress.clear();

if build_runner.bcx.gctx.cli_unstable().cargo_lints {
let mut warn_count = 0;
let mut error_count = 0;
drop(self.unused_dep_state.emit_unused_warnings(
&mut warn_count,
&mut error_count,
build_runner,
));
errors.count += error_count;
build_runner.compilation.lint_warning_count += warn_count;
}

let profile_name = build_runner.bcx.build_config.requested_profile;
// NOTE: this may be a bit inaccurate, since this may not display the
// profile for what was actually built. Profile overrides can change
Expand Down Expand Up @@ -853,7 +873,7 @@ impl<'gctx> DrainState<'gctx> {
if let Some(error) = errors.to_error() {
// Any errors up to this point have already been printed via the
// `display_error` inside `handle_error`.
Some(anyhow::Error::new(AlreadyPrintedError::new(error)))
(Err(anyhow::Error::new(AlreadyPrintedError::new(error))),)
} else if self.queue.is_empty() && self.pending_queue.is_empty() {
let profile_link = build_runner.bcx.gctx.shell().err_hyperlink(
"https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles",
Expand All @@ -868,10 +888,10 @@ impl<'gctx> DrainState<'gctx> {
&self.per_package_future_incompat_reports,
);

None
(Ok(()),)
} else {
debug!("queue: {:#?}", self.queue);
Some(internal("finished with jobs still left in the queue"))
(Err(internal("finished with jobs still left in the queue")),)
}
}

Expand Down
28 changes: 25 additions & 3 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub mod timings;
mod unit;
pub mod unit_dependencies;
pub mod unit_graph;
mod unused_deps;

use std::borrow::Cow;
use std::cell::OnceCell;
Expand All @@ -74,6 +75,7 @@ use tracing::{debug, instrument, trace};
pub use self::build_config::UserIntent;
pub use self::build_config::{BuildConfig, CompileMode, MessageFormat};
pub use self::build_context::BuildContext;
pub use self::build_context::DepKindSet;
pub use self::build_context::FileFlavor;
pub use self::build_context::FileType;
pub use self::build_context::RustcTargetData;
Expand Down Expand Up @@ -836,6 +838,12 @@ fn prepare_rustc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResult
base.env("CARGO_TARGET_TMPDIR", tmp.display().to_string());
}

if build_runner.bcx.gctx.cli_unstable().cargo_lints {
// Added last to reduce the risk of RUSTFLAGS or `[lints]` from interfering with
// `unused_dependencies` tracking
base.arg("-Wunused_crate_dependencies");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentionally placed after the call to extra_args_for? I'm just wondering about the interaction of things like cargo rustc -- -Dwarnings or other such flags that could conflict with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to have this very last so that any user provided modification of unused_crate_dependencies would be overridden unless it was in source code which we can't do anything about.

If there are some particular cases you'd like covered to see how they behave, I can add them.

}

Ok(base)
}

Expand Down Expand Up @@ -1178,8 +1186,11 @@ fn add_error_format_and_color(build_runner: &BuildRunner<'_, '_>, cmd: &mut Proc
}

cmd.arg("--error-format=json");
let mut json = String::from("--json=diagnostic-rendered-ansi,artifacts,future-incompat");

let mut json = String::from("--json=diagnostic-rendered-ansi,artifacts,future-incompat");
if build_runner.bcx.gctx.cli_unstable().cargo_lints {
json.push_str(",unused-externs-silent");
}
if let MessageFormat::Short | MessageFormat::Json { short: true, .. } =
build_runner.bcx.build_config.message_format
{
Expand All @@ -1189,11 +1200,9 @@ fn add_error_format_and_color(build_runner: &BuildRunner<'_, '_>, cmd: &mut Proc
{
json.push_str(",diagnostic-unicode");
}

if enable_timings {
json.push_str(",timings");
}

cmd.arg(json);

let gctx = build_runner.bcx.gctx;
Expand Down Expand Up @@ -2313,6 +2322,19 @@ fn on_stderr_line_inner(
return Ok(false);
}

#[derive(serde::Deserialize)]
struct UnusedExterns {
unused_extern_names: Vec<String>,
}
if let Ok(uext) = serde_json::from_str::<UnusedExterns>(compiler_message.get()) {
trace!(
"obtained unused externs list from rustc: `{:?}`",
uext.unused_extern_names
);
state.unused_externs(uext.unused_extern_names);
return Ok(true);
}

// And failing all that above we should have a legitimate JSON diagnostic
// from the compiler, so wrap it in an external Cargo JSON message
// indicating which package it came from and then emit it.
Expand Down
Loading