Skip to content

Commit

Permalink
Make Emitter::emit_diagnostic consuming.
Browse files Browse the repository at this point in the history
All the other `emit`/`emit_diagnostic` methods were recently made
consuming (e.g. #119606), but this one wasn't. But it makes sense to.

Much of this is straightforward, and lots of `clone` calls are avoided.
There are a couple of tricky bits.
- `Emitter::primary_span_formatted` no longer takes a `Diagnostic` and
  returns a pair. Instead it takes the two fields from `Diagnostic` that
  it used (`span` and `suggestions`) as `&mut`, and modifies them. This
  is necessary to avoid the cloning of `diag.children` in two emitters.
- `from_errors_diagnostic` is rearranged so various uses of `diag` occur
  before the consuming `emit_diagnostic` call.
  • Loading branch information
nnethercote committed Feb 2, 2024
1 parent a5404f1 commit 226b3f3
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 76 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1799,8 +1799,8 @@ impl Translate for SharedEmitter {
}

impl Emitter for SharedEmitter {
fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) {
drop(self.sender.send(SharedEmitterMessage::Diagnostic(diag.clone())));
fn emit_diagnostic(&mut self, diag: rustc_errors::Diagnostic) {
drop(self.sender.send(SharedEmitterMessage::Diagnostic(diag)));
}

fn source_map(&self) -> Option<&Lrc<SourceMap>> {
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs
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
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, 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());
}
Expand Down
52 changes: 28 additions & 24 deletions compiler/rustc_errors/src/json.rs
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
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 @@ -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> {
Expand Down Expand Up @@ -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 {
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/lint/check_code_block_syntax.rs
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
20 changes: 10 additions & 10 deletions src/tools/rustfmt/src/parse/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 226b3f3

Please sign in to comment.