Tighten up ErrorGuaranteed handling.

- In `emit_producing_error_guaranteed`, only allow `Level::Error`.
- In `emit_diagnostic`, only produce `ErrorGuaranteed` for `Level` and
  `DelayedBug`. (Not `Bug` or `Fatal`. They don't need it, because the
  relevant `emit` methods abort.)
- Add/update various comments.
This commit is contained in:
Nicholas Nethercote 2024-02-07 10:26:50 +11:00
parent 83adf883a2
commit 97c157fe1e
3 changed files with 22 additions and 13 deletions

View File

@ -99,16 +99,20 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
} }
/// `ErrorGuaranteed::emit_producing_guarantee` uses this. /// `ErrorGuaranteed::emit_producing_guarantee` uses this.
// FIXME(eddyb) make `ErrorGuaranteed` impossible to create outside `.emit()`.
fn emit_producing_error_guaranteed(mut self) -> ErrorGuaranteed { fn emit_producing_error_guaranteed(mut self) -> ErrorGuaranteed {
let diag = self.take_diag(); let diag = self.take_diag();
// Only allow a guarantee if the `level` wasn't switched to a // The only error levels that produce `ErrorGuaranteed` are
// non-error. The field isn't `pub`, but the whole `Diagnostic` can be // `Error` and `DelayedBug`. But `DelayedBug` should never occur here
// overwritten with a new one, thanks to `DerefMut`. // because delayed bugs have their level changed to `Bug` when they are
// actually printed, so they produce an ICE.
//
// (Also, even though `level` isn't `pub`, the whole `Diagnostic` could
// be overwritten with a new one thanks to `DerefMut`. So this assert
// protects against that, too.)
assert!( assert!(
diag.is_error(), matches!(diag.level, Level::Error | Level::DelayedBug),
"emitted non-error ({:?}) diagnostic from `DiagnosticBuilder<ErrorGuaranteed>`", "invalid diagnostic level ({:?})",
diag.level, diag.level,
); );

View File

@ -931,6 +931,7 @@ impl DiagCtxt {
/// This excludes lint errors and delayed bugs. /// This excludes lint errors and delayed bugs.
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().then(|| {
// FIXME(nnethercote) find a way to store an `ErrorGuaranteed`.
#[allow(deprecated)] #[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted() ErrorGuaranteed::unchecked_claim_error_was_emitted()
}) })
@ -942,6 +943,7 @@ impl DiagCtxt {
let inner = self.inner.borrow(); let inner = self.inner.borrow();
let result = inner.has_errors() || inner.lint_err_count > 0; let result = inner.has_errors() || inner.lint_err_count > 0;
result.then(|| { result.then(|| {
// FIXME(nnethercote) find a way to store an `ErrorGuaranteed`.
#[allow(deprecated)] #[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted() ErrorGuaranteed::unchecked_claim_error_was_emitted()
}) })
@ -954,6 +956,7 @@ impl DiagCtxt {
let result = let result =
inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty(); inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty();
result.then(|| { result.then(|| {
// FIXME(nnethercote) find a way to store an `ErrorGuaranteed`.
#[allow(deprecated)] #[allow(deprecated)]
ErrorGuaranteed::unchecked_claim_error_was_emitted() ErrorGuaranteed::unchecked_claim_error_was_emitted()
}) })
@ -1238,6 +1241,7 @@ impl DiagCtxtInner {
} }
} }
// Return value is only `Some` if the level is `Error` or `DelayedBug`.
fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuaranteed> { fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
assert!(diagnostic.level.can_be_top_or_sub().0); assert!(diagnostic.level.can_be_top_or_sub().0);
@ -1316,6 +1320,7 @@ impl DiagCtxtInner {
!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();
@ -1352,6 +1357,7 @@ impl DiagCtxtInner {
self.emitter.emit_diagnostic(diagnostic); self.emitter.emit_diagnostic(diagnostic);
} }
if is_error { if is_error {
if is_lint { if is_lint {
self.lint_err_count += 1; self.lint_err_count += 1;
@ -1359,12 +1365,12 @@ impl DiagCtxtInner {
self.err_count += 1; self.err_count += 1;
} }
self.panic_if_treat_err_as_bug(); self.panic_if_treat_err_as_bug();
}
#[allow(deprecated)] #[allow(deprecated)]
{ if level == Level::Error {
guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted());
} }
}
}); });
guaranteed guaranteed

View File

@ -2477,9 +2477,8 @@ where
pub struct ErrorGuaranteed(()); pub struct ErrorGuaranteed(());
impl ErrorGuaranteed { impl ErrorGuaranteed {
/// To be used only if you really know what you are doing... ideally, we would find a way to /// Don't use this outside of `DiagCtxtInner::emit_diagnostic`!
/// eliminate all calls to this method. #[deprecated = "should only be used in `DiagCtxtInner::emit_diagnostic`"]
#[deprecated = "`Session::span_delayed_bug` should be preferred over this function"]
pub fn unchecked_claim_error_was_emitted() -> Self { pub fn unchecked_claim_error_was_emitted() -> Self {
ErrorGuaranteed(()) ErrorGuaranteed(())
} }