diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index dd9dfe3fe7983..67be79596d3d5 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -288,7 +288,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 `must_produce_diag` 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) @@ -304,7 +304,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 `must_produce_diag` 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 e174cba7813f5..a1abe8fd4f306 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -376,7 +376,7 @@ impl From> for DiagnosticMessage { } } -/// A workaround for good_path_delayed_bug ICEs when formatting types in disabled lints. +/// A workaround for must_produce_diag 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 37f568f12a797..1a34a83c1a44a 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -85,11 +85,7 @@ 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::Fatal - | Level::Error - | Level::DelayedBug - | Level::GoodPathDelayedBug => AnnotationType::Error, + Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => 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 2deb18484ec2e..b14a12175c79b 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -237,8 +237,7 @@ impl Diagnostic { match self.level { Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => true, - Level::GoodPathDelayedBug - | Level::ForceWarning(_) + Level::ForceWarning(_) | Level::Warning | Level::Note | Level::OnceNote diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index ab3ad0e9d6843..58a3cabfb3cda 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -448,9 +448,13 @@ struct DiagCtxtInner { emitter: Box, delayed_bugs: Vec, - good_path_delayed_bugs: Vec, + + /// Must we produce a diagnostic to justify the use of the expensive + /// `trimmed_def_paths` function? + must_produce_diag: bool, + /// 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 `must_produce_diag` check. suppressed_expected_diag: bool, /// This set contains the code of all emitted diagnostics to avoid @@ -531,11 +535,6 @@ fn default_track_diagnostic(diag: Diagnostic, f: &mut dyn FnMut(Diagnostic)) { pub static TRACK_DIAGNOSTIC: AtomicRef = AtomicRef::new(&(default_track_diagnostic as _)); -enum DelayedBugKind { - Normal, - GoodPath, -} - #[derive(Copy, Clone, Default)] pub struct DiagCtxtFlags { /// If false, warning-level lints are suppressed. @@ -561,11 +560,16 @@ impl Drop for DiagCtxtInner { self.emit_stashed_diagnostics(); if !self.has_errors() { - self.flush_delayed(DelayedBugKind::Normal) + self.flush_delayed() } if !self.has_printed && !self.suppressed_expected_diag && !std::thread::panicking() { - self.flush_delayed(DelayedBugKind::GoodPath); + if self.must_produce_diag { + panic!( + "must_produce_diag: trimmed_def_paths called but no diagnostics emitted; \ + use `DelayDm` for lints or `with_no_trimmed_paths` for debugging" + ); + } } if self.check_unstable_expect_diagnostics { @@ -612,7 +616,7 @@ impl DiagCtxt { has_printed: false, emitter, delayed_bugs: Vec::new(), - good_path_delayed_bugs: Vec::new(), + must_produce_diag: false, suppressed_expected_diag: false, taught_diagnostics: Default::default(), emitted_diagnostic_codes: Default::default(), @@ -670,7 +674,7 @@ impl DiagCtxt { // actually free the underlying memory (which `clear` would not do) inner.delayed_bugs = Default::default(); - inner.good_path_delayed_bugs = Default::default(); + inner.must_produce_diag = false; inner.taught_diagnostics = Default::default(); inner.emitted_diagnostic_codes = Default::default(); inner.emitted_diagnostics = Default::default(); @@ -883,9 +887,10 @@ impl DiagCtxt { DiagnosticBuilder::::new(self, DelayedBug, msg).with_span(sp).emit() } - /// Ensures that a diagnostic is printed. See `Level::GoodPathDelayedBug`. - pub fn good_path_delayed_bug(&self, msg: impl Into) { - DiagnosticBuilder::<()>::new(self, GoodPathDelayedBug, msg).emit() + /// Used when trimmed_def_paths is called and we must produce a diagnostic + /// to justify its cost. + pub fn set_must_produce_diag(&self) { + self.inner.borrow_mut().must_produce_diag = true; } #[track_caller] @@ -1225,7 +1230,7 @@ impl DiagCtxt { } pub fn flush_delayed(&self) { - self.inner.borrow_mut().flush_delayed(DelayedBugKind::Normal); + self.inner.borrow_mut().flush_delayed(); } } @@ -1275,14 +1280,12 @@ impl DiagCtxtInner { if diagnostic.has_future_breakage() { // 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 good-path bug check. + // so they'll trigger the must_produce_diag check. self.suppressed_expected_diag = true; self.future_breakage_diagnostics.push(diagnostic.clone()); } - if matches!(diagnostic.level, DelayedBug | GoodPathDelayedBug) - && self.flags.eagerly_emit_delayed_bugs - { + if diagnostic.level == DelayedBug && self.flags.eagerly_emit_delayed_bugs { diagnostic.level = Error; } @@ -1302,12 +1305,6 @@ impl DiagCtxtInner { #[allow(deprecated)] return Some(ErrorGuaranteed::unchecked_error_guaranteed()); } - GoodPathDelayedBug => { - let backtrace = std::backtrace::Backtrace::capture(); - self.good_path_delayed_bugs - .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); - return None; - } Warning if !self.flags.can_emit_warnings => { if diagnostic.has_future_breakage() { (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); @@ -1409,19 +1406,8 @@ impl DiagCtxtInner { self.emit_diagnostic(Diagnostic::new(FailureNote, msg)); } - fn flush_delayed(&mut self, kind: DelayedBugKind) { - let (bugs, note1) = match kind { - DelayedBugKind::Normal => ( - 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 were created", - ), - }; - let note2 = "those delayed bugs will now be shown as internal compiler errors"; - + fn flush_delayed(&mut self) { + let bugs = std::mem::take(&mut self.delayed_bugs); if bugs.is_empty() { return; } @@ -1449,6 +1435,8 @@ impl DiagCtxtInner { // frame them better (e.g. separate warnings from them). Also, // make it a note so it doesn't count as an error, because that // could trigger `-Ztreat-err-as-bug`, which we don't want. + let note1 = "no errors encountered even though delayed bugs were created"; + let note2 = "those delayed bugs will now be shown as internal compiler errors"; self.emit_diagnostic(Diagnostic::new(Note, note1)); self.emit_diagnostic(Diagnostic::new(Note, note2)); } @@ -1457,7 +1445,7 @@ impl DiagCtxtInner { if backtrace || self.ice_file.is_none() { bug.decorate() } else { bug.inner }; // "Undelay" the delayed bugs (into plain `Bug`s). - if !matches!(bug.level, DelayedBug | GoodPathDelayedBug) { + if bug.level != DelayedBug { // NOTE(eddyb) not panicking here because we're already producing // an ICE, and the more information the merrier. bug.subdiagnostic(InvalidFlushedDelayedDiagnosticLevel { @@ -1529,7 +1517,6 @@ impl DelayedDiagnostic { /// Fatal yes FatalAbort/FatalError(*) yes - - /// Error yes ErrorGuaranteed yes - yes /// DelayedBug yes ErrorGuaranteed yes - - -/// GoodPathDelayedBug - () yes - - /// ForceWarning - () yes - lint-only /// Warning - () yes yes yes /// Note - () rare yes - @@ -1562,20 +1549,6 @@ pub enum Level { /// 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. - /// - /// FIXME(nnethercote) good path 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, and "good path" isn't a good name. It would be good - /// to remove them. - GoodPathDelayedBug, - /// A `force-warn` lint warning about the code being compiled. Does not prevent compilation /// from finishing. /// @@ -1619,7 +1592,7 @@ impl Level { fn color(self) -> ColorSpec { let mut spec = ColorSpec::new(); match self { - Bug | Fatal | Error | DelayedBug | GoodPathDelayedBug => { + Bug | Fatal | Error | DelayedBug => { spec.set_fg(Some(Color::Red)).set_intense(true); } ForceWarning(_) | Warning => { @@ -1639,7 +1612,7 @@ impl Level { pub fn to_str(self) -> &'static str { match self { - Bug | DelayedBug | GoodPathDelayedBug => "error: internal compiler error", + Bug | DelayedBug => "error: internal compiler error", Fatal | Error => "error", ForceWarning(_) | Warning => "warning", Note | OnceNote => "note", @@ -1664,8 +1637,8 @@ impl Level { // subdiagnostic message? fn can_be_top_or_sub(&self) -> (bool, bool) { match self { - Bug | DelayedBug | Fatal | Error | GoodPathDelayedBug | ForceWarning(_) - | FailureNote | Allow | Expect(_) => (true, false), + Bug | DelayedBug | Fatal | Error | ForceWarning(_) | FailureNote | Allow + | Expect(_) => (true, false), Warning | Note | Help => (true, true), diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs index 5cf90e94907b0..92ec1a83beeb9 100644 --- a/compiler/rustc_middle/src/ty/print/pretty.rs +++ b/compiler/rustc_middle/src/ty/print/pretty.rs @@ -3156,13 +3156,12 @@ fn for_each_def(tcx: TyCtxt<'_>, mut collect_fn: impl for<'b> FnMut(&'b Ident, N // this is pub to be able to intra-doc-link it 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. + // reporting. Record the fact that we did it, so we can abort if we later found it was + // unnecessary. // - // For good 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( - "trimmed_def_paths constructed but no error emitted; use `DelayDm` for lints or `with_no_trimmed_paths` for debugging", - ); + // The `rustc_middle::ty::print::with_no_trimmed_paths` wrapper can be used to suppress this + // checking, in exchange for full paths being formatted. + tcx.sess.record_trimmed_def_paths(); // Once constructed, unique namespace+symbol pairs will have a `Some(_)` entry, while // non-unique pairs will have a `None` entry. diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 0c084660761d8..25c20be8e627b 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -322,10 +322,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) { + /// Record the fact that we called `trimmed_def_paths`, and do some + /// checking about whether its cost was justified. + pub fn record_trimmed_def_paths(&self) { if self.opts.unstable_opts.print_type_sizes || self.opts.unstable_opts.query_dep_graph || self.opts.unstable_opts.dump_mir.is_some() @@ -336,7 +335,7 @@ impl Session { return; } - self.dcx().good_path_delayed_bug(msg) + self.dcx().set_must_produce_diag() } #[inline] @@ -546,8 +545,8 @@ impl Session { 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). + // Otherwise, this could cause a `must_produce_diag` ICE + // (issue #79546). self.dcx().emit_warn(errors::OptimisationFuelExhausted { msg: msg() }); } fuel.out_of_fuel = true; diff --git a/tests/ui/treat-err-as-bug/eagerly-emit.rs b/tests/ui/treat-err-as-bug/eagerly-emit.rs deleted file mode 100644 index ede190575d529..0000000000000 --- a/tests/ui/treat-err-as-bug/eagerly-emit.rs +++ /dev/null @@ -1,10 +0,0 @@ -// compile-flags: -Zeagerly-emit-delayed-bugs - -trait Foo {} - -fn main() {} - -fn f() -> impl Foo { - //~^ ERROR the trait bound `i32: Foo` is not satisfied - 1i32 -} diff --git a/tests/ui/treat-err-as-bug/eagerly-emit.stderr b/tests/ui/treat-err-as-bug/eagerly-emit.stderr deleted file mode 100644 index 4ae596435aa7f..0000000000000 --- a/tests/ui/treat-err-as-bug/eagerly-emit.stderr +++ /dev/null @@ -1,20 +0,0 @@ -error: trimmed_def_paths constructed but no error emitted; use `DelayDm` for lints or `with_no_trimmed_paths` for debugging - -error[E0277]: the trait bound `i32: Foo` is not satisfied - --> $DIR/eagerly-emit.rs:7:11 - | -LL | fn f() -> impl Foo { - | ^^^^^^^^ the trait `Foo` is not implemented for `i32` -LL | -LL | 1i32 - | ---- return type was inferred to be `i32` here - | -help: this trait has no implementations, consider adding one - --> $DIR/eagerly-emit.rs:3:1 - | -LL | trait Foo {} - | ^^^^^^^^^ - -error: aborting due to 2 previous errors - -For more information about this error, try `rustc --explain E0277`.