From 0e388f21928219472d34a121cc33fcc4cf3e2c77 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 9 Jan 2024 12:28:45 +1100 Subject: [PATCH] Change how `force-warn` lint diagnostics are recorded. `is_force_warn` is only possible for diagnostics with `Level::Warning`, but it is currently stored in `Diagnostic::code`, which every diagnostic has. This commit: - removes the boolean `DiagnosticId::Lint::is_force_warn` field; - adds a `ForceWarning` variant to `Level`. Benefits: - The common `Level::Warning` case now has no arguments, replacing lots of `Warning(None)` occurrences. - `rustc_session::lint::Level` and `rustc_errors::Level` are more similar, both having `ForceWarning` and `Warning`. --- compiler/rustc_builtin_macros/src/test.rs | 2 +- compiler/rustc_codegen_llvm/src/back/write.rs | 2 +- compiler/rustc_codegen_ssa/src/back/write.rs | 9 ++---- .../src/annotate_snippet_emitter_writer.rs | 2 +- compiler/rustc_errors/src/diagnostic.rs | 17 ++++++---- compiler/rustc_errors/src/json.rs | 2 +- compiler/rustc_errors/src/lib.rs | 32 +++++++++---------- .../rustc_expand/src/proc_macro_server.rs | 2 +- compiler/rustc_middle/src/lint.rs | 14 ++++---- compiler/rustc_session/src/parse.rs | 6 +--- src/tools/miri/src/diagnostics.rs | 2 +- src/tools/rustfmt/src/parse/session.rs | 8 ++--- 12 files changed, 45 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/test.rs b/compiler/rustc_builtin_macros/src/test.rs index 298bdc557ab..4d44e340ae1 100644 --- a/compiler/rustc_builtin_macros/src/test.rs +++ b/compiler/rustc_builtin_macros/src/test.rs @@ -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); diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index deffb10c759..a912ef9e755 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -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); diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 6c066e61a58..8e835039970 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -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 { diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 5c0e210f147..97f2efa7874 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -87,7 +87,7 @@ fn source_string(file: Lrc, 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 diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 701c1c02ab0..d8d6922a1bc 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -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, ) { - 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) -> &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, ) -> &mut Self { - self.sub(Level::Warning(None), msg, sp.into()); + self.sub(Level::Warning, msg, sp.into()); self } diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 52fcb50e9fb..87bf9c23456 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -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( diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index d27fe8779f4..f64a5d65fcf 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -738,7 +738,7 @@ impl DiagCtxt { #[rustc_lint_diagnostics] #[track_caller] pub fn struct_warn(&self, msg: impl Into) -> DiagnosticBuilder<'_, ()> { - DiagnosticBuilder::new(self, Warning(None), msg) + DiagnosticBuilder::new(self, Warning, msg) } /// Construct a builder at the `Allow` level with the `msg`. @@ -1005,7 +1005,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 +1094,7 @@ impl DiagCtxt { &'a self, warning: impl IntoDiagnostic<'a, ()>, ) -> DiagnosticBuilder<'a, ()> { - warning.into_diagnostic(self, Warning(None)) + warning.into_diagnostic(self, Warning) } #[track_caller] @@ -1304,10 +1304,7 @@ 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_DIAGNOSTIC)(diagnostic, &mut |_| {}); } @@ -1359,7 +1356,7 @@ 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; } } @@ -1562,14 +1559,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), + /// 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), + Warning, /// A message giving additional context. Rare, because notes are more commonly attached to other /// diagnostics such as errors. @@ -1622,7 +1622,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 +1641,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 +1655,7 @@ impl Level { pub fn get_expectation_id(&self) -> Option { match self { - Expect(id) | Warning(Some(id)) => Some(*id), + Expect(id) | ForceWarning(Some(id)) => Some(*id), _ => None, } } diff --git a/compiler/rustc_expand/src/proc_macro_server.rs b/compiler/rustc_expand/src/proc_macro_server.rs index 0053f550318..6392894fea2 100644 --- a/compiler/rustc_expand/src/proc_macro_server.rs +++ b/compiler/rustc_expand/src/proc_macro_server.rs @@ -380,7 +380,7 @@ impl ToInternal 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), diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index 212e831e106..c5e4dfaf19e 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -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 diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 439fa18b7fa..598178c3c2a 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -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)); diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 819d2018a15..bf3284df596 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -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); diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index 2663f16b8e8..f4fb5073dfd 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -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);