diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index f6fddbb509d..7679b7eb2dd 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -1709,7 +1709,7 @@ impl Emitter for SharedEmitter { drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { msg: diag.message(), code: diag.code.clone(), - lvl: diag.level, + lvl: diag.level(), }))); for child in &diag.children { drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 9db8f751390..2835afb0208 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -66,7 +66,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::Fatal | Level::Error { .. } => AnnotationType::Error, + Level::Bug | Level::DelayedBug | Level::Fatal | Level::Error { .. } => { + AnnotationType::Error + } Level::Warning => AnnotationType::Warning, Level::Note => AnnotationType::Note, Level::Help => AnnotationType::Help, diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 8cfecafd20c..f62a3567c56 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -19,7 +19,10 @@ pub struct SuggestionsDisabled; #[must_use] #[derive(Clone, Debug, Encodable, Decodable)] pub struct Diagnostic { - pub level: Level, + // NOTE(eddyb) this is private to disallow arbitrary after-the-fact changes, + // outside of what methods in this crate themselves allow. + crate level: Level, + pub message: Vec<(String, Style)>, pub code: Option, pub span: MultiSpan, @@ -117,9 +120,18 @@ impl Diagnostic { } } + #[inline(always)] + pub fn level(&self) -> Level { + self.level + } + pub fn is_error(&self) -> bool { match self.level { - Level::Bug | Level::Fatal | Level::Error { .. } | Level::FailureNote => true, + Level::Bug + | Level::DelayedBug + | Level::Fatal + | Level::Error { .. } + | Level::FailureNote => true, Level::Warning | Level::Note | Level::Help | Level::Cancelled | Level::Allow => false, } @@ -150,6 +162,33 @@ impl Diagnostic { self.level == Level::Cancelled } + /// Delay emission of this diagnostic as a bug. + /// + /// This can be useful in contexts where an error indicates a bug but + /// typically this only happens when other compilation errors have already + /// happened. In those cases this can be used to defer emission of this + /// diagnostic as a bug in the compiler only if no other errors have been + /// emitted. + /// + /// In the meantime, though, callsites are required to deal with the "bug" + /// locally in whichever way makes the most sense. + #[track_caller] + pub fn downgrade_to_delayed_bug(&mut self) -> &mut Self { + // FIXME(eddyb) this check is only necessary because cancellation exists, + // but hopefully that can be removed in the future, if enough callers + // of `.cancel()` can take `DiagnosticBuilder`, and by-value. + if !self.cancelled() { + assert!( + self.is_error(), + "downgrade_to_delayed_bug: cannot downgrade {:?} to DelayedBug: not an error", + self.level + ); + self.level = Level::DelayedBug; + } + + self + } + /// Adds a span/label to be included in the resulting snippet. /// /// This is pushed onto the [`MultiSpan`] that was created when the diagnostic diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index 3c8751a7a35..b4189cbfc62 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -105,10 +105,9 @@ impl<'a> DiagnosticBuilder<'a> { /// See `emit` and `delay_as_bug` for details. pub fn emit_unless(&mut self, delay: bool) { if delay { - self.delay_as_bug(); - } else { - self.emit(); + self.downgrade_to_delayed_bug(); } + self.emit(); } /// Stashes diagnostic for possible later improvement in a different, @@ -162,12 +161,17 @@ impl<'a> DiagnosticBuilder<'a> { /// /// In the meantime, though, callsites are required to deal with the "bug" /// locally in whichever way makes the most sense. + #[track_caller] pub fn delay_as_bug(&mut self) { - self.level = Level::Bug; - self.handler.delay_as_bug((*self.diagnostic).clone()); - self.cancel(); + self.downgrade_to_delayed_bug(); + self.emit(); } + forward!( + #[track_caller] + pub fn downgrade_to_delayed_bug(&mut self,) -> &mut Self + ); + /// Appends a labeled span to the diagnostic. /// /// Labels are used to convey additional context for the diagnostic's primary span. They will diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index a5c954cca13..539ddcec332 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -491,10 +491,15 @@ impl Drop for HandlerInner { self.flush_delayed(bugs, "no errors encountered even though `delay_span_bug` issued"); } + // FIXME(eddyb) this explains what `delayed_good_path_bugs` are! + // They're `delayed_span_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_any_message() { let bugs = std::mem::replace(&mut self.delayed_good_path_bugs, Vec::new()); self.flush_delayed( - bugs.into_iter().map(DelayedDiagnostic::decorate).collect(), + bugs.into_iter().map(DelayedDiagnostic::decorate), "no warnings or errors encountered even though `delayed_good_path_bugs` issued", ); } @@ -815,6 +820,8 @@ impl Handler { self.inner.borrow_mut().delay_span_bug(span, msg) } + // FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's + // where the explanation of what "good path" is (also, it should be renamed). pub fn delay_good_path_bug(&self, msg: &str) { self.inner.borrow_mut().delay_good_path_bug(msg) } @@ -915,10 +922,6 @@ impl Handler { pub fn emit_unused_externs(&self, lint_level: &str, unused_externs: &[&str]) { self.inner.borrow_mut().emit_unused_externs(lint_level, unused_externs) } - - pub fn delay_as_bug(&self, diagnostic: Diagnostic) { - self.inner.borrow_mut().delay_as_bug(diagnostic) - } } impl HandlerInner { @@ -936,11 +939,24 @@ impl HandlerInner { diags.iter().for_each(|diag| self.emit_diagnostic(diag)); } + // FIXME(eddyb) this should ideally take `diagnostic` by value. fn emit_diagnostic(&mut self, diagnostic: &Diagnostic) { if diagnostic.cancelled() { return; } + if diagnostic.level == Level::DelayedBug { + // FIXME(eddyb) this should check for `has_errors` and stop pushing + // once *any* errors were emitted (and truncate `delayed_span_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. + self.delayed_span_bugs.push(diagnostic.clone()); + + if !self.flags.report_delayed_bugs { + return; + } + } + if diagnostic.has_future_breakage() { self.future_breakage_diagnostics.push(diagnostic.clone()); } @@ -1119,14 +1135,16 @@ impl HandlerInner { // FIXME: don't abort here if report_delayed_bugs is off self.span_bug(sp, msg); } - let mut diagnostic = Diagnostic::new(Level::Bug, msg); + let mut diagnostic = Diagnostic::new(Level::DelayedBug, msg); diagnostic.set_span(sp.into()); diagnostic.note(&format!("delayed at {}", std::panic::Location::caller())); - self.delay_as_bug(diagnostic) + self.emit_diagnostic(&diagnostic) } + // FIXME(eddyb) note the comment inside `impl Drop for HandlerInner`, that's + // where the explanation of what "good path" is (also, it should be renamed). fn delay_good_path_bug(&mut self, msg: &str) { - let diagnostic = Diagnostic::new(Level::Bug, msg); + let diagnostic = Diagnostic::new(Level::DelayedBug, msg); if self.flags.report_delayed_bugs { self.emit_diagnostic(&diagnostic); } @@ -1160,20 +1178,34 @@ impl HandlerInner { panic::panic_any(ExplicitBug); } - fn delay_as_bug(&mut self, diagnostic: Diagnostic) { - if self.flags.report_delayed_bugs { - self.emit_diagnostic(&diagnostic); - } - self.delayed_span_bugs.push(diagnostic); - } + fn flush_delayed(&mut self, bugs: impl IntoIterator, explanation: &str) { + let mut no_bugs = true; + for mut bug in bugs { + if no_bugs { + // Put the overall explanation before the `DelayedBug`s, to + // frame them better (e.g. separate warnings from them). + self.emit_diagnostic(&Diagnostic::new(Bug, explanation)); + no_bugs = false; + } + + // "Undelay" the `DelayedBug`s (into plain `Bug`s). + if bug.level != Level::DelayedBug { + // NOTE(eddyb) not panicking here because we're already producing + // an ICE, and the more information the merrier. + bug.note(&format!( + "`flushed_delayed` got diagnostic with level {:?}, \ + instead of the expected `DelayedBug`", + bug.level, + )); + } + bug.level = Level::Bug; - fn flush_delayed(&mut self, bugs: Vec, explanation: &str) { - let has_bugs = !bugs.is_empty(); - for bug in bugs { self.emit_diagnostic(&bug); } - if has_bugs { - panic!("{}", explanation); + + // Panic with `ExplicitBug` to avoid "unexpected panic" messages. + if !no_bugs { + panic::panic_any(ExplicitBug); } } @@ -1227,6 +1259,7 @@ impl DelayedDiagnostic { #[derive(Copy, PartialEq, Clone, Hash, Debug, Encodable, Decodable)] pub enum Level { Bug, + DelayedBug, Fatal, Error { /// If this error comes from a lint, don't abort compilation even when abort_if_errors() is called. @@ -1250,7 +1283,7 @@ impl Level { fn color(self) -> ColorSpec { let mut spec = ColorSpec::new(); match self { - Bug | Fatal | Error { .. } => { + Bug | DelayedBug | Fatal | Error { .. } => { spec.set_fg(Some(Color::Red)).set_intense(true); } Warning => { @@ -1270,7 +1303,7 @@ impl Level { pub fn to_str(self) -> &'static str { match self { - Bug => "error: internal compiler error", + Bug | DelayedBug => "error: internal compiler error", Fatal | Error { .. } => "error", Warning => "warning", Note => "note", diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index f20cf29cc89..159afa1bbbe 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1066,7 +1066,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { }) .unwrap_or(false) { - err.delay_as_bug(); + err.downgrade_to_delayed_bug(); // We already suggested changing `:` into `::` during parsing. return false; } @@ -1472,7 +1472,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { .borrow_mut() .insert(colon_sp) { - err.delay_as_bug(); + err.downgrade_to_delayed_bug(); } } if let Ok(base_snippet) = base_snippet { diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 466f32aa800..aa18a2e4b9b 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -251,7 +251,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if !lhs.is_syntactic_place_expr() { // We already emitted E0070 "invalid left-hand side of assignment", so we // silence this. - err.delay_as_bug(); + err.downgrade_to_delayed_bug(); } } _ => {} diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index d3d42a1f37c..34e30fe49b1 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -837,7 +837,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { kind: hir::ExprKind::Loop(_, _, hir::LoopSource::While, _), .. })) => { - err.delay_as_bug(); + err.downgrade_to_delayed_bug(); } _ => {} } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 33a1530d588..bc2b99cb587 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -2098,8 +2098,15 @@ fn resolution_failure( ) } ResolutionFailure::NoParentItem => { - diag.level = rustc_errors::Level::Bug; - "all intra-doc links should have a parent item".to_owned() + // FIXME(eddyb) this doesn't belong here, whatever made + // the `ResolutionFailure::NoParentItem` should emit an + // immediate or delayed `span_bug` about the issue. + tcx.sess.delay_span_bug( + sp.unwrap_or(DUMMY_SP), + "intra-doc link missing parent item", + ); + + "BUG: all intra-doc links should have a parent item".to_owned() } ResolutionFailure::MalformedGenerics(variant) => match variant { MalformedGenerics::UnbalancedAngleBrackets => { diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index 7fc3778376c..d26bb8c2025 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -60,7 +60,7 @@ impl Emitter for SilentOnIgnoredFilesEmitter { None } fn emit_diagnostic(&mut self, db: &Diagnostic) { - if db.level == DiagnosticLevel::Fatal { + if db.level() == DiagnosticLevel::Fatal { return self.handle_non_ignoreable_error(db); } if let Some(primary_span) = &db.span.primary_span() { @@ -292,7 +292,7 @@ mod tests { use super::*; use crate::config::IgnoreList; use crate::utils::mk_sp; - use rustc_span::{FileName as SourceMapFileName, MultiSpan, RealFileName, DUMMY_SP}; + use rustc_span::{FileName as SourceMapFileName, MultiSpan, RealFileName}; use std::path::PathBuf; use std::sync::atomic::AtomicU32; @@ -310,16 +310,12 @@ mod tests { } fn build_diagnostic(level: DiagnosticLevel, span: Option) -> Diagnostic { - Diagnostic { - level, - code: None, - message: vec![], - children: vec![], - suggestions: Ok(vec![]), - span: span.unwrap_or_else(MultiSpan::new), - sort_span: DUMMY_SP, - is_lint: false, + let mut diag = Diagnostic::new(level, ""); + diag.message.clear(); + if let Some(span) = span { + diag.span = span; } + diag } fn build_emitter(