Skip to content

Commit 92a8401

Browse files
authored
fix(compile): build.warnings=deny shouldn't block hard warnings (#16213)
### What does this PR try to resolve? Motivation for this: rustc is trying to adopt this feature but hard warnings are getting in the way, see rust-lang/rust#148332 This is a part of #14802. ### How to test and review this PR? Reasons to do this: - Less flexibility in how hard warnings are resolved - Matches Cargo not erroring for Cargo hard warnings - This matches `-Dwarnings` behavior which this feature is meant to replace Reasons not do this: - A user may see a hard warning from rustc and get confused - Cargo's hard warnings are not blocking in part because we would need to audit them to see if they should actually be hard warnings We do have some flexibility on warnings evolving over time, so this is likely a two-way door (on an unstable feature). See also the discussion at #14802 (comment) Ideally, we'd also test for duplicate hard warnings but unsure how to trigger that.
2 parents 5854d39 + 6fc9761 commit 92a8401

File tree

6 files changed

+102
-11
lines changed

6 files changed

+102
-11
lines changed

src/cargo/core/compiler/compilation.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ pub struct Compilation<'gctx> {
123123
/// The linker to use for each host or target.
124124
target_linkers: HashMap<CompileKind, Option<PathBuf>>,
125125

126-
/// The total number of warnings emitted by the compilation.
127-
pub warning_count: usize,
126+
/// The total number of lint warnings emitted by the compilation.
127+
pub lint_warning_count: usize,
128128
}
129129

130130
impl<'gctx> Compilation<'gctx> {
@@ -162,7 +162,7 @@ impl<'gctx> Compilation<'gctx> {
162162
.chain(Some(&CompileKind::Host))
163163
.map(|kind| Ok((*kind, target_linker(bcx, *kind)?)))
164164
.collect::<CargoResult<HashMap<_, _>>>()?,
165-
warning_count: 0,
165+
lint_warning_count: 0,
166166
})
167167
}
168168

src/cargo/core/compiler/job_queue/job_state.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,12 +93,19 @@ impl<'a, 'gctx> JobState<'a, 'gctx> {
9393
}
9494

9595
/// See [`Message::Diagnostic`] and [`Message::WarningCount`].
96-
pub fn emit_diag(&self, level: &str, diag: String, fixable: bool) -> CargoResult<()> {
96+
pub fn emit_diag(
97+
&self,
98+
level: &str,
99+
diag: String,
100+
lint: bool,
101+
fixable: bool,
102+
) -> CargoResult<()> {
97103
if let Some(dedupe) = self.output {
98104
let emitted = dedupe.emit_diag(&diag)?;
99105
if level == "warning" {
100106
self.messages.push(Message::WarningCount {
101107
id: self.id,
108+
lint,
102109
emitted,
103110
fixable,
104111
});
@@ -108,6 +115,7 @@ impl<'a, 'gctx> JobState<'a, 'gctx> {
108115
id: self.id,
109116
level: level.to_string(),
110117
diag,
118+
lint,
111119
fixable,
112120
});
113121
}

src/cargo/core/compiler/job_queue/mod.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,8 @@ struct DrainState<'gctx> {
207207
pub struct WarningCount {
208208
/// total number of warnings
209209
pub total: usize,
210+
/// number of lint warnings
211+
pub lints: usize,
210212
/// number of warnings that were suppressed because they
211213
/// were duplicates of a previous warning
212214
pub duplicates: usize,
@@ -350,12 +352,14 @@ enum Message {
350352
id: JobId,
351353
level: String,
352354
diag: String,
355+
lint: bool,
353356
fixable: bool,
354357
},
355358
// This handles duplicate output that is suppressed, for showing
356359
// only a count of duplicate messages instead
357360
WarningCount {
358361
id: JobId,
362+
lint: bool,
359363
emitted: bool,
360364
fixable: bool,
361365
},
@@ -616,11 +620,12 @@ impl<'gctx> DrainState<'gctx> {
616620
id,
617621
level,
618622
diag,
623+
lint,
619624
fixable,
620625
} => {
621626
let emitted = self.diag_dedupe.emit_diag(&diag)?;
622627
if level == "warning" {
623-
self.bump_warning_count(id, emitted, fixable);
628+
self.bump_warning_count(id, lint, emitted, fixable);
624629
}
625630
if level == "error" {
626631
let cnts = self.warning_count.entry(id).or_default();
@@ -632,14 +637,18 @@ impl<'gctx> DrainState<'gctx> {
632637
if warning_handling != WarningHandling::Allow {
633638
build_runner.bcx.gctx.shell().warn(warning)?;
634639
}
635-
self.bump_warning_count(id, true, false);
640+
let lint = false;
641+
let emitted = true;
642+
let fixable = false;
643+
self.bump_warning_count(id, lint, emitted, fixable);
636644
}
637645
Message::WarningCount {
638646
id,
647+
lint,
639648
emitted,
640649
fixable,
641650
} => {
642-
self.bump_warning_count(id, emitted, fixable);
651+
self.bump_warning_count(id, lint, emitted, fixable);
643652
}
644653
Message::FixDiagnostic(msg) => {
645654
self.print.print(&msg)?;
@@ -1002,9 +1011,12 @@ impl<'gctx> DrainState<'gctx> {
10021011
Ok(())
10031012
}
10041013

1005-
fn bump_warning_count(&mut self, id: JobId, emitted: bool, fixable: bool) {
1014+
fn bump_warning_count(&mut self, id: JobId, lint: bool, emitted: bool, fixable: bool) {
10061015
let cnts = self.warning_count.entry(id).or_default();
10071016
cnts.total += 1;
1017+
if lint {
1018+
cnts.lints += 1;
1019+
}
10081020
if !emitted {
10091021
cnts.duplicates += 1;
10101022
// Don't add to fixable if it's already been emitted
@@ -1037,7 +1049,7 @@ impl<'gctx> DrainState<'gctx> {
10371049
Some(count) if count.total > 0 => count,
10381050
None | Some(_) => return,
10391051
};
1040-
runner.compilation.warning_count += count.total;
1052+
runner.compilation.lint_warning_count += count.lints;
10411053
let unit = &self.active[&id];
10421054
let mut message = descriptive_pkg_name(&unit.pkg.name(), &unit.target, &unit.mode);
10431055
message.push_str(" generated ");

src/cargo/core/compiler/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2162,12 +2162,14 @@ fn on_stderr_line_inner(
21622162
count_diagnostic(&msg.level, options);
21632163
if msg
21642164
.code
2165+
.as_ref()
21652166
.is_some_and(|c| c.code == "exported_private_dependencies")
21662167
&& options.format != MessageFormat::Short
21672168
{
21682169
add_pub_in_priv_diagnostic(&mut rendered);
21692170
}
2170-
state.emit_diag(&msg.level, rendered, machine_applicable)?;
2171+
let lint = msg.code.is_some();
2172+
state.emit_diag(&msg.level, rendered, lint, machine_applicable)?;
21712173
}
21722174
return Ok(true);
21732175
}

src/cargo/ops/cargo_compile/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ pub fn compile_with_exec<'a>(
143143
) -> CargoResult<Compilation<'a>> {
144144
ws.emit_warnings()?;
145145
let compilation = compile_ws(ws, options, exec)?;
146-
if ws.gctx().warning_handling()? == WarningHandling::Deny && compilation.warning_count > 0 {
146+
if ws.gctx().warning_handling()? == WarningHandling::Deny && compilation.lint_warning_count > 0
147+
{
147148
anyhow::bail!("warnings are denied by `build.warnings` configuration")
148149
}
149150
Ok(compilation)

tests/testsuite/warning_override.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,3 +221,71 @@ Caused by:
221221
.with_status(101)
222222
.run();
223223
}
224+
225+
#[cargo_test]
226+
fn hard_warning_deny() {
227+
let p = project()
228+
.file(
229+
"Cargo.toml",
230+
&format!(
231+
r#"
232+
[package]
233+
name = "foo"
234+
version = "0.0.1"
235+
edition = "2021"
236+
"#
237+
),
238+
)
239+
.file("src/main.rs", "fn main() {}")
240+
.build();
241+
p.cargo("rustc")
242+
.masquerade_as_nightly_cargo(&["warnings"])
243+
.arg("-Zwarnings")
244+
.arg("--config")
245+
.arg("build.warnings='deny'")
246+
.arg("--")
247+
.arg("-ox.rs")
248+
.with_stderr_data(str![[r#"
249+
[COMPILING] foo v0.0.1 ([ROOT]/foo)
250+
[WARNING] [..]
251+
252+
[WARNING] [..]
253+
254+
[WARNING] `foo` (bin "foo") generated 2 warnings
255+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
256+
257+
"#]])
258+
.run();
259+
}
260+
261+
#[cargo_test]
262+
fn hard_warning_allow() {
263+
let p = project()
264+
.file(
265+
"Cargo.toml",
266+
&format!(
267+
r#"
268+
[package]
269+
name = "foo"
270+
version = "0.0.1"
271+
edition = "2021"
272+
"#
273+
),
274+
)
275+
.file("src/main.rs", "fn main() {}")
276+
.build();
277+
p.cargo("rustc")
278+
.masquerade_as_nightly_cargo(&["warnings"])
279+
.arg("-Zwarnings")
280+
.arg("--config")
281+
.arg("build.warnings='allow'")
282+
.arg("--")
283+
.arg("-ox.rs")
284+
.with_stderr_data(str![[r#"
285+
[COMPILING] foo v0.0.1 ([ROOT]/foo)
286+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
287+
288+
"#]])
289+
.with_status(0)
290+
.run();
291+
}

0 commit comments

Comments
 (0)