diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index c14bd142efacc..90ff17a889f68 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -287,7 +287,7 @@ impl<'tcx> fmt::Display for FrameInfo<'tcx> { if tcx.def_key(self.instance.def_id()).disambiguated_data.data == DefPathData::Closure { write!(f, "inside closure") } else { - // Note: this triggers a `good_path_delayed_bug` state, which means that if we ever + // Note: this triggers a `weak_delayed_bug` state, which means that if we ever // get here we must emit a diagnostic. We should never display a `FrameInfo` unless // we actually want to emit a warning or error to the user. write!(f, "inside `{}`", self.instance) @@ -303,7 +303,7 @@ impl<'tcx> FrameInfo<'tcx> { errors::FrameNote { where_: "closure", span, instance: String::new(), times: 0 } } else { let instance = format!("{}", self.instance); - // Note: this triggers a `good_path_delayed_bug` state, which means that if we ever get + // Note: this triggers a `weak_delayed_bug` state, which means that if we ever get // here we must emit a diagnostic. We should never display a `FrameInfo` unless we // actually want to emit a warning or error to the user. errors::FrameNote { where_: "instance", span, instance, times: 0 } diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 8fd7c5764797e..54d7e6dd80c4c 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -378,7 +378,7 @@ impl From> for DiagnosticMessage { } } -/// A workaround for "good path" ICEs when formatting types in disabled lints. +/// A workaround for weak_delayed_bug ICEs when formatting types in disabled lints. /// /// Delays formatting until `.into(): DiagnosticMessage` is used. pub struct DelayDm(pub F); diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 949f52ef6b586..d8041d459bea3 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -85,7 +85,9 @@ fn source_string(file: Lrc, line: &Line) -> String { /// Maps `Diagnostic::Level` to `snippet::AnnotationType` fn annotation_type_for_level(level: Level) -> AnnotationType { match level { - Level::Bug | Level::DelayedBug(_) | Level::Fatal | Level::Error => AnnotationType::Error, + Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug | Level::WeakDelayedBug => { + AnnotationType::Error + } Level::ForceWarning(_) | Level::Warning => AnnotationType::Warning, Level::Note | Level::OnceNote => AnnotationType::Note, Level::Help | Level::OnceHelp => AnnotationType::Help, diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index d888c4540b9fa..8394212742105 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -1,8 +1,7 @@ use crate::snippet::Style; use crate::{ - CodeSuggestion, DelayedBugKind, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, - ErrCode, Level, MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart, - SuggestionStyle, + CodeSuggestion, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, ErrCode, Level, + MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart, SuggestionStyle, }; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_error_messages::fluent_value_from_str_list_sep_by_and; @@ -235,14 +234,11 @@ impl Diagnostic { pub fn is_error(&self) -> bool { match self.level { - Level::Bug - | Level::DelayedBug(DelayedBugKind::Normal) - | Level::Fatal - | Level::Error => true, + Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => true, - Level::ForceWarning(_) + Level::WeakDelayedBug + | Level::ForceWarning(_) | Level::Warning - | Level::DelayedBug(DelayedBugKind::GoodPath) | Level::Note | Level::OnceNote | Level::Help @@ -306,11 +302,11 @@ impl Diagnostic { #[track_caller] pub fn downgrade_to_delayed_bug(&mut self) { assert!( - matches!(self.level, Level::Error | Level::DelayedBug(_)), + matches!(self.level, Level::Error | Level::DelayedBug), "downgrade_to_delayed_bug: cannot downgrade {:?} to DelayedBug: not an error", self.level ); - self.level = Level::DelayedBug(DelayedBugKind::Normal); + self.level = Level::DelayedBug; } /// Appends a labeled span to the diagnostic. diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 9f76c1dd248b0..760e66fd34ad5 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -2118,6 +2118,7 @@ impl HumanEmitter { } if !self.short_message { for child in children { + assert!(child.level.can_be_top_or_sub().1); let span = &child.span; if let Err(err) = self.emit_messages_default_inner( span, diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 357688354fbd9..84cff4de0eac6 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -440,10 +440,10 @@ struct DiagCtxtInner { has_printed: bool, emitter: Box, - span_delayed_bugs: Vec, - good_path_delayed_bugs: Vec, + delayed_bugs: Vec, + weak_delayed_bugs: Vec, /// 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 `weak_delayed_bugs` check. suppressed_expected_diag: bool, /// This set contains the code of all emitted diagnostics to avoid @@ -524,10 +524,9 @@ fn default_track_diagnostic(diag: Diagnostic, f: &mut dyn FnMut(Diagnostic)) { pub static TRACK_DIAGNOSTIC: AtomicRef = AtomicRef::new(&(default_track_diagnostic as _)); -#[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)] -pub enum DelayedBugKind { +enum DelayedBugKind { Normal, - GoodPath, + Weak, } #[derive(Copy, Clone, Default)] @@ -558,13 +557,8 @@ impl Drop for DiagCtxtInner { self.flush_delayed(DelayedBugKind::Normal) } - // FIXME(eddyb) this explains what `good_path_delayed_bugs` are! - // They're `span_delayed_bugs` but for "require some diagnostic happened" - // instead of "require some error happened". Sadly that isn't ideal, as - // lints can be `#[allow]`'d, potentially leading to this triggering. - // Also, "good path" should be replaced with a better naming. if !self.has_printed && !self.suppressed_expected_diag && !std::thread::panicking() { - self.flush_delayed(DelayedBugKind::GoodPath); + self.flush_delayed(DelayedBugKind::Weak); } if self.check_unstable_expect_diagnostics { @@ -609,8 +603,8 @@ impl DiagCtxt { deduplicated_warn_count: 0, has_printed: false, emitter, - span_delayed_bugs: Vec::new(), - good_path_delayed_bugs: Vec::new(), + delayed_bugs: Vec::new(), + weak_delayed_bugs: Vec::new(), suppressed_expected_diag: false, taught_diagnostics: Default::default(), emitted_diagnostic_codes: Default::default(), @@ -665,8 +659,8 @@ impl DiagCtxt { inner.has_printed = false; // actually free the underlying memory (which `clear` would not do) - inner.span_delayed_bugs = Default::default(); - inner.good_path_delayed_bugs = Default::default(); + inner.delayed_bugs = Default::default(); + inner.weak_delayed_bugs = Default::default(); inner.taught_diagnostics = Default::default(); inner.emitted_diagnostic_codes = Default::default(); inner.emitted_diagnostics = Default::default(); @@ -866,8 +860,7 @@ impl DiagCtxt { /// directly). #[track_caller] pub fn delayed_bug(&self, msg: impl Into) -> ErrorGuaranteed { - DiagnosticBuilder::::new(self, DelayedBug(DelayedBugKind::Normal), msg) - .emit() + DiagnosticBuilder::::new(self, DelayedBug, msg).emit() } /// Like `delayed_bug`, but takes an additional span. @@ -880,15 +873,12 @@ impl DiagCtxt { sp: impl Into, msg: impl Into, ) -> ErrorGuaranteed { - DiagnosticBuilder::::new(self, DelayedBug(DelayedBugKind::Normal), msg) - .with_span(sp) - .emit() + DiagnosticBuilder::::new(self, DelayedBug, msg).with_span(sp).emit() } - // FIXME(eddyb) note the comment inside `impl Drop for DiagCtxtInner`, that's - // where the explanation of what "good path" is (also, it should be renamed). - pub fn good_path_delayed_bug(&self, msg: impl Into) { - DiagnosticBuilder::<()>::new(self, DelayedBug(DelayedBugKind::GoodPath), msg).emit() + /// Ensures that a diagnostic is printed. See `Level::WeakDelayedBug`. + pub fn weak_delayed_bug(&self, msg: impl Into) { + DiagnosticBuilder::<()>::new(self, WeakDelayedBug, msg).emit() } #[track_caller] @@ -962,7 +952,7 @@ impl DiagCtxt { pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option { let inner = self.inner.borrow(); let result = - inner.has_errors() || inner.lint_err_count > 0 || !inner.span_delayed_bugs.is_empty(); + inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty(); result.then(|| { #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() @@ -1248,6 +1238,8 @@ impl DiagCtxtInner { } fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { + assert!(diagnostic.level.can_be_top_or_sub().0); + if let Some(expectation_id) = diagnostic.level.get_expectation_id() { // The `LintExpectationId` can be stable or unstable depending on when it was created. // Diagnostics created before the definition of `HirId`s are unstable and can not yet @@ -1264,32 +1256,33 @@ impl DiagCtxtInner { self.future_breakage_diagnostics.push(diagnostic.clone()); } - if matches!(diagnostic.level, DelayedBug(_)) && self.flags.eagerly_emit_delayed_bugs { + if matches!(diagnostic.level, DelayedBug | WeakDelayedBug) + && self.flags.eagerly_emit_delayed_bugs + { diagnostic.level = Error; } match diagnostic.level { - // This must come after the possible promotion of `DelayedBug` to `Error` above. + // This must come after the possible promotion of `DelayedBug`/`WeakDelayedBug` to + // `Error` above. Fatal | Error if self.treat_next_err_as_bug() => { diagnostic.level = Bug; } - DelayedBug(DelayedBugKind::Normal) => { + DelayedBug => { // FIXME(eddyb) this should check for `has_errors` and stop pushing - // once *any* errors were emitted (and truncate `span_delayed_bugs` + // once *any* errors were emitted (and truncate `delayed_bugs` // when an error is first emitted, also), but maybe there's a case // in which that's not sound? otherwise this is really inefficient. let backtrace = std::backtrace::Backtrace::capture(); - self.span_delayed_bugs + self.delayed_bugs .push(DelayedDiagnostic::with_backtrace(diagnostic.clone(), backtrace)); - #[allow(deprecated)] return Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); } - DelayedBug(DelayedBugKind::GoodPath) => { + WeakDelayedBug => { let backtrace = std::backtrace::Backtrace::capture(); - self.good_path_delayed_bugs + self.weak_delayed_bugs .push(DelayedDiagnostic::with_backtrace(diagnostic.clone(), backtrace)); - return None; } Warning if !self.flags.can_emit_warnings => { @@ -1395,12 +1388,12 @@ impl DiagCtxtInner { fn flush_delayed(&mut self, kind: DelayedBugKind) { let (bugs, note1) = match kind { DelayedBugKind::Normal => ( - std::mem::take(&mut self.span_delayed_bugs), - "no errors encountered even though `span_delayed_bug` issued", + std::mem::take(&mut self.delayed_bugs), + "no errors encountered even though delayed bugs were created", ), - DelayedBugKind::GoodPath => ( - std::mem::take(&mut self.good_path_delayed_bugs), - "no warnings or errors encountered even though `good_path_delayed_bugs` issued", + DelayedBugKind::Weak => ( + std::mem::take(&mut self.weak_delayed_bugs), + "no warnings or errors encountered even though weak delayed bugs were created", ), }; let note2 = "those delayed bugs will now be shown as internal compiler errors"; @@ -1439,8 +1432,8 @@ impl DiagCtxtInner { let mut bug = if backtrace || self.ice_file.is_none() { bug.decorate() } else { bug.inner }; - // "Undelay" the `DelayedBug`s (into plain `Bug`s). - if !matches!(bug.level, DelayedBug(_)) { + // "Undelay" the delayed bugs (into plain `Bug`s). + if !matches!(bug.level, DelayedBug | WeakDelayedBug) { // NOTE(eddyb) not panicking here because we're already producing // an ICE, and the more information the merrier. bug.subdiagnostic(InvalidFlushedDelayedDiagnosticLevel { @@ -1506,85 +1499,91 @@ impl DelayedDiagnostic { } } +/// Level is_error EmissionGuarantee Top-level Sub Lint-only +/// ----- -------- ----------------- --------- --- --------- +/// Bug yes BugAbort yes - - +/// Fatal yes FatalAbort/FatalError [1] yes - - +/// Error yes ErrorGuaranteed yes - - +/// DelayedBug yes ErrorGuaranteed yes - - +/// WeakDelayedBug - () yes - - +/// ForceWarning - () yes - yes +/// Warning - () yes yes - +/// Note - () rare yes - +/// OnceNote - () - rare - +/// Help - () rare yes - +/// OnceHelp - () - rare - +/// FailureNote - () rare - - +/// Allow - () yes - yes +/// Expect - () yes - yes +/// +/// [1] `FatalAbort` normally, `FatalError` in the non-aborting "almost fatal" case that is +/// occasionally used. +/// #[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)] pub enum Level { /// For bugs in the compiler. Manifests as an ICE (internal compiler error) panic. - /// - /// Its `EmissionGuarantee` is `BugAbort`. Bug, - /// This is a strange one: lets you register an error without emitting it. If compilation ends - /// without any other errors occurring, this will be emitted as a bug. Otherwise, it will be - /// silently dropped. I.e. "expect other errors are emitted" semantics. Useful on code paths - /// that should only be reached when compiling erroneous code. - /// - /// Its `EmissionGuarantee` is `ErrorGuaranteed` for `Normal` delayed bugs, and `()` for - /// `GoodPath` delayed bugs. - DelayedBug(DelayedBugKind), - /// An error that causes an immediate abort. Used for things like configuration errors, /// internal overflows, some file operation errors. - /// - /// Its `EmissionGuarantee` is `FatalAbort`, except in the non-aborting "almost fatal" case - /// that is occasionally used, where it is `FatalError`. Fatal, /// An error in the code being compiled, which prevents compilation from finishing. This is the /// most common case. - /// - /// Its `EmissionGuarantee` is `ErrorGuaranteed`. Error, + /// This is a strange one: lets you register an error without emitting it. If compilation ends + /// without any other errors occurring, this will be emitted as a bug. Otherwise, it will be + /// silently dropped. I.e. "expect other errors are emitted" semantics. Useful on code paths + /// that should only be reached when compiling erroneous code. + DelayedBug, + + /// Like `DelayedBug`, but weaker: lets you register an error without emitting it. If + /// compilation ends without any other diagnostics being emitted (and without an expected lint + /// being suppressed), this will be emitted as a bug. Otherwise, it will be silently dropped. + /// I.e. "expect other diagnostics are emitted (or suppressed)" semantics. Useful on code paths + /// that should only be reached when emitting diagnostics, e.g. for expensive one-time + /// diagnostic formatting operations. + /// + /// Its `EmissionGuarantee` is `()`, because it does not guarantee an error will be emitted, + /// only a diagnostic of some kind (which could a warning, note, etc.) + /// + /// FIXME(nnethercote) weak delayed bugs are semantically strange: if printed they produce an + /// ICE, but they don't satisfy `is_error` and they don't guarantee an error is emitted. Plus + /// there's the extra complication with expected (suppressed) lints. They have limited use, and + /// are used in very few places. It would be good to remove them. + WeakDelayedBug, + /// A `force-warn` lint warning about the code being compiled. Does not prevent compilation /// from finishing. /// /// The [`LintExpectationId`] is used for expected lint diagnostics. In all other cases this /// should be `None`. - /// - /// Its `EmissionGuarantee` is `()`. ForceWarning(Option), /// A warning about the code being compiled. Does not prevent compilation from finishing. - /// - /// Its `EmissionGuarantee` is `()`. Warning, - /// A message giving additional context. Rare, because notes are more commonly attached to other - /// diagnostics such as errors. - /// - /// Its `EmissionGuarantee` is `()`. + /// A message giving additional context. Note, - /// A note that is only emitted once. Rare, mostly used in circumstances relating to lints. - /// - /// Its `EmissionGuarantee` is `()`. + /// A note that is only emitted once. OnceNote, - /// A message suggesting how to fix something. Rare, because help messages are more commonly - /// attached to other diagnostics such as errors. - /// - /// Its `EmissionGuarantee` is `()`. + /// A message suggesting how to fix something. Help, - /// A help that is only emitted once. Rare. - /// - /// Its `EmissionGuarantee` is `()`. + /// A help that is only emitted once. OnceHelp, /// Similar to `Note`, but used in cases where compilation has failed. When printed for human - /// consumption, it doesn't have any kind of `note:` label. Rare. - /// - /// Its `EmissionGuarantee` is `()`. + /// consumption, it doesn't have any kind of `note:` label. FailureNote, /// Only used for lints. - /// - /// Its `EmissionGuarantee` is `()`. Allow, /// Only used for lints. - /// - /// Its `EmissionGuarantee` is `()`. Expect(LintExpectationId), } @@ -1598,7 +1597,7 @@ impl Level { fn color(self) -> ColorSpec { let mut spec = ColorSpec::new(); match self { - Bug | DelayedBug(_) | Fatal | Error => { + Bug | Fatal | Error | DelayedBug | WeakDelayedBug => { spec.set_fg(Some(Color::Red)).set_intense(true); } ForceWarning(_) | Warning => { @@ -1618,7 +1617,7 @@ impl Level { pub fn to_str(self) -> &'static str { match self { - Bug | DelayedBug(_) => "error: internal compiler error", + Bug | DelayedBug | WeakDelayedBug => "error: internal compiler error", Fatal | Error => "error", ForceWarning(_) | Warning => "warning", Note | OnceNote => "note", @@ -1638,6 +1637,19 @@ impl Level { _ => None, } } + + // Can this level be used in a top-level diagnostic message and/or a + // subdiagnostic message? + fn can_be_top_or_sub(&self) -> (bool, bool) { + match self { + Bug | DelayedBug | Fatal | Error | WeakDelayedBug | ForceWarning(_) | FailureNote + | Allow | Expect(_) => (true, false), + + Warning | Note | Help => (true, true), + + OnceNote | OnceHelp => (false, true), + } + } } // FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite. diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index db01b5bd70719..14b6bb387c0f3 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -140,7 +140,7 @@ impl Drop for TypeErrCtxt<'_, '_> { self.infcx .tcx .sess - .good_path_delayed_bug("used a `TypeErrCtxt` without raising an error or lint"); + .weak_delayed_bug("used a `TypeErrCtxt` without raising an error or lint"); } } } diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index bac5068a69b86..901eaa1e83f8e 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -3065,9 +3065,9 @@ pub fn trimmed_def_paths(tcx: TyCtxt<'_>, (): ()) -> DefIdMap { // Trimming paths is expensive and not optimized, since we expect it to only be used for error // reporting. // - // For good paths causing this bug, the `rustc_middle::ty::print::with_no_trimmed_paths` + // For paths causing this bug, the `rustc_middle::ty::print::with_no_trimmed_paths` // wrapper can be used to suppress this query, in exchange for full paths being formatted. - tcx.sess.good_path_delayed_bug( + tcx.sess.weak_delayed_bug( "trimmed_def_paths constructed but no error emitted; use `DelayDm` for lints or `with_no_trimmed_paths` for debugging", ); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 3a0ae74dd9215..6b2d5991cde3c 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -333,9 +333,9 @@ impl Session { } /// Used for code paths of expensive computations that should only take place when - /// warnings or errors are emitted. If no messages are emitted ("good path"), then - /// it's likely a bug. - pub fn good_path_delayed_bug(&self, msg: impl Into) { + /// warnings or errors are emitted. If no messages are emitted, then it's + /// likely a bug. + pub fn weak_delayed_bug(&self, msg: impl Into) { if self.opts.unstable_opts.print_type_sizes || self.opts.unstable_opts.query_dep_graph || self.opts.unstable_opts.dump_mir.is_some() @@ -346,7 +346,7 @@ impl Session { return; } - self.dcx().good_path_delayed_bug(msg) + self.dcx().weak_delayed_bug(msg) } #[inline] @@ -555,9 +555,8 @@ impl Session { ret = fuel.remaining != 0; if fuel.remaining == 0 && !fuel.out_of_fuel { if self.dcx().can_emit_warnings() { - // We only call `msg` in case we can actually emit warnings. - // Otherwise, this could cause a `good_path_delayed_bug` to - // trigger (issue #79546). + // We only call `msg` in case we can actually emit warnings. Otherwise, + // this could cause a `weak_delayed_bug` to trigger (issue #79546). self.dcx().emit_warn(errors::OptimisationFuelExhausted { msg: msg() }); } fuel.out_of_fuel = true; diff --git a/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr b/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr index 1011fc4163bca..0e3cd2ff06099 100644 --- a/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr +++ b/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr @@ -1,4 +1,4 @@ -note: no errors encountered even though `span_delayed_bug` issued +note: no errors encountered even though delayed bugs were created note: those delayed bugs will now be shown as internal compiler errors diff --git a/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr b/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr index d92bafce142c2..fd76526644bdd 100644 --- a/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr +++ b/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr @@ -1,4 +1,4 @@ -note: no errors encountered even though `span_delayed_bug` issued +note: no errors encountered even though delayed bugs were created note: those delayed bugs will now be shown as internal compiler errors