Remove final unwanted unchecked_error_guaranteed calls.

Now that error counts can't go up and down due to stashing/stealing, we
have a nice property:

  (err_count > 0) iff (an ErrorGuaranteed has been produced)

So we can now record `ErrorGuaranteed`s within `DiagCtxt` and use that
in methods like `has_error`, instead of checking that the count is
greater than 0 and calling `unchecked_error_guaranteed` to create the
`ErrorGuaranteed`.

In fact, we can record a `Vec<ErrorGuaranteed>` and use its length to
count the number, instead of maintaining a separate count.
This commit is contained in:
Nicholas Nethercote 2024-02-09 16:12:18 +11:00
parent de4d615e6b
commit 30774b0061

View File

@ -428,10 +428,14 @@ pub struct DiagCtxt {
struct DiagCtxtInner { struct DiagCtxtInner {
flags: DiagCtxtFlags, flags: DiagCtxtFlags,
/// The number of lint errors that have been emitted, including duplicates. /// The error guarantees from all emitted errors. The length gives the error count.
lint_err_count: usize, err_guars: Vec<ErrorGuaranteed>,
/// The number of non-lint errors that have been emitted, including duplicates. /// The error guarantee from all emitted lint errors. The length gives the
err_count: usize, /// lint error count.
lint_err_guars: Vec<ErrorGuaranteed>,
/// The delayed bugs and their error guarantees.
delayed_bugs: Vec<(DelayedDiagnostic, ErrorGuaranteed)>,
good_path_delayed_bugs: Vec<DelayedDiagnostic>,
/// The number of stashed errors. Unlike the other counts, this can go up /// The number of stashed errors. Unlike the other counts, this can go up
/// and down, so it doesn't guarantee anything. /// and down, so it doesn't guarantee anything.
@ -447,8 +451,6 @@ struct DiagCtxtInner {
has_printed: bool, has_printed: bool,
emitter: Box<DynEmitter>, emitter: Box<DynEmitter>,
delayed_bugs: Vec<DelayedDiagnostic>,
good_path_delayed_bugs: Vec<DelayedDiagnostic>,
/// This flag indicates that an expected diagnostic was emitted and suppressed. /// This flag indicates that an expected diagnostic was emitted and suppressed.
/// This is used for the `good_path_delayed_bugs` check. /// This is used for the `good_path_delayed_bugs` check.
suppressed_expected_diag: bool, suppressed_expected_diag: bool,
@ -560,7 +562,7 @@ impl Drop for DiagCtxtInner {
fn drop(&mut self) { fn drop(&mut self) {
self.emit_stashed_diagnostics(); self.emit_stashed_diagnostics();
if !self.has_errors() { if self.err_guars.is_empty() {
self.flush_delayed(DelayedBugKind::Normal) self.flush_delayed(DelayedBugKind::Normal)
} }
@ -604,15 +606,15 @@ pub fn with_emitter(emitter: Box<DynEmitter>) -> Self {
Self { Self {
inner: Lock::new(DiagCtxtInner { inner: Lock::new(DiagCtxtInner {
flags: DiagCtxtFlags { can_emit_warnings: true, ..Default::default() }, flags: DiagCtxtFlags { can_emit_warnings: true, ..Default::default() },
lint_err_count: 0, err_guars: Vec::new(),
err_count: 0, lint_err_guars: Vec::new(),
delayed_bugs: Vec::new(),
good_path_delayed_bugs: Vec::new(),
stashed_err_count: 0, stashed_err_count: 0,
deduplicated_err_count: 0, deduplicated_err_count: 0,
deduplicated_warn_count: 0, deduplicated_warn_count: 0,
has_printed: false, has_printed: false,
emitter, emitter,
delayed_bugs: Vec::new(),
good_path_delayed_bugs: Vec::new(),
suppressed_expected_diag: false, suppressed_expected_diag: false,
taught_diagnostics: Default::default(), taught_diagnostics: Default::default(),
emitted_diagnostic_codes: Default::default(), emitted_diagnostic_codes: Default::default(),
@ -661,14 +663,14 @@ pub fn can_emit_warnings(&self) -> bool {
/// the overall count of emitted error diagnostics. /// the overall count of emitted error diagnostics.
pub fn reset_err_count(&self) { pub fn reset_err_count(&self) {
let mut inner = self.inner.borrow_mut(); let mut inner = self.inner.borrow_mut();
inner.lint_err_count = 0;
inner.err_count = 0;
inner.stashed_err_count = 0; inner.stashed_err_count = 0;
inner.deduplicated_err_count = 0; inner.deduplicated_err_count = 0;
inner.deduplicated_warn_count = 0; inner.deduplicated_warn_count = 0;
inner.has_printed = false; inner.has_printed = false;
// actually free the underlying memory (which `clear` would not do) // actually free the underlying memory (which `clear` would not do)
inner.err_guars = Default::default();
inner.lint_err_guars = Default::default();
inner.delayed_bugs = Default::default(); inner.delayed_bugs = Default::default();
inner.good_path_delayed_bugs = Default::default(); inner.good_path_delayed_bugs = Default::default();
inner.taught_diagnostics = Default::default(); inner.taught_diagnostics = Default::default();
@ -932,7 +934,7 @@ pub fn bug(&self, msg: impl Into<DiagnosticMessage>) -> ! {
/// This excludes lint errors, delayed bugs, and stashed errors. /// This excludes lint errors, delayed bugs, and stashed errors.
#[inline] #[inline]
pub fn err_count(&self) -> usize { pub fn err_count(&self) -> usize {
self.inner.borrow().err_count self.inner.borrow().err_guars.len()
} }
/// This excludes normal errors, lint errors and delayed bugs. Unless /// This excludes normal errors, lint errors and delayed bugs. Unless
@ -946,36 +948,19 @@ pub fn stashed_err_count(&self) -> usize {
/// This excludes lint errors, delayed bugs, and stashed errors. /// This excludes lint errors, delayed bugs, and stashed errors.
pub fn has_errors(&self) -> Option<ErrorGuaranteed> { pub fn has_errors(&self) -> Option<ErrorGuaranteed> {
self.inner.borrow().has_errors().then(|| { self.inner.borrow().has_errors()
// FIXME(nnethercote) find a way to store an `ErrorGuaranteed`.
#[allow(deprecated)]
ErrorGuaranteed::unchecked_error_guaranteed()
})
} }
/// This excludes delayed bugs and stashed errors. Unless absolutely /// This excludes delayed bugs and stashed errors. Unless absolutely
/// necessary, prefer `has_errors` to this method. /// necessary, prefer `has_errors` to this method.
pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> { pub fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
let inner = self.inner.borrow(); self.inner.borrow().has_errors_or_lint_errors()
let result = inner.has_errors() || inner.lint_err_count > 0;
result.then(|| {
// FIXME(nnethercote) find a way to store an `ErrorGuaranteed`.
#[allow(deprecated)]
ErrorGuaranteed::unchecked_error_guaranteed()
})
} }
/// This excludes stashed errors. Unless absolutely necessary, prefer /// This excludes stashed errors. Unless absolutely necessary, prefer
/// `has_errors` or `has_errors_or_lint_errors` to this method. /// `has_errors` or `has_errors_or_lint_errors` to this method.
pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> { pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
let inner = self.inner.borrow(); self.inner.borrow().has_errors_or_lint_errors_or_delayed_bugs()
let result =
inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty();
result.then(|| {
// FIXME(nnethercote) find a way to store an `ErrorGuaranteed`.
#[allow(deprecated)]
ErrorGuaranteed::unchecked_error_guaranteed()
})
} }
pub fn print_error_count(&self, registry: &Registry) { pub fn print_error_count(&self, registry: &Registry) {
@ -1055,7 +1040,7 @@ pub fn print_error_count(&self, registry: &Registry) {
pub fn abort_if_errors(&self) { pub fn abort_if_errors(&self) {
let mut inner = self.inner.borrow_mut(); let mut inner = self.inner.borrow_mut();
inner.emit_stashed_diagnostics(); inner.emit_stashed_diagnostics();
if inner.has_errors() { if !inner.err_guars.is_empty() {
FatalError.raise(); FatalError.raise();
} }
} }
@ -1175,8 +1160,21 @@ pub fn emit_unused_externs(
) { ) {
let mut inner = self.inner.borrow_mut(); let mut inner = self.inner.borrow_mut();
// This "error" is an odd duck.
// - It's only produce with JSON output.
// - It's not emitted the usual way, via `emit_diagnostic`.
// - The `$message_type` field is "unused_externs" rather than the usual
// "diagnosic".
//
// We count it as a lint error because it has a lint level. The value
// of `loud` (which comes from "unused-externs" or
// "unused-externs-silent"), also affects whether it's treated like a
// hard error or not.
if loud && lint_level.is_error() { if loud && lint_level.is_error() {
inner.lint_err_count += 1; // This `unchecked_error_guaranteed` is valid. It is where the
// `ErrorGuaranteed` for unused_extern errors originates.
#[allow(deprecated)]
inner.lint_err_guars.push(ErrorGuaranteed::unchecked_error_guaranteed());
inner.panic_if_treat_err_as_bug(); inner.panic_if_treat_err_as_bug();
} }
@ -1236,7 +1234,7 @@ pub fn flush_delayed(&self) {
impl DiagCtxtInner { impl DiagCtxtInner {
/// Emit all stashed diagnostics. /// Emit all stashed diagnostics.
fn emit_stashed_diagnostics(&mut self) { fn emit_stashed_diagnostics(&mut self) {
let has_errors = self.has_errors(); let has_errors = !self.err_guars.is_empty();
for (_, diag) in std::mem::take(&mut self.stashed_diagnostics).into_iter() { for (_, diag) in std::mem::take(&mut self.stashed_diagnostics).into_iter() {
// Decrement the count tracking the stash; emitting will increment it. // Decrement the count tracking the stash; emitting will increment it.
if diag.is_error() { if diag.is_error() {
@ -1298,9 +1296,13 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuarant
// when an error is first emitted, also), but maybe there's a case // when an error is first emitted, also), but maybe there's a case
// in which that's not sound? otherwise this is really inefficient. // in which that's not sound? otherwise this is really inefficient.
let backtrace = std::backtrace::Backtrace::capture(); let backtrace = std::backtrace::Backtrace::capture();
self.delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); // This `unchecked_error_guaranteed` is valid. It is where the
// `ErrorGuaranteed` for delayed bugs originates.
#[allow(deprecated)] #[allow(deprecated)]
return Some(ErrorGuaranteed::unchecked_error_guaranteed()); let guar = ErrorGuaranteed::unchecked_error_guaranteed();
self.delayed_bugs
.push((DelayedDiagnostic::with_backtrace(diagnostic, backtrace), guar));
return Some(guar);
} }
GoodPathDelayedBug => { GoodPathDelayedBug => {
let backtrace = std::backtrace::Backtrace::capture(); let backtrace = std::backtrace::Backtrace::capture();
@ -1334,7 +1336,6 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuarant
!self.emitted_diagnostics.insert(diagnostic_hash) !self.emitted_diagnostics.insert(diagnostic_hash)
}; };
let level = diagnostic.level;
let is_error = diagnostic.is_error(); let is_error = diagnostic.is_error();
let is_lint = diagnostic.is_lint.is_some(); let is_lint = diagnostic.is_lint.is_some();
@ -1373,36 +1374,47 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuarant
} }
if is_error { if is_error {
// This `unchecked_error_guaranteed` is valid. It is where the
// `ErrorGuaranteed` for errors and lint errors originates.
#[allow(deprecated)]
let guar = ErrorGuaranteed::unchecked_error_guaranteed();
guaranteed = Some(guar);
if is_lint { if is_lint {
self.lint_err_count += 1; self.lint_err_guars.push(guar);
} else { } else {
self.err_count += 1; self.err_guars.push(guar);
} }
self.panic_if_treat_err_as_bug(); self.panic_if_treat_err_as_bug();
} }
#[allow(deprecated)]
if level == Level::Error {
guaranteed = Some(ErrorGuaranteed::unchecked_error_guaranteed());
}
}); });
guaranteed guaranteed
} }
fn treat_err_as_bug(&self) -> bool { fn treat_err_as_bug(&self) -> bool {
self.flags.treat_err_as_bug.is_some_and(|c| self.err_count + self.lint_err_count >= c.get()) self.flags
.treat_err_as_bug
.is_some_and(|c| self.err_guars.len() + self.lint_err_guars.len() >= c.get())
} }
// Use this one before incrementing `err_count`. // Use this one before incrementing `err_count`.
fn treat_next_err_as_bug(&self) -> bool { fn treat_next_err_as_bug(&self) -> bool {
self.flags self.flags
.treat_err_as_bug .treat_err_as_bug
.is_some_and(|c| self.err_count + self.lint_err_count + 1 >= c.get()) .is_some_and(|c| self.err_guars.len() + self.lint_err_guars.len() + 1 >= c.get())
} }
fn has_errors(&self) -> bool { fn has_errors(&self) -> Option<ErrorGuaranteed> {
self.err_count > 0 self.err_guars.get(0).copied()
}
fn has_errors_or_lint_errors(&self) -> Option<ErrorGuaranteed> {
self.has_errors().or_else(|| self.lint_err_guars.get(0).copied())
}
fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option<ErrorGuaranteed> {
self.has_errors_or_lint_errors()
.or_else(|| self.delayed_bugs.get(0).map(|(_, guar)| guar).copied())
} }
fn failure_note(&mut self, msg: impl Into<DiagnosticMessage>) { fn failure_note(&mut self, msg: impl Into<DiagnosticMessage>) {
@ -1412,7 +1424,7 @@ fn failure_note(&mut self, msg: impl Into<DiagnosticMessage>) {
fn flush_delayed(&mut self, kind: DelayedBugKind) { fn flush_delayed(&mut self, kind: DelayedBugKind) {
let (bugs, note1) = match kind { let (bugs, note1) = match kind {
DelayedBugKind::Normal => ( DelayedBugKind::Normal => (
std::mem::take(&mut self.delayed_bugs), std::mem::take(&mut self.delayed_bugs).into_iter().map(|(b, _)| b).collect(),
"no errors encountered even though delayed bugs were created", "no errors encountered even though delayed bugs were created",
), ),
DelayedBugKind::GoodPath => ( DelayedBugKind::GoodPath => (
@ -1477,7 +1489,7 @@ fn flush_delayed(&mut self, kind: DelayedBugKind) {
fn panic_if_treat_err_as_bug(&self) { fn panic_if_treat_err_as_bug(&self) {
if self.treat_err_as_bug() { if self.treat_err_as_bug() {
let n = self.flags.treat_err_as_bug.map(|c| c.get()).unwrap(); let n = self.flags.treat_err_as_bug.map(|c| c.get()).unwrap();
assert_eq!(n, self.err_count + self.lint_err_count); assert_eq!(n, self.err_guars.len() + self.lint_err_guars.len());
if n == 1 { if n == 1 {
panic!("aborting due to `-Z treat-err-as-bug=1`"); panic!("aborting due to `-Z treat-err-as-bug=1`");
} else { } else {