Rollup merge of #120575 - nnethercote:simplify-codegen-diag-handling, r=estebank

Simplify codegen diagnostic handling

Some nice improvements. Details in the individual commit logs.

r? `@estebank`

Author: matthiaskrgr

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -15,10 +15,7 @@ use rustc_data_structures::profiling::{SelfProfilerRef, VerboseTimingGuard};
 use rustc_data_structures::sync::Lrc;
 use rustc_errors::emitter::Emitter;
 use rustc_errors::translation::Translate;
-use rustc_errors::{
-    DiagCtxt, DiagnosticArgName, DiagnosticArgValue, DiagnosticBuilder, DiagnosticMessage, ErrCode,
-    FatalError, FluentBundle, Level, Style,
-};
+use rustc_errors::{DiagCtxt, Diagnostic, DiagnosticBuilder, FatalError, FluentBundle, Level};
 use rustc_fs_util::link_or_copy;
 use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
 use rustc_incremental::{
@@ -39,7 +36,6 @@ use rustc_target::spec::{MergeFunctions, SanitizerSet};
 
 use crate::errors::ErrorCreatingRemarkDir;
 use std::any::Any;
-use std::borrow::Cow;
 use std::fs;
 use std::io;
 use std::marker::PhantomData;
@@ -998,13 +994,6 @@ pub(crate) enum Message<B: WriteBackendMethods> {
 /// process another codegen unit.
 pub struct CguMessage;
 
-struct Diagnostic {
-    msgs: Vec<(DiagnosticMessage, Style)>,
-    args: FxHashMap<DiagnosticArgName, DiagnosticArgValue>,
-    code: Option<ErrCode>,
-    lvl: Level,
-}
-
 #[derive(PartialEq, Clone, Copy, Debug)]
 enum MainThreadState {
     /// Doing nothing.
@@ -1765,7 +1754,6 @@ fn spawn_work<'a, B: ExtraBackendMethods>(
 enum SharedEmitterMessage {
     Diagnostic(Diagnostic),
     InlineAsmError(u32, String, Level, Option<(String, Vec<InnerSpan>)>),
-    AbortIfErrors,
     Fatal(String),
 }
 
@@ -1811,24 +1799,8 @@ impl Translate for SharedEmitter {
 }
 
 impl Emitter for SharedEmitter {
-    fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) {
-        let args: FxHashMap<Cow<'_, str>, DiagnosticArgValue> =
-            diag.args().map(|(name, arg)| (name.clone(), arg.clone())).collect();
-        drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic {
-            msgs: diag.messages.clone(),
-            args: args.clone(),
-            code: diag.code.clone(),
-            lvl: diag.level(),
-        })));
-        for child in &diag.children {
-            drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic {
-                msgs: child.messages.clone(),
-                args: args.clone(),
-                code: None,
-                lvl: child.level,
-            })));
-        }
-        drop(self.sender.send(SharedEmitterMessage::AbortIfErrors));
+    fn emit_diagnostic(&mut self, diag: rustc_errors::Diagnostic) {
+        drop(self.sender.send(SharedEmitterMessage::Diagnostic(diag)));
     }
 
     fn source_map(&self) -> Option<&Lrc<SourceMap>> {
@@ -1853,13 +1825,8 @@ impl SharedEmitterMain {
 
             match message {
                 Ok(SharedEmitterMessage::Diagnostic(diag)) => {
-                    let dcx = sess.dcx();
-                    let mut d = rustc_errors::Diagnostic::new_with_messages(diag.lvl, diag.msgs);
-                    if let Some(code) = diag.code {
-                        d.code(code);
-                    }
-                    d.replace_args(diag.args);
-                    dcx.emit_diagnostic(d);
+                    sess.dcx().emit_diagnostic(diag);
+                    sess.dcx().abort_if_errors();
                 }
                 Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
                     assert!(matches!(level, Level::Error | Level::Warning | Level::Note));
@@ -1892,9 +1859,6 @@ impl SharedEmitterMain {
 
                     err.emit();
                 }
-                Ok(SharedEmitterMessage::AbortIfErrors) => {
-                    sess.dcx().abort_if_errors();
-                }
                 Ok(SharedEmitterMessage::Fatal(msg)) => {
                     sess.dcx().fatal(msg);
                 }

compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs

@@ -44,15 +44,15 @@ impl Translate for AnnotateSnippetEmitter {
 
 impl Emitter for AnnotateSnippetEmitter {
     /// The entry point for the diagnostics generation
-    fn emit_diagnostic(&mut self, diag: &Diagnostic) {
+    fn emit_diagnostic(&mut self, mut diag: Diagnostic) {
         let fluent_args = to_fluent_args(diag.args());
 
-        let mut children = diag.children.clone();
-        let (mut primary_span, suggestions) = self.primary_span_formatted(diag, &fluent_args);
+        let mut suggestions = diag.suggestions.unwrap_or(vec![]);
+        self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args);
 
         self.fix_multispans_in_extern_macros_and_render_macro_backtrace(
-            &mut primary_span,
-            &mut children,
+            &mut diag.span,
+            &mut diag.children,
             &diag.level,
             self.macro_backtrace,
         );
@@ -62,9 +62,9 @@ impl Emitter for AnnotateSnippetEmitter {
             &diag.messages,
             &fluent_args,
             &diag.code,
-            &primary_span,
-            &children,
-            suggestions,
+            &diag.span,
+            &diag.children,
+            &suggestions,
         );
     }
 

compiler/rustc_errors/src/emitter.rs

@@ -193,7 +193,7 @@ pub type DynEmitter = dyn Emitter + DynSend;
 /// Emitter trait for emitting errors.
 pub trait Emitter: Translate {
     /// Emit a structured diagnostic.
-    fn emit_diagnostic(&mut self, diag: &Diagnostic);
+    fn emit_diagnostic(&mut self, diag: Diagnostic);
 
     /// Emit a notification that an artifact has been output.
     /// Currently only supported for the JSON format.
@@ -230,17 +230,17 @@ pub trait Emitter: Translate {
     ///
     /// * If the current `Diagnostic` has only one visible `CodeSuggestion`,
     ///   we format the `help` suggestion depending on the content of the
-    ///   substitutions. In that case, we return the modified span only.
+    ///   substitutions. In that case, we modify the span and clear the
+    ///   suggestions.
     ///
     /// * If the current `Diagnostic` has multiple suggestions,
-    ///   we return the original `primary_span` and the original suggestions.
-    fn primary_span_formatted<'a>(
+    ///   we leave `primary_span` and the suggestions untouched.
+    fn primary_span_formatted(
         &mut self,
-        diag: &'a Diagnostic,
+        primary_span: &mut MultiSpan,
+        suggestions: &mut Vec<CodeSuggestion>,
         fluent_args: &FluentArgs<'_>,
-    ) -> (MultiSpan, &'a [CodeSuggestion]) {
-        let mut primary_span = diag.span.clone();
-        let suggestions = diag.suggestions.as_deref().unwrap_or(&[]);
+    ) {
         if let Some((sugg, rest)) = suggestions.split_first() {
             let msg = self.translate_message(&sugg.msg, fluent_args).map_err(Report::new).unwrap();
             if rest.is_empty() &&
@@ -287,16 +287,15 @@ pub trait Emitter: Translate {
                 primary_span.push_span_label(sugg.substitutions[0].parts[0].span, msg);
 
                 // We return only the modified primary_span
-                (primary_span, &[])
+                suggestions.clear();
             } else {
                 // if there are multiple suggestions, print them all in full
                 // to be consistent. We could try to figure out if we can
                 // make one (or the first one) inline, but that would give
                 // undue importance to a semi-random suggestion
-                (primary_span, suggestions)
             }
         } else {
-            (primary_span, suggestions)
+            // do nothing
         }
     }
 
@@ -518,16 +517,15 @@ impl Emitter for HumanEmitter {
         self.sm.as_ref()
     }
 
-    fn emit_diagnostic(&mut self, diag: &Diagnostic) {
+    fn emit_diagnostic(&mut self, mut diag: Diagnostic) {
         let fluent_args = to_fluent_args(diag.args());
 
-        let mut children = diag.children.clone();
-        let (mut primary_span, suggestions) = self.primary_span_formatted(diag, &fluent_args);
-        debug!("emit_diagnostic: suggestions={:?}", suggestions);
+        let mut suggestions = diag.suggestions.unwrap_or(vec![]);
+        self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args);
 
         self.fix_multispans_in_extern_macros_and_render_macro_backtrace(
-            &mut primary_span,
-            &mut children,
+            &mut diag.span,
+            &mut diag.children,
             &diag.level,
             self.macro_backtrace,
         );
@@ -537,9 +535,9 @@ impl Emitter for HumanEmitter {
             &diag.messages,
             &fluent_args,
             &diag.code,
-            &primary_span,
-            &children,
-            suggestions,
+            &diag.span,
+            &diag.children,
+            &suggestions,
             self.track_diagnostics.then_some(&diag.emitted_at),
         );
     }
@@ -576,9 +574,8 @@ impl Emitter for SilentEmitter {
         None
     }
 
-    fn emit_diagnostic(&mut self, d: &Diagnostic) {
+    fn emit_diagnostic(&mut self, mut d: Diagnostic) {
         if d.level == Level::Fatal {
-            let mut d = d.clone();
             if let Some(ref note) = self.fatal_note {
                 d.note(note.clone());
             }

compiler/rustc_errors/src/json.rs

@@ -176,7 +176,7 @@ impl Translate for JsonEmitter {
 }
 
 impl Emitter for JsonEmitter {
-    fn emit_diagnostic(&mut self, diag: &crate::Diagnostic) {
+    fn emit_diagnostic(&mut self, diag: crate::Diagnostic) {
         let data = Diagnostic::from_errors_diagnostic(diag, self);
         let result = self.emit(EmitTyped::Diagnostic(data));
         if let Err(e) = result {
@@ -201,7 +201,7 @@ impl Emitter for JsonEmitter {
                 }
                 FutureBreakageItem {
                     diagnostic: EmitTyped::Diagnostic(Diagnostic::from_errors_diagnostic(
-                        &diag, self,
+                        diag, self,
                     )),
                 }
             })
@@ -340,7 +340,7 @@ struct UnusedExterns<'a, 'b, 'c> {
 }
 
 impl Diagnostic {
-    fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic {
+    fn from_errors_diagnostic(diag: crate::Diagnostic, je: &JsonEmitter) -> Diagnostic {
         let args = to_fluent_args(diag.args());
         let sugg = diag.suggestions.iter().flatten().map(|sugg| {
             let translated_message =
@@ -382,6 +382,28 @@ impl Diagnostic {
                 Ok(())
             }
         }
+
+        let translated_message = je.translate_messages(&diag.messages, &args);
+
+        let code = if let Some(code) = diag.code {
+            Some(DiagnosticCode {
+                code: code.to_string(),
+                explanation: je.registry.as_ref().unwrap().try_find_description(code).ok(),
+            })
+        } else if let Some(IsLint { name, .. }) = &diag.is_lint {
+            Some(DiagnosticCode { code: name.to_string(), explanation: None })
+        } else {
+            None
+        };
+        let level = diag.level.to_str();
+        let spans = DiagnosticSpan::from_multispan(&diag.span, &args, je);
+        let children = diag
+            .children
+            .iter()
+            .map(|c| Diagnostic::from_sub_diagnostic(c, &args, je))
+            .chain(sugg)
+            .collect();
+
         let buf = BufWriter::default();
         let output = buf.clone();
         je.json_rendered
@@ -398,30 +420,12 @@ impl Diagnostic {
         let output = Arc::try_unwrap(output.0).unwrap().into_inner().unwrap();
         let output = String::from_utf8(output).unwrap();
 
-        let translated_message = je.translate_messages(&diag.messages, &args);
-
-        let code = if let Some(code) = diag.code {
-            Some(DiagnosticCode {
-                code: code.to_string(),
-                explanation: je.registry.as_ref().unwrap().try_find_description(code).ok(),
-            })
-        } else if let Some(IsLint { name, .. }) = &diag.is_lint {
-            Some(DiagnosticCode { code: name.to_string(), explanation: None })
-        } else {
-            None
-        };
-
         Diagnostic {
             message: translated_message.to_string(),
             code,
-            level: diag.level.to_str(),
-            spans: DiagnosticSpan::from_multispan(&diag.span, &args, je),
-            children: diag
-                .children
-                .iter()
-                .map(|c| Diagnostic::from_sub_diagnostic(c, &args, je))
-                .chain(sugg)
-                .collect(),
+            level,
+            spans,
+            children,
             rendered: Some(output),
         }
     }

compiler/rustc_errors/src/lib.rs

@@ -991,9 +991,13 @@ impl DiagCtxt {
 
         match (errors.len(), warnings.len()) {
             (0, 0) => return,
-            (0, _) => inner
-                .emitter
-                .emit_diagnostic(&Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))),
+            (0, _) => {
+                // Use `inner.emitter` directly, otherwise the warning might not be emitted, e.g.
+                // with a configuration like `--cap-lints allow --force-warn bare_trait_objects`.
+                inner
+                    .emitter
+                    .emit_diagnostic(Diagnostic::new(Warning, DiagnosticMessage::Str(warnings)));
+            }
             (_, 0) => {
                 inner.emit_diagnostic(Diagnostic::new(Fatal, errors));
             }
@@ -1061,7 +1065,7 @@ impl DiagCtxt {
     }
 
     pub fn force_print_diagnostic(&self, db: Diagnostic) {
-        self.inner.borrow_mut().emitter.emit_diagnostic(&db);
+        self.inner.borrow_mut().emitter.emit_diagnostic(db);
     }
 
     pub fn emit_diagnostic(&self, diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
@@ -1328,11 +1332,15 @@ impl DiagCtxtInner {
                 !self.emitted_diagnostics.insert(diagnostic_hash)
             };
 
+            let is_error = diagnostic.is_error();
+            let is_lint = diagnostic.is_lint.is_some();
+
             // Only emit the diagnostic if we've been asked to deduplicate or
             // haven't already emitted an equivalent diagnostic.
             if !(self.flags.deduplicate_diagnostics && already_emitted) {
                 debug!(?diagnostic);
                 debug!(?self.emitted_diagnostics);
+
                 let already_emitted_sub = |sub: &mut SubDiagnostic| {
                     debug!(?sub);
                     if sub.level != OnceNote && sub.level != OnceHelp {
@@ -1344,24 +1352,24 @@ impl DiagCtxtInner {
                     debug!(?diagnostic_hash);
                     !self.emitted_diagnostics.insert(diagnostic_hash)
                 };
-
                 diagnostic.children.extract_if(already_emitted_sub).for_each(|_| {});
                 if already_emitted {
                     diagnostic.note(
                         "duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`",
                     );
                 }
 
-                self.emitter.emit_diagnostic(&diagnostic);
-                if diagnostic.is_error() {
+                if is_error {
                     self.deduplicated_err_count += 1;
                 } else if matches!(diagnostic.level, ForceWarning(_) | Warning) {
                     self.deduplicated_warn_count += 1;
                 }
                 self.has_printed = true;
+
+                self.emitter.emit_diagnostic(diagnostic);
             }
-            if diagnostic.is_error() {
-                if diagnostic.is_lint.is_some() {
+            if is_error {
+                if is_lint {
                     self.lint_err_count += 1;
                 } else {
                     self.err_count += 1;

src/librustdoc/passes/lint/check_code_block_syntax.rs

@@ -156,7 +156,7 @@ impl Translate for BufferEmitter {
 }
 
 impl Emitter for BufferEmitter {
-    fn emit_diagnostic(&mut self, diag: &Diagnostic) {
+    fn emit_diagnostic(&mut self, diag: Diagnostic) {
         let mut buffer = self.buffer.borrow_mut();
 
         let fluent_args = to_fluent_args(diag.args());