Rollup merge of #119763 - nnethercote:cleanup-Diagnostic, r=oli-obk

Cleanup things in and around `Diagnostic`

These changes all arose when I was looking closely at how to simplify `DiagCtxtInner::emit_diagnostic`.

r? `@compiler-errors`
This commit is contained in:
Matthias Krüger 2024-01-11 03:02:41 +01:00 committed by GitHub
commit 88493fca13
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 72 additions and 95 deletions

View File

@ -394,7 +394,7 @@ fn not_testable_error(cx: &ExtCtxt<'_>, attr_sp: Span, item: Option<&ast::Item>)
let level = match item.map(|i| &i.kind) {
// These were a warning before #92959 and need to continue being that to avoid breaking
// stable user code (#94508).
Some(ast::ItemKind::MacCall(_)) => Level::Warning(None),
Some(ast::ItemKind::MacCall(_)) => Level::Warning,
_ => Level::Error,
};
let mut err = DiagnosticBuilder::<()>::new(dcx, level, msg);

View File

@ -417,7 +417,7 @@ fn report_inline_asm(
}
let level = match level {
llvm::DiagnosticLevel::Error => Level::Error,
llvm::DiagnosticLevel::Warning => Level::Warning(None),
llvm::DiagnosticLevel::Warning => Level::Warning,
llvm::DiagnosticLevel::Note | llvm::DiagnosticLevel::Remark => Level::Note,
};
cgcx.diag_emitter.inline_asm_error(cookie as u32, msg, level, source);

View File

@ -1847,14 +1847,9 @@ impl SharedEmitterMain {
dcx.emit_diagnostic(d);
}
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
let err_level = match level {
Level::Error => Level::Error,
Level::Warning(_) => Level::Warning(None),
Level::Note => Level::Note,
_ => bug!("Invalid inline asm diagnostic level"),
};
assert!(matches!(level, Level::Error | Level::Warning | Level::Note));
let msg = msg.strip_prefix("error: ").unwrap_or(&msg).to_string();
let mut err = DiagnosticBuilder::<()>::new(sess.dcx(), err_level, msg);
let mut err = DiagnosticBuilder::<()>::new(sess.dcx(), level, msg);
// If the cookie is 0 then we don't have span information.
if cookie != 0 {

View File

@ -87,7 +87,7 @@ fn source_string(file: Lrc<SourceFile>, line: &Line) -> String {
fn annotation_type_for_level(level: Level) -> AnnotationType {
match level {
Level::Bug | Level::DelayedBug | Level::Fatal | Level::Error => AnnotationType::Error,
Level::Warning(_) => AnnotationType::Warning,
Level::ForceWarning(_) | Level::Warning => AnnotationType::Warning,
Level::Note | Level::OnceNote => AnnotationType::Note,
Level::Help | Level::OnceHelp => AnnotationType::Help,
// FIXME(#59346): Not sure how to map this level

View File

@ -152,7 +152,6 @@ pub enum DiagnosticId {
name: String,
/// Indicates whether this lint should show up in cargo's future breakage report.
has_future_breakage: bool,
is_force_warn: bool,
},
}
@ -248,7 +247,8 @@ impl Diagnostic {
true
}
Level::Warning(_)
Level::ForceWarning(_)
| Level::Warning
| Level::Note
| Level::OnceNote
| Level::Help
@ -262,7 +262,7 @@ impl Diagnostic {
&mut self,
unstable_to_stable: &FxIndexMap<LintExpectationId, LintExpectationId>,
) {
if let Level::Expect(expectation_id) | Level::Warning(Some(expectation_id)) =
if let Level::Expect(expectation_id) | Level::ForceWarning(Some(expectation_id)) =
&mut self.level
{
if expectation_id.is_stable() {
@ -292,8 +292,11 @@ impl Diagnostic {
}
pub(crate) fn is_force_warn(&self) -> bool {
match self.code {
Some(DiagnosticId::Lint { is_force_warn, .. }) => is_force_warn,
match self.level {
Level::ForceWarning(_) => {
assert!(self.is_lint);
true
}
_ => false,
}
}
@ -472,7 +475,7 @@ impl Diagnostic {
/// Add a warning attached to this diagnostic.
#[rustc_lint_diagnostics]
pub fn warn(&mut self, msg: impl Into<SubdiagnosticMessage>) -> &mut Self {
self.sub(Level::Warning(None), msg, MultiSpan::new());
self.sub(Level::Warning, msg, MultiSpan::new());
self
}
@ -484,7 +487,7 @@ impl Diagnostic {
sp: S,
msg: impl Into<SubdiagnosticMessage>,
) -> &mut Self {
self.sub(Level::Warning(None), msg, sp.into());
self.sub(Level::Warning, msg, sp.into());
self
}

View File

@ -198,7 +198,7 @@ impl Emitter for JsonEmitter {
.into_iter()
.map(|mut diag| {
if diag.level == crate::Level::Allow {
diag.level = crate::Level::Warning(None);
diag.level = crate::Level::Warning;
}
FutureBreakageItem {
diagnostic: EmitTyped::Diagnostic(Diagnostic::from_errors_diagnostic(

View File

@ -420,6 +420,7 @@ pub struct DiagCtxt {
/// as well as inconsistent state observation.
struct DiagCtxtInner {
flags: DiagCtxtFlags,
/// The number of lint errors that have been emitted.
lint_err_count: usize,
/// The number of errors that have been emitted, including duplicates.
@ -427,8 +428,13 @@ struct DiagCtxtInner {
/// This is not necessarily the count that's reported to the user once
/// compilation ends.
err_count: usize,
warn_count: usize,
deduplicated_err_count: usize,
/// The warning count, used for a recap upon finishing
deduplicated_warn_count: usize,
/// Has this diagnostic context printed any diagnostics? (I.e. has
/// `self.emitter.emit_diagnostic()` been called?
has_printed: bool,
emitter: Box<DynEmitter>,
span_delayed_bugs: Vec<DelayedDiagnostic>,
good_path_delayed_bugs: Vec<DelayedDiagnostic>,
@ -455,9 +461,6 @@ struct DiagCtxtInner {
/// When `.abort_if_errors()` is called, these are also emitted.
stashed_diagnostics: FxIndexMap<(Span, StashKey), Diagnostic>,
/// The warning count, used for a recap upon finishing
deduplicated_warn_count: usize,
future_breakage_diagnostics: Vec<Diagnostic>,
/// The [`Self::unstable_expect_diagnostics`] should be empty when this struct is
@ -513,7 +516,7 @@ fn default_track_diagnostic(diag: Diagnostic, f: &mut dyn FnMut(Diagnostic)) {
(*f)(diag)
}
pub static TRACK_DIAGNOSTICS: AtomicRef<fn(Diagnostic, &mut dyn FnMut(Diagnostic))> =
pub static TRACK_DIAGNOSTIC: AtomicRef<fn(Diagnostic, &mut dyn FnMut(Diagnostic))> =
AtomicRef::new(&(default_track_diagnostic as _));
#[derive(Copy, Clone, Default)]
@ -547,8 +550,7 @@ impl Drop for DiagCtxtInner {
// 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.
let has_any_message = self.err_count > 0 || self.lint_err_count > 0 || self.warn_count > 0;
if !has_any_message && !self.suppressed_expected_diag && !std::thread::panicking() {
if !self.has_printed && !self.suppressed_expected_diag && !std::thread::panicking() {
let bugs = std::mem::replace(&mut self.good_path_delayed_bugs, Vec::new());
self.flush_delayed(
bugs,
@ -594,9 +596,9 @@ impl DiagCtxt {
flags: DiagCtxtFlags { can_emit_warnings: true, ..Default::default() },
lint_err_count: 0,
err_count: 0,
warn_count: 0,
deduplicated_err_count: 0,
deduplicated_warn_count: 0,
has_printed: false,
emitter,
span_delayed_bugs: Vec::new(),
good_path_delayed_bugs: Vec::new(),
@ -647,10 +649,11 @@ impl DiagCtxt {
/// the overall count of emitted error diagnostics.
pub fn reset_err_count(&self) {
let mut inner = self.inner.borrow_mut();
inner.lint_err_count = 0;
inner.err_count = 0;
inner.warn_count = 0;
inner.deduplicated_err_count = 0;
inner.deduplicated_warn_count = 0;
inner.has_printed = false;
// actually free the underlying memory (which `clear` would not do)
inner.span_delayed_bugs = Default::default();
@ -669,16 +672,11 @@ impl DiagCtxt {
let key = (span.with_parent(None), key);
if diag.is_error() {
if diag.level == Error && diag.is_lint {
if diag.is_lint {
inner.lint_err_count += 1;
} else {
inner.err_count += 1;
}
} else {
// Warnings are only automatically flushed if they're forced.
if diag.is_force_warn() {
inner.warn_count += 1;
}
}
// FIXME(Centril, #69537): Consider reintroducing panic on overwriting a stashed diagnostic
@ -693,15 +691,11 @@ impl DiagCtxt {
let key = (span.with_parent(None), key);
let diag = inner.stashed_diagnostics.remove(&key)?;
if diag.is_error() {
if diag.level == Error && diag.is_lint {
if diag.is_lint {
inner.lint_err_count -= 1;
} else {
inner.err_count -= 1;
}
} else {
if diag.is_force_warn() {
inner.warn_count -= 1;
}
}
Some(DiagnosticBuilder::new_diagnostic(self, diag))
}
@ -738,7 +732,7 @@ impl DiagCtxt {
#[rustc_lint_diagnostics]
#[track_caller]
pub fn struct_warn(&self, msg: impl Into<DiagnosticMessage>) -> DiagnosticBuilder<'_, ()> {
DiagnosticBuilder::new(self, Warning(None), msg)
DiagnosticBuilder::new(self, Warning, msg)
}
/// Construct a builder at the `Allow` level with the `msg`.
@ -1005,7 +999,7 @@ impl DiagCtxt {
(0, 0) => return,
(0, _) => inner
.emitter
.emit_diagnostic(&Diagnostic::new(Warning(None), DiagnosticMessage::Str(warnings))),
.emit_diagnostic(&Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))),
(_, 0) => {
inner.emit_diagnostic(Diagnostic::new(Fatal, errors));
}
@ -1094,7 +1088,7 @@ impl DiagCtxt {
&'a self,
warning: impl IntoDiagnostic<'a, ()>,
) -> DiagnosticBuilder<'a, ()> {
warning.into_diagnostic(self, Warning(None))
warning.into_diagnostic(self, Warning)
}
#[track_caller]
@ -1241,21 +1235,17 @@ impl DiagCtxtInner {
for diag in diags {
// Decrement the count tracking the stash; emitting will increment it.
if diag.is_error() {
if diag.level == Error && diag.is_lint {
if diag.is_lint {
self.lint_err_count -= 1;
} else {
self.err_count -= 1;
}
} else {
if diag.is_force_warn() {
self.warn_count -= 1;
} else {
// Unless they're forced, don't flush stashed warnings when
// there are errors, to avoid causing warning overload. The
// stash would've been stolen already if it were important.
if has_errors {
continue;
}
// Unless they're forced, don't flush stashed warnings when
// there are errors, to avoid causing warning overload. The
// stash would've been stolen already if it were important.
if !diag.is_force_warn() && has_errors {
continue;
}
}
let reported_this = self.emit_diagnostic(diag);
@ -1304,23 +1294,20 @@ impl DiagCtxtInner {
self.fulfilled_expectations.insert(expectation_id.normalize());
}
if matches!(diagnostic.level, Warning(_))
&& !self.flags.can_emit_warnings
&& !diagnostic.is_force_warn()
{
if diagnostic.level == Warning && !self.flags.can_emit_warnings {
if diagnostic.has_future_breakage() {
(*TRACK_DIAGNOSTICS)(diagnostic, &mut |_| {});
(*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {});
}
return None;
}
if matches!(diagnostic.level, Expect(_) | Allow) {
(*TRACK_DIAGNOSTICS)(diagnostic, &mut |_| {});
(*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {});
return None;
}
let mut guaranteed = None;
(*TRACK_DIAGNOSTICS)(diagnostic, &mut |mut diagnostic| {
(*TRACK_DIAGNOSTIC)(diagnostic, &mut |mut diagnostic| {
if let Some(ref code) = diagnostic.code {
self.emitted_diagnostic_codes.insert(code.clone());
}
@ -1359,12 +1346,13 @@ impl DiagCtxtInner {
self.emitter.emit_diagnostic(&diagnostic);
if diagnostic.is_error() {
self.deduplicated_err_count += 1;
} else if let Warning(_) = diagnostic.level {
} else if matches!(diagnostic.level, ForceWarning(_) | Warning) {
self.deduplicated_warn_count += 1;
}
self.has_printed = true;
}
if diagnostic.is_error() {
if diagnostic.level == Error && diagnostic.is_lint {
if diagnostic.is_lint {
self.bump_lint_err_count();
} else {
self.bump_err_count();
@ -1374,8 +1362,6 @@ impl DiagCtxtInner {
{
guaranteed = Some(ErrorGuaranteed::unchecked_claim_error_was_emitted());
}
} else {
self.bump_warn_count();
}
});
@ -1471,10 +1457,6 @@ impl DiagCtxtInner {
self.panic_if_treat_err_as_bug();
}
fn bump_warn_count(&mut self) {
self.warn_count += 1;
}
fn panic_if_treat_err_as_bug(&self) {
if self.treat_err_as_bug() {
match (
@ -1562,14 +1544,17 @@ pub enum Level {
/// Its `EmissionGuarantee` is `ErrorGuaranteed`.
Error,
/// 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`.
ForceWarning(Option<LintExpectationId>),
/// A warning about the code being compiled. Does not prevent compilation from finishing.
///
/// This [`LintExpectationId`] is used for expected lint diagnostics, which should
/// also emit a warning due to the `force-warn` flag. In all other cases this should
/// be `None`.
///
/// Its `EmissionGuarantee` is `()`.
Warning(Option<LintExpectationId>),
Warning,
/// A message giving additional context. Rare, because notes are more commonly attached to other
/// diagnostics such as errors.
@ -1622,7 +1607,7 @@ impl Level {
Bug | DelayedBug | Fatal | Error => {
spec.set_fg(Some(Color::Red)).set_intense(true);
}
Warning(_) => {
ForceWarning(_) | Warning => {
spec.set_fg(Some(Color::Yellow)).set_intense(cfg!(windows));
}
Note | OnceNote => {
@ -1641,7 +1626,7 @@ impl Level {
match self {
Bug | DelayedBug => "error: internal compiler error",
Fatal | Error => "error",
Warning(_) => "warning",
ForceWarning(_) | Warning => "warning",
Note | OnceNote => "note",
Help | OnceHelp => "help",
FailureNote => "failure-note",
@ -1655,7 +1640,7 @@ impl Level {
pub fn get_expectation_id(&self) -> Option<LintExpectationId> {
match self {
Expect(id) | Warning(Some(id)) => Some(*id),
Expect(id) | ForceWarning(Some(id)) => Some(*id),
_ => None,
}
}

View File

@ -380,7 +380,7 @@ impl ToInternal<rustc_errors::Level> for Level {
fn to_internal(self) -> rustc_errors::Level {
match self {
Level::Error => rustc_errors::Level::Error,
Level::Warning => rustc_errors::Level::Warning(None),
Level::Warning => rustc_errors::Level::Warning,
Level::Note => rustc_errors::Level::Note,
Level::Help => rustc_errors::Level::Help,
_ => unreachable!("unknown proc_macro::Level variant: {:?}", self),

View File

@ -9,7 +9,7 @@
//! The functions in this file should fall back to the default set in their
//! origin crate when the `TyCtxt` is not present in TLS.
use rustc_errors::{Diagnostic, TRACK_DIAGNOSTICS};
use rustc_errors::{Diagnostic, TRACK_DIAGNOSTIC};
use rustc_middle::dep_graph::{DepNodeExt, TaskDepsRef};
use rustc_middle::ty::tls;
use rustc_query_system::dep_graph::dep_node::default_dep_kind_debug;
@ -103,5 +103,5 @@ pub fn setup_callbacks() {
.swap(&(dep_kind_debug as fn(_, &mut fmt::Formatter<'_>) -> _));
rustc_query_system::dep_graph::dep_node::DEP_NODE_DEBUG
.swap(&(dep_node_debug as fn(_, &mut fmt::Formatter<'_>) -> _));
TRACK_DIAGNOSTICS.swap(&(track_diagnostic as _));
TRACK_DIAGNOSTIC.swap(&(track_diagnostic as _));
}

View File

@ -312,8 +312,9 @@ pub fn struct_lint_level(
// create a `DiagnosticBuilder` and continue as we would for warnings.
rustc_errors::Level::Expect(expect_id)
}
Level::ForceWarn(Some(expect_id)) => rustc_errors::Level::Warning(Some(expect_id)),
Level::Warn | Level::ForceWarn(None) => rustc_errors::Level::Warning(None),
Level::ForceWarn(Some(expect_id)) => rustc_errors::Level::ForceWarning(Some(expect_id)),
Level::ForceWarn(None) => rustc_errors::Level::ForceWarning(None),
Level::Warn => rustc_errors::Level::Warning,
Level::Deny | Level::Forbid => rustc_errors::Level::Error,
};
let mut err = DiagnosticBuilder::new(sess.dcx(), err_level, "");
@ -350,22 +351,19 @@ pub fn struct_lint_level(
// suppressed the lint due to macros.
err.primary_message(msg);
let name = lint.name_lower();
err.code(DiagnosticId::Lint { name, has_future_breakage });
// Lint diagnostics that are covered by the expect level will not be emitted outside
// the compiler. It is therefore not necessary to add any information for the user.
// This will therefore directly call the decorate function which will in turn emit
// the `Diagnostic`.
if let Level::Expect(_) = level {
let name = lint.name_lower();
err.code(DiagnosticId::Lint { name, has_future_breakage, is_force_warn: false });
decorate(&mut err);
err.emit();
return;
}
let name = lint.name_lower();
let is_force_warn = matches!(level, Level::ForceWarn(_));
err.code(DiagnosticId::Lint { name, has_future_breakage, is_force_warn });
if let Some(future_incompatible) = future_incompatible {
let explanation = match future_incompatible.reason {
FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps

View File

@ -143,11 +143,7 @@ pub fn feature_warn_issue(
// Decorate this as a future-incompatibility lint as in rustc_middle::lint::struct_lint_level
let lint = UNSTABLE_SYNTAX_PRE_EXPANSION;
let future_incompatible = lint.future_incompatible.as_ref().unwrap();
err.code(DiagnosticId::Lint {
name: lint.name_lower(),
has_future_breakage: false,
is_force_warn: false,
});
err.code(DiagnosticId::Lint { name: lint.name_lower(), has_future_breakage: false });
err.warn(lint.desc);
err.note(format!("for more information, see {}", future_incompatible.reference));

View File

@ -455,7 +455,7 @@ pub fn report_msg<'tcx>(
let sess = machine.tcx.sess;
let level = match diag_level {
DiagLevel::Error => Level::Error,
DiagLevel::Warning => Level::Warning(None),
DiagLevel::Warning => Level::Warning,
DiagLevel::Note => Level::Note,
};
let mut err = DiagnosticBuilder::<()>::new(sess.dcx(), level, title);

View File

@ -446,7 +446,7 @@ mod tests {
Some(ignore_list),
);
let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning(None), Some(span));
let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span));
emitter.emit_diagnostic(&non_fatal_diagnostic);
assert_eq!(num_emitted_errors.load(Ordering::Acquire), 0);
assert_eq!(can_reset_errors.load(Ordering::Acquire), true);
@ -470,7 +470,7 @@ mod tests {
None,
);
let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning(None), Some(span));
let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span));
emitter.emit_diagnostic(&non_fatal_diagnostic);
assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1);
assert_eq!(can_reset_errors.load(Ordering::Acquire), false);
@ -507,8 +507,8 @@ mod tests {
);
let bar_span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
let foo_span = MultiSpan::from_span(mk_sp(BytePos(21), BytePos(22)));
let bar_diagnostic = build_diagnostic(DiagnosticLevel::Warning(None), Some(bar_span));
let foo_diagnostic = build_diagnostic(DiagnosticLevel::Warning(None), Some(foo_span));
let bar_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(bar_span));
let foo_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(foo_span));
let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, None);
emitter.emit_diagnostic(&bar_diagnostic);
emitter.emit_diagnostic(&foo_diagnostic);