Skip to content

Commit 56c3265

Browse files
committed
Replace warn_count.
There are four functions that adjust error and warning counts: - `stash_diagnostic` (increment) - `steal_diagnostic` (decrement) - `emit_stashed_diagnostics) (decrement) - `emit_diagnostic` (increment) The first three all behave similarly, and only update `warn_count` for forced warnings. But the last one updates `warn_count` for both forced and non-forced warnings. Seems like a bug. How should it be fixed? Well, `warn_count` is only used in one place: `DiagCtxtInner::drop`, where it's part of the condition relating to the printing of `good_path_delayed_bugs`. The intention of that condition seems to be "have any errors been printed?" so this commit replaces `warn_count` with `has_printed`, which is set when printing occurs. This is simpler than all the ahead-of-time incrementing and decrementing.
1 parent 8866780 commit 56c3265

File tree

1 file changed

+12
-29
lines changed
  • compiler/rustc_errors/src

1 file changed

+12
-29
lines changed

compiler/rustc_errors/src/lib.rs

+12-29
Original file line numberDiff line numberDiff line change
@@ -428,10 +428,12 @@ struct DiagCtxtInner {
428428
/// This is not necessarily the count that's reported to the user once
429429
/// compilation ends.
430430
err_count: usize,
431-
warn_count: usize,
432431
deduplicated_err_count: usize,
433432
/// The warning count, used for a recap upon finishing
434433
deduplicated_warn_count: usize,
434+
/// Has this diagnostic context printed any diagnostics? (I.e. has
435+
/// `self.emitter.emit_diagnostic()` been called?
436+
has_printed: bool,
435437

436438
emitter: Box<DynEmitter>,
437439
span_delayed_bugs: Vec<DelayedDiagnostic>,
@@ -548,8 +550,7 @@ impl Drop for DiagCtxtInner {
548550
// instead of "require some error happened". Sadly that isn't ideal, as
549551
// lints can be `#[allow]`'d, potentially leading to this triggering.
550552
// Also, "good path" should be replaced with a better naming.
551-
let has_any_message = self.err_count > 0 || self.lint_err_count > 0 || self.warn_count > 0;
552-
if !has_any_message && !self.suppressed_expected_diag && !std::thread::panicking() {
553+
if !self.has_printed && !self.suppressed_expected_diag && !std::thread::panicking() {
553554
let bugs = std::mem::replace(&mut self.good_path_delayed_bugs, Vec::new());
554555
self.flush_delayed(
555556
bugs,
@@ -595,9 +596,9 @@ impl DiagCtxt {
595596
flags: DiagCtxtFlags { can_emit_warnings: true, ..Default::default() },
596597
lint_err_count: 0,
597598
err_count: 0,
598-
warn_count: 0,
599599
deduplicated_err_count: 0,
600600
deduplicated_warn_count: 0,
601+
has_printed: false,
601602
emitter,
602603
span_delayed_bugs: Vec::new(),
603604
good_path_delayed_bugs: Vec::new(),
@@ -650,9 +651,9 @@ impl DiagCtxt {
650651
let mut inner = self.inner.borrow_mut();
651652
inner.lint_err_count = 0;
652653
inner.err_count = 0;
653-
inner.warn_count = 0;
654654
inner.deduplicated_err_count = 0;
655655
inner.deduplicated_warn_count = 0;
656+
inner.has_printed = false;
656657

657658
// actually free the underlying memory (which `clear` would not do)
658659
inner.span_delayed_bugs = Default::default();
@@ -676,11 +677,6 @@ impl DiagCtxt {
676677
} else {
677678
inner.err_count += 1;
678679
}
679-
} else {
680-
// Warnings are only automatically flushed if they're forced.
681-
if diag.is_force_warn() {
682-
inner.warn_count += 1;
683-
}
684680
}
685681

686682
// FIXME(Centril, #69537): Consider reintroducing panic on overwriting a stashed diagnostic
@@ -700,10 +696,6 @@ impl DiagCtxt {
700696
} else {
701697
inner.err_count -= 1;
702698
}
703-
} else {
704-
if diag.is_force_warn() {
705-
inner.warn_count -= 1;
706-
}
707699
}
708700
Some(DiagnosticBuilder::new_diagnostic(self, diag))
709701
}
@@ -1249,15 +1241,11 @@ impl DiagCtxtInner {
12491241
self.err_count -= 1;
12501242
}
12511243
} else {
1252-
if diag.is_force_warn() {
1253-
self.warn_count -= 1;
1254-
} else {
1255-
// Unless they're forced, don't flush stashed warnings when
1256-
// there are errors, to avoid causing warning overload. The
1257-
// stash would've been stolen already if it were important.
1258-
if has_errors {
1259-
continue;
1260-
}
1244+
// Unless they're forced, don't flush stashed warnings when
1245+
// there are errors, to avoid causing warning overload. The
1246+
// stash would've been stolen already if it were important.
1247+
if !diag.is_force_warn() && has_errors {
1248+
continue;
12611249
}
12621250
}
12631251
let reported_this = self.emit_diagnostic(diag);
@@ -1361,6 +1349,7 @@ impl DiagCtxtInner {
13611349
} else if matches!(diagnostic.level, ForceWarning(_) | Warning) {
13621350
self.deduplicated_warn_count += 1;
13631351
}
1352+
self.has_printed = true;
13641353
}
13651354
if diagnostic.is_error() {
13661355
if diagnostic.level == Error && diagnostic.is_lint {
@@ -1373,8 +1362,6 @@ impl DiagCtxtInner {
13731362
{
13741363
guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted());
13751364
}
1376-
} else {
1377-
self.bump_warn_count();
13781365
}
13791366
});
13801367

@@ -1470,10 +1457,6 @@ impl DiagCtxtInner {
14701457
self.panic_if_treat_err_as_bug();
14711458
}
14721459

1473-
fn bump_warn_count(&mut self) {
1474-
self.warn_count += 1;
1475-
}
1476-
14771460
fn panic_if_treat_err_as_bug(&self) {
14781461
if self.treat_err_as_bug() {
14791462
match (

0 commit comments

Comments
 (0)