Skip to content

Commit 8e9eb16

Browse files
nnethercotennethercote
committed
Make Emitter::emit_diagnostic consuming.
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.
1 parent a5404f1 commit 8e9eb16

File tree

7 files changed

+86
-76
lines changed

7 files changed

+86
-76
lines changed

‎compiler/rustc_codegen_ssa/src/back/write.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1799,8 +1799,8 @@ impl Translate for SharedEmitter {
17991799
}
18001800

18011801
impl Emitter for SharedEmitter {
1802-
fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) {
1803-
drop(self.sender.send(SharedEmitterMessage::Diagnostic(diag.clone())));
1802+
fn emit_diagnostic(&mut self, diag: rustc_errors::Diagnostic) {
1803+
drop(self.sender.send(SharedEmitterMessage::Diagnostic(diag)));
18041804
}
18051805

18061806
fn source_map(&self) -> Option<&Lrc<SourceMap>> {

‎compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,15 @@ impl Translate for AnnotateSnippetEmitter {
4444

4545
impl Emitter for AnnotateSnippetEmitter {
4646
/// The entry point for the diagnostics generation
47-
fn emit_diagnostic(&mut self, diag: &Diagnostic) {
47+
fn emit_diagnostic(&mut self, mut diag: Diagnostic) {
4848
let fluent_args = to_fluent_args(diag.args());
4949

50-
let mut children = diag.children.clone();
51-
let (mut primary_span, suggestions) = self.primary_span_formatted(diag, &fluent_args);
50+
let mut suggestions = diag.suggestions.unwrap_or(vec![]);
51+
self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args);
5252

5353
self.fix_multispans_in_extern_macros_and_render_macro_backtrace(
54-
&mut primary_span,
55-
&mut children,
54+
&mut diag.span,
55+
&mut diag.children,
5656
&diag.level,
5757
self.macro_backtrace,
5858
);
@@ -62,9 +62,9 @@ impl Emitter for AnnotateSnippetEmitter {
6262
&diag.messages,
6363
&fluent_args,
6464
&diag.code,
65-
&primary_span,
66-
&children,
67-
suggestions,
65+
&diag.span,
66+
&diag.children,
67+
&suggestions,
6868
);
6969
}
7070

‎compiler/rustc_errors/src/emitter.rs

+20-22
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ pub type DynEmitter = dyn Emitter + DynSend;
193193
/// Emitter trait for emitting errors.
194194
pub trait Emitter: Translate {
195195
/// Emit a structured diagnostic.
196-
fn emit_diagnostic(&mut self, diag: &Diagnostic);
196+
fn emit_diagnostic(&mut self, diag: Diagnostic);
197197

198198
/// Emit a notification that an artifact has been output.
199199
/// Currently only supported for the JSON format.
@@ -224,23 +224,24 @@ pub trait Emitter: Translate {
224224

225225
fn source_map(&self) -> Option<&Lrc<SourceMap>>;
226226

227+
/// njn: update comment
227228
/// Formats the substitutions of the primary_span
228229
///
229230
/// There are a lot of conditions to this method, but in short:
230231
///
231232
/// * If the current `Diagnostic` has only one visible `CodeSuggestion`,
232233
/// we format the `help` suggestion depending on the content of the
233-
/// substitutions. In that case, we return the modified span only.
234+
/// substitutions. In that case, we modify the span and clear the
235+
/// suggestions.
234236
///
235237
/// * If the current `Diagnostic` has multiple suggestions,
236-
/// we return the original `primary_span` and the original suggestions.
237-
fn primary_span_formatted<'a>(
238+
/// we leave `primary_span` and the suggestions untouched.
239+
fn primary_span_formatted(
238240
&mut self,
239-
diag: &'a Diagnostic,
241+
primary_span: &mut MultiSpan,
242+
suggestions: &mut Vec<CodeSuggestion>,
240243
fluent_args: &FluentArgs<'_>,
241-
) -> (MultiSpan, &'a [CodeSuggestion]) {
242-
let mut primary_span = diag.span.clone();
243-
let suggestions = diag.suggestions.as_deref().unwrap_or(&[]);
244+
) {
244245
if let Some((sugg, rest)) = suggestions.split_first() {
245246
let msg = self.translate_message(&sugg.msg, fluent_args).map_err(Report::new).unwrap();
246247
if rest.is_empty() &&
@@ -287,16 +288,15 @@ pub trait Emitter: Translate {
287288
primary_span.push_span_label(sugg.substitutions[0].parts[0].span, msg);
288289

289290
// We return only the modified primary_span
290-
(primary_span, &[])
291+
suggestions.clear();
291292
} else {
292293
// if there are multiple suggestions, print them all in full
293294
// to be consistent. We could try to figure out if we can
294295
// make one (or the first one) inline, but that would give
295296
// undue importance to a semi-random suggestion
296-
(primary_span, suggestions)
297297
}
298298
} else {
299-
(primary_span, suggestions)
299+
// do nothing
300300
}
301301
}
302302

@@ -518,16 +518,15 @@ impl Emitter for HumanEmitter {
518518
self.sm.as_ref()
519519
}
520520

521-
fn emit_diagnostic(&mut self, diag: &Diagnostic) {
521+
fn emit_diagnostic(&mut self, mut diag: Diagnostic) {
522522
let fluent_args = to_fluent_args(diag.args());
523523

524-
let mut children = diag.children.clone();
525-
let (mut primary_span, suggestions) = self.primary_span_formatted(diag, &fluent_args);
526-
debug!("emit_diagnostic: suggestions={:?}", suggestions);
524+
let mut suggestions = diag.suggestions.unwrap_or(vec![]);
525+
self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args);
527526

528527
self.fix_multispans_in_extern_macros_and_render_macro_backtrace(
529-
&mut primary_span,
530-
&mut children,
528+
&mut diag.span,
529+
&mut diag.children,
531530
&diag.level,
532531
self.macro_backtrace,
533532
);
@@ -537,9 +536,9 @@ impl Emitter for HumanEmitter {
537536
&diag.messages,
538537
&fluent_args,
539538
&diag.code,
540-
&primary_span,
541-
&children,
542-
suggestions,
539+
&diag.span,
540+
&diag.children,
541+
&suggestions,
543542
self.track_diagnostics.then_some(&diag.emitted_at),
544543
);
545544
}
@@ -576,9 +575,8 @@ impl Emitter for SilentEmitter {
576575
None
577576
}
578577

579-
fn emit_diagnostic(&mut self, d: &Diagnostic) {
578+
fn emit_diagnostic(&mut self, mut d: Diagnostic) {
580579
if d.level == Level::Fatal {
581-
let mut d = d.clone();
582580
if let Some(ref note) = self.fatal_note {
583581
d.note(note.clone());
584582
}

‎compiler/rustc_errors/src/json.rs

+28-24
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ impl Translate for JsonEmitter {
176176
}
177177

178178
impl Emitter for JsonEmitter {
179-
fn emit_diagnostic(&mut self, diag: &crate::Diagnostic) {
179+
fn emit_diagnostic(&mut self, diag: crate::Diagnostic) {
180180
let data = Diagnostic::from_errors_diagnostic(diag, self);
181181
let result = self.emit(EmitTyped::Diagnostic(data));
182182
if let Err(e) = result {
@@ -201,7 +201,7 @@ impl Emitter for JsonEmitter {
201201
}
202202
FutureBreakageItem {
203203
diagnostic: EmitTyped::Diagnostic(Diagnostic::from_errors_diagnostic(
204-
&diag, self,
204+
diag, self,
205205
)),
206206
}
207207
})
@@ -340,7 +340,7 @@ struct UnusedExterns<'a, 'b, 'c> {
340340
}
341341

342342
impl Diagnostic {
343-
fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic {
343+
fn from_errors_diagnostic(diag: crate::Diagnostic, je: &JsonEmitter) -> Diagnostic {
344344
let args = to_fluent_args(diag.args());
345345
let sugg = diag.suggestions.iter().flatten().map(|sugg| {
346346
let translated_message =
@@ -382,6 +382,28 @@ impl Diagnostic {
382382
Ok(())
383383
}
384384
}
385+
386+
let translated_message = je.translate_messages(&diag.messages, &args);
387+
388+
let code = if let Some(code) = diag.code {
389+
Some(DiagnosticCode {
390+
code: code.to_string(),
391+
explanation: je.registry.as_ref().unwrap().try_find_description(code).ok(),
392+
})
393+
} else if let Some(IsLint { name, .. }) = &diag.is_lint {
394+
Some(DiagnosticCode { code: name.to_string(), explanation: None })
395+
} else {
396+
None
397+
};
398+
let level = diag.level.to_str();
399+
let spans = DiagnosticSpan::from_multispan(&diag.span, &args, je);
400+
let children = diag
401+
.children
402+
.iter()
403+
.map(|c| Diagnostic::from_sub_diagnostic(c, &args, je))
404+
.chain(sugg)
405+
.collect();
406+
385407
let buf = BufWriter::default();
386408
let output = buf.clone();
387409
je.json_rendered
@@ -398,30 +420,12 @@ impl Diagnostic {
398420
let output = Arc::try_unwrap(output.0).unwrap().into_inner().unwrap();
399421
let output = String::from_utf8(output).unwrap();
400422

401-
let translated_message = je.translate_messages(&diag.messages, &args);
402-
403-
let code = if let Some(code) = diag.code {
404-
Some(DiagnosticCode {
405-
code: code.to_string(),
406-
explanation: je.registry.as_ref().unwrap().try_find_description(code).ok(),
407-
})
408-
} else if let Some(IsLint { name, .. }) = &diag.is_lint {
409-
Some(DiagnosticCode { code: name.to_string(), explanation: None })
410-
} else {
411-
None
412-
};
413-
414423
Diagnostic {
415424
message: translated_message.to_string(),
416425
code,
417-
level: diag.level.to_str(),
418-
spans: DiagnosticSpan::from_multispan(&diag.span, &args, je),
419-
children: diag
420-
.children
421-
.iter()
422-
.map(|c| Diagnostic::from_sub_diagnostic(c, &args, je))
423-
.chain(sugg)
424-
.collect(),
426+
level,
427+
spans,
428+
children,
425429
rendered: Some(output),
426430
}
427431
}

‎compiler/rustc_errors/src/lib.rs

+17-9
Original file line numberDiff line numberDiff line change
@@ -991,9 +991,13 @@ impl DiagCtxt {
991991

992992
match (errors.len(), warnings.len()) {
993993
(0, 0) => return,
994-
(0, _) => inner
995-
.emitter
996-
.emit_diagnostic(&Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))),
994+
(0, _) => {
995+
// Use `inner.emitter` directly, otherwise the warning might not be emitted, e.g.
996+
// with a configuration like `--cap-lints allow --force-warn bare_trait_objects`.
997+
inner
998+
.emitter
999+
.emit_diagnostic(Diagnostic::new(Warning, DiagnosticMessage::Str(warnings)));
1000+
}
9971001
(_, 0) => {
9981002
inner.emit_diagnostic(Diagnostic::new(Fatal, errors));
9991003
}
@@ -1061,7 +1065,7 @@ impl DiagCtxt {
10611065
}
10621066

10631067
pub fn force_print_diagnostic(&self, db: Diagnostic) {
1064-
self.inner.borrow_mut().emitter.emit_diagnostic(&db);
1068+
self.inner.borrow_mut().emitter.emit_diagnostic(db);
10651069
}
10661070

10671071
pub fn emit_diagnostic(&self, diagnostic: Diagnostic) -> Option<ErrorGuaranteed> {
@@ -1328,11 +1332,15 @@ impl DiagCtxtInner {
13281332
!self.emitted_diagnostics.insert(diagnostic_hash)
13291333
};
13301334

1335+
let is_error = diagnostic.is_error();
1336+
let is_lint = diagnostic.is_lint.is_some();
1337+
13311338
// Only emit the diagnostic if we've been asked to deduplicate or
13321339
// haven't already emitted an equivalent diagnostic.
13331340
if !(self.flags.deduplicate_diagnostics && already_emitted) {
13341341
debug!(?diagnostic);
13351342
debug!(?self.emitted_diagnostics);
1343+
13361344
let already_emitted_sub = |sub: &mut SubDiagnostic| {
13371345
debug!(?sub);
13381346
if sub.level != OnceNote && sub.level != OnceHelp {
@@ -1344,24 +1352,24 @@ impl DiagCtxtInner {
13441352
debug!(?diagnostic_hash);
13451353
!self.emitted_diagnostics.insert(diagnostic_hash)
13461354
};
1347-
13481355
diagnostic.children.extract_if(already_emitted_sub).for_each(|_| {});
13491356
if already_emitted {
13501357
diagnostic.note(
13511358
"duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`",
13521359
);
13531360
}
13541361

1355-
self.emitter.emit_diagnostic(&diagnostic);
1356-
if diagnostic.is_error() {
1362+
if is_error {
13571363
self.deduplicated_err_count += 1;
13581364
} else if matches!(diagnostic.level, ForceWarning(_) | Warning) {
13591365
self.deduplicated_warn_count += 1;
13601366
}
13611367
self.has_printed = true;
1368+
1369+
self.emitter.emit_diagnostic(diagnostic);
13621370
}
1363-
if diagnostic.is_error() {
1364-
if diagnostic.is_lint.is_some() {
1371+
if is_error {
1372+
if is_lint {
13651373
self.lint_err_count += 1;
13661374
} else {
13671375
self.err_count += 1;

‎src/librustdoc/passes/lint/check_code_block_syntax.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ impl Translate for BufferEmitter {
156156
}
157157

158158
impl Emitter for BufferEmitter {
159-
fn emit_diagnostic(&mut self, diag: &Diagnostic) {
159+
fn emit_diagnostic(&mut self, diag: Diagnostic) {
160160
let mut buffer = self.buffer.borrow_mut();
161161

162162
let fluent_args = to_fluent_args(diag.args());

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

+10-10
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ impl Emitter for SilentEmitter {
4747
None
4848
}
4949

50-
fn emit_diagnostic(&mut self, _db: &Diagnostic) {}
50+
fn emit_diagnostic(&mut self, _db: Diagnostic) {}
5151
}
5252

5353
fn silent_emitter() -> Box<DynEmitter> {
@@ -64,7 +64,7 @@ struct SilentOnIgnoredFilesEmitter {
6464
}
6565

6666
impl SilentOnIgnoredFilesEmitter {
67-
fn handle_non_ignoreable_error(&mut self, db: &Diagnostic) {
67+
fn handle_non_ignoreable_error(&mut self, db: Diagnostic) {
6868
self.has_non_ignorable_parser_errors = true;
6969
self.can_reset.store(false, Ordering::Release);
7070
self.emitter.emit_diagnostic(db);
@@ -86,7 +86,7 @@ impl Emitter for SilentOnIgnoredFilesEmitter {
8686
None
8787
}
8888

89-
fn emit_diagnostic(&mut self, db: &Diagnostic) {
89+
fn emit_diagnostic(&mut self, db: Diagnostic) {
9090
if db.level() == DiagnosticLevel::Fatal {
9191
return self.handle_non_ignoreable_error(db);
9292
}
@@ -365,7 +365,7 @@ mod tests {
365365
None
366366
}
367367

368-
fn emit_diagnostic(&mut self, _db: &Diagnostic) {
368+
fn emit_diagnostic(&mut self, _db: Diagnostic) {
369369
self.num_emitted_errors.fetch_add(1, Ordering::Release);
370370
}
371371
}
@@ -424,7 +424,7 @@ mod tests {
424424
);
425425
let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
426426
let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, Some(span));
427-
emitter.emit_diagnostic(&fatal_diagnostic);
427+
emitter.emit_diagnostic(fatal_diagnostic);
428428
assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1);
429429
assert_eq!(can_reset_errors.load(Ordering::Acquire), false);
430430
}
@@ -449,7 +449,7 @@ mod tests {
449449
);
450450
let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
451451
let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span));
452-
emitter.emit_diagnostic(&non_fatal_diagnostic);
452+
emitter.emit_diagnostic(non_fatal_diagnostic);
453453
assert_eq!(num_emitted_errors.load(Ordering::Acquire), 0);
454454
assert_eq!(can_reset_errors.load(Ordering::Acquire), true);
455455
}
@@ -473,7 +473,7 @@ mod tests {
473473
);
474474
let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1)));
475475
let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span));
476-
emitter.emit_diagnostic(&non_fatal_diagnostic);
476+
emitter.emit_diagnostic(non_fatal_diagnostic);
477477
assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1);
478478
assert_eq!(can_reset_errors.load(Ordering::Acquire), false);
479479
}
@@ -512,9 +512,9 @@ mod tests {
512512
let bar_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(bar_span));
513513
let foo_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(foo_span));
514514
let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, None);
515-
emitter.emit_diagnostic(&bar_diagnostic);
516-
emitter.emit_diagnostic(&foo_diagnostic);
517-
emitter.emit_diagnostic(&fatal_diagnostic);
515+
emitter.emit_diagnostic(bar_diagnostic);
516+
emitter.emit_diagnostic(foo_diagnostic);
517+
emitter.emit_diagnostic(fatal_diagnostic);
518518
assert_eq!(num_emitted_errors.load(Ordering::Acquire), 2);
519519
assert_eq!(can_reset_errors.load(Ordering::Acquire), false);
520520
}

0 commit comments

Comments
 (0)