Rollup merge of rust-lang#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

@@ -1810,7 +1810,7 @@ impl Translate for SharedEmitter {
 }
 
 impl Emitter for SharedEmitter {
-    fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) {
+    fn emit_diagnostic(&mut self, diag: rustc_errors::Diagnostic) {
         let args: FxHashMap<DiagnosticArgName, DiagnosticArgValue> =
             diag.args().map(|(name, arg)| (name.clone(), arg.clone())).collect();
         drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic {

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, diag: &Diagnostic) {
+    fn emit_diagnostic(&mut self, mut diag: Diagnostic) {
         if diag.level == Level::Fatal {
-            let mut diag = diag.clone();
             diag.note(self.fatal_note.clone());
             self.fatal_dcx.emit_diagnostic(diag);
         }

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

@@ -981,9 +981,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));
             }
@@ -1047,7 +1051,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> {
@@ -1315,11 +1319,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 {
@@ -1331,24 +1339,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());

src/tools/rustfmt/src/parse/session.rs

@@ -47,7 +47,7 @@ impl Emitter for SilentEmitter {
         None
     }
 
-    fn emit_diagnostic(&mut self, _db: &Diagnostic) {}
+    fn emit_diagnostic(&mut self, _db: Diagnostic) {}
 }
 
 fn silent_emitter() -> Box<DynEmitter> {
@@ -64,7 +64,7 @@ struct SilentOnIgnoredFilesEmitter {
 }
 
 impl SilentOnIgnoredFilesEmitter {
-    fn handle_non_ignoreable_error(&mut self, db: &Diagnostic) {
+    fn handle_non_ignoreable_error(&mut self, db: Diagnostic) {
         self.has_non_ignorable_parser_errors = true;
         self.can_reset.store(false, Ordering::Release);
         self.emitter.emit_diagnostic(db);
@@ -86,7 +86,7 @@ impl Emitter for SilentOnIgnoredFilesEmitter {
         None
     }
 
-    fn emit_diagnostic(&mut self, db: &Diagnostic) {
+    fn emit_diagnostic(&mut self, db: Diagnostic) {
         if db.level() == DiagnosticLevel::Fatal {
             return self.handle_non_ignoreable_error(db);
         }
@@ -365,7 +365,7 @@ mod tests {
                 None
             }
 
-            fn emit_diagnostic(&mut self, _db: &Diagnostic) {
+            fn emit_diagnostic(&mut self, _db: Diagnostic) {
                 self.num_emitted_errors.fetch_add(1, Ordering::Release);
             }
         }
@@ -424,7 +424,7 @@ mod tests {
             );
             let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
             let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, Some(span));
-            emitter.emit_diagnostic(&fatal_diagnostic);
+            emitter.emit_diagnostic(fatal_diagnostic);
             assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1);
             assert_eq!(can_reset_errors.load(Ordering::Acquire), false);
         }
@@ -449,7 +449,7 @@ mod tests {
             );
             let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
             let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span));
-            emitter.emit_diagnostic(&non_fatal_diagnostic);
+            emitter.emit_diagnostic(non_fatal_diagnostic);
             assert_eq!(num_emitted_errors.load(Ordering::Acquire), 0);
             assert_eq!(can_reset_errors.load(Ordering::Acquire), true);
         }
@@ -473,7 +473,7 @@ mod tests {
             );
             let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
             let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span));
-            emitter.emit_diagnostic(&non_fatal_diagnostic);
+            emitter.emit_diagnostic(non_fatal_diagnostic);
             assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1);
             assert_eq!(can_reset_errors.load(Ordering::Acquire), false);
         }
@@ -512,9 +512,9 @@ mod tests {
             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);
-            emitter.emit_diagnostic(&fatal_diagnostic);
+            emitter.emit_diagnostic(bar_diagnostic);
+            emitter.emit_diagnostic(foo_diagnostic);
+            emitter.emit_diagnostic(fatal_diagnostic);
             assert_eq!(num_emitted_errors.load(Ordering::Acquire), 2);
             assert_eq!(can_reset_errors.load(Ordering::Acquire), false);
         }