Skip to content

Commit

Permalink
Rollup merge of #120575 - nnethercote:simplify-codegen-diag-handling,…
Browse files Browse the repository at this point in the history 
… r=estebank

Simplify codegen diagnostic handling

Some nice improvements. Details in the individual commit logs.

r? ``@estebank``
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Feb 5, 2024
2 parents 749af61 + f4dce1e commit bc0d618
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 116 deletions.
45 changes: 5 additions & 40 deletions compiler/rustc_codegen_ssa/src/back/write.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -997,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.
Expand Down Expand Up @@ -1764,7 +1754,6 @@ fn spawn_work<'a, B: ExtraBackendMethods>(
enum SharedEmitterMessage {
Diagnostic(Diagnostic),
InlineAsmError(u32, String, Level, Option<(String, Vec<InnerSpan>)>),
AbortIfErrors,
Fatal(String),
}

Expand Down Expand Up @@ -1810,24 +1799,8 @@ impl Translate for SharedEmitter {
}

impl Emitter for SharedEmitter {
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 {
msgs: diag.messages.clone(),
args: args.clone(),
code: diag.code,
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>> {
Expand All @@ -1852,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));
Expand Down Expand Up @@ -1891,9 +1859,6 @@ impl SharedEmitterMain {

err.emit();
}
Ok(SharedEmitterMessage::AbortIfErrors) => {
sess.dcx().abort_if_errors();
}
Ok(SharedEmitterMessage::Fatal(msg)) => {
sess.dcx().fatal(msg);
}
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand All @@ -62,9 +62,9 @@ impl Emitter for AnnotateSnippetEmitter {
&diag.messages,
&fluent_args,
&diag.code,
&primary_span,
&children,
suggestions,
&diag.span,
&diag.children,
&suggestions,
);
}

Expand Down
41 changes: 19 additions & 22 deletions compiler/rustc_errors/src/emitter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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() &&
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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,
);
Expand All @@ -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),
);
}
Expand Down Expand Up @@ -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);
}
Expand Down
52 changes: 28 additions & 24 deletions compiler/rustc_errors/src/json.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -201,7 +201,7 @@ impl Emitter for JsonEmitter {
}
FutureBreakageItem {
diagnostic: EmitTyped::Diagnostic(Diagnostic::from_errors_diagnostic(
&diag, self,
diag, self,
)),
}
})
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand All @@ -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),
}
}
Expand Down
26 changes: 17 additions & 9 deletions compiler/rustc_errors/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down Expand Up @@ -1057,7 +1061,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> {
Expand Down Expand Up @@ -1325,11 +1329,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 {
Expand All @@ -1341,24 +1349,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;
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/lint/check_code_block_syntax.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading

0 comments on commit bc0d618

Please sign in to comment.