Skip to content

Commit

Permalink
Rollup merge of #121819 - nnethercote:fix-121812, r=oli-obk
Browse files Browse the repository at this point in the history
Handle stashing of delayed bugs

By just emitting them immediately, because it does happen in practice, when errors are downgraded to delayed bugs.

We already had one case in `lint.rs` where we handled this at the callsite. This commit changes things so it's handled within `stash_diagnostic` instead, because #121812 identified a second case, and it's possible there are more.

Fixes #121812.

r? ````@oli-obk````
  • Loading branch information
matthiaskrgr authored Mar 1, 2024
2 parents b5ef517 + 44f0043 commit 47a491d
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 28 deletions.
50 changes: 32 additions & 18 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -712,33 +712,47 @@ impl DiagCtxt {
/// Stashes a diagnostic for possible later improvement in a different,
/// later stage of the compiler. Possible actions depend on the diagnostic
/// level:
/// - Level::Bug, Level:Fatal: not allowed, will trigger a panic.
/// - Level::Error: immediately counted as an error that has occurred, because it
/// is guaranteed to be emitted eventually. Can be later accessed with the
/// provided `span` and `key` through
/// [`DiagCtxt::try_steal_modify_and_emit_err`] or
/// [`DiagCtxt::try_steal_replace_and_emit_err`]. These do not allow
/// cancellation or downgrading of the error. Returns
/// `Some(ErrorGuaranteed)`.
/// - Level::DelayedBug: this does happen occasionally with errors that are
/// downgraded to delayed bugs. It is not stashed, but immediately
/// emitted as a delayed bug. This is because stashing it would cause it
/// to be counted by `err_count` which we don't want. It doesn't matter
/// that we cannot steal and improve it later, because it's not a
/// user-facing error. Returns `Some(ErrorGuaranteed)` as is normal for
/// delayed bugs.
/// - Level::Warning and lower (i.e. !is_error()): can be accessed with the
/// provided `span` and `key` through [`DiagCtxt::steal_non_err()`]. This
/// allows cancelling and downgrading of the diagnostic. Returns `None`.
/// - Others: not allowed, will trigger a panic.
pub fn stash_diagnostic(
&self,
span: Span,
key: StashKey,
diag: DiagInner,
) -> Option<ErrorGuaranteed> {
let guar = if diag.level() == Level::Error {
// This `unchecked_error_guaranteed` is valid. It is where the
// `ErrorGuaranteed` for stashed errors originates. See
// `DiagCtxtInner::drop`.
#[allow(deprecated)]
Some(ErrorGuaranteed::unchecked_error_guaranteed())
} else if !diag.is_error() {
None
} else {
self.span_bug(span, format!("invalid level in `stash_diagnostic`: {}", diag.level));
let guar = match diag.level {
Bug | Fatal => {
self.span_bug(
span,
format!("invalid level in `stash_diagnostic`: {:?}", diag.level),
);
}
Error => {
// This `unchecked_error_guaranteed` is valid. It is where the
// `ErrorGuaranteed` for stashed errors originates. See
// `DiagCtxtInner::drop`.
#[allow(deprecated)]
Some(ErrorGuaranteed::unchecked_error_guaranteed())
}
DelayedBug => return self.inner.borrow_mut().emit_diagnostic(diag),
ForceWarning(_) | Warning | Note | OnceNote | Help | OnceHelp | FailureNote | Allow
| Expect(_) => None,
};

// FIXME(Centril, #69537): Consider reintroducing panic on overwriting a stashed diagnostic
Expand Down Expand Up @@ -780,11 +794,11 @@ impl DiagCtxt {
let err = self.inner.borrow_mut().stashed_diagnostics.swap_remove(&key);
err.map(|(err, guar)| {
// The use of `::<ErrorGuaranteed>` is safe because level is `Level::Error`.
assert_eq!(err.level, Level::Error);
assert_eq!(err.level, Error);
assert!(guar.is_some());
let mut err = Diag::<ErrorGuaranteed>::new_diagnostic(self, err);
modify_err(&mut err);
assert_eq!(err.level, Level::Error);
assert_eq!(err.level, Error);
err.emit()
})
}
Expand All @@ -803,7 +817,7 @@ impl DiagCtxt {
let old_err = self.inner.borrow_mut().stashed_diagnostics.swap_remove(&key);
match old_err {
Some((old_err, guar)) => {
assert_eq!(old_err.level, Level::Error);
assert_eq!(old_err.level, Error);
assert!(guar.is_some());
// Because `old_err` has already been counted, it can only be
// safely cancelled because the `new_err` supplants it.
Expand Down Expand Up @@ -1367,7 +1381,7 @@ impl DiagCtxtInner {
}

if diagnostic.has_future_breakage() {
// Future breakages aren't emitted if they're Level::Allow,
// Future breakages aren't emitted if they're `Level::Allow`,
// but they still need to be constructed and stashed below,
// so they'll trigger the must_produce_diag check.
self.suppressed_expected_diag = true;
Expand Down Expand Up @@ -1453,7 +1467,7 @@ impl DiagCtxtInner {
diagnostic.children.extract_if(already_emitted_sub).for_each(|_| {});
if already_emitted {
let msg = "duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`";
diagnostic.sub(Level::Note, msg, MultiSpan::new());
diagnostic.sub(Note, msg, MultiSpan::new());
}

if is_error {
Expand Down Expand Up @@ -1623,7 +1637,7 @@ impl DiagCtxtInner {
bug.arg("level", bug.level);
let msg = crate::fluent_generated::errors_invalid_flushed_delayed_diagnostic_level;
let msg = self.eagerly_translate_for_subdiag(&bug, msg); // after the `arg` call
bug.sub(Level::Note, msg, bug.span.primary_span().unwrap().into());
bug.sub(Note, msg, bug.span.primary_span().unwrap().into());
}
bug.level = Bug;

Expand Down Expand Up @@ -1671,7 +1685,7 @@ impl DelayedDiagInner {
diag.arg("emitted_at", diag.emitted_at.clone());
diag.arg("note", self.note);
let msg = dcx.eagerly_translate_for_subdiag(&diag, msg); // after the `arg` calls
diag.sub(Level::Note, msg, diag.span.primary_span().unwrap_or(DUMMY_SP).into());
diag.sub(Note, msg, diag.span.primary_span().unwrap_or(DUMMY_SP).into());
diag
}
}
Expand Down
12 changes: 2 additions & 10 deletions compiler/rustc_hir_analysis/src/astconv/lint.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_ast::TraitObjectSyntax;
use rustc_errors::{codes::*, Diag, EmissionGuarantee, Level, StashKey};
use rustc_errors::{codes::*, Diag, EmissionGuarantee, StashKey};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_lint_defs::{builtin::BARE_TRAIT_OBJECTS, Applicability};
Expand Down Expand Up @@ -237,15 +237,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
}
// check if the impl trait that we are considering is a impl of a local trait
self.maybe_lint_blanket_trait_impl(self_ty, &mut diag);
match diag.level() {
Level::Error => {
diag.stash(self_ty.span, StashKey::TraitMissingMethod);
}
Level::DelayedBug => {
diag.emit();
}
_ => unreachable!(),
}
diag.stash(self_ty.span, StashKey::TraitMissingMethod);
} else {
let msg = "trait objects without an explicit `dyn` are deprecated";
tcx.node_span_lint(BARE_TRAIT_OBJECTS, self_ty.hir_id, self_ty.span, msg, |lint| {
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/typeck/invalid-stashed-level-issue-121812.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
union U {
a: u16,
b: [u8; 3],
}

fn main() {
_ = U { b: [()] }; //~ ERROR mismatched types
}
9 changes: 9 additions & 0 deletions tests/ui/typeck/invalid-stashed-level-issue-121812.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0308]: mismatched types
--> $DIR/invalid-stashed-level-issue-121812.rs:7:17
|
LL | _ = U { b: [()] };
| ^^ expected `u8`, found `()`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0308`.

0 comments on commit 47a491d

Please sign in to comment.