Skip to content

Commit eaa0d57

Browse files
committed
Auto merge of #110579 - nnethercote:restrict-From-for-Diagnostics, r=davidtwco
Restrict `From<S>` for `{D,Subd}iagnosticMessage`. Currently a `{D,Subd}iagnosticMessage` can be created from any type that impls `Into<String>`. That includes `&str`, `String`, and `Cow<'static, str>`, which are reasonable. It also includes `&String`, which is pretty weird, and results in many places making unnecessary allocations for patterns like this: ``` self.fatal(&format!(...)) ``` This creates a string with `format!`, takes a reference, passes the reference to `fatal`, which does an `into()`, which clones the reference, doing a second allocation. Two allocations for a single string, bleh. This commit changes the `From` impls so that you can only create a `{D,Subd}iagnosticMessage` from `&str`, `String`, or `Cow<'static, str>`. This requires changing all the places that currently create one from a `&String`. Most of these are of the `&format!(...)` form described above; each one removes an unnecessary static `&`, plus an allocation when executed. There are also a few places where the existing use of `&String` was more reasonable; these now just use `clone()` at the call site. As well as making the code nicer and more efficient, this is a step towards possibly using `Cow<'static, str>` in `{D,Subd}iagnosticMessage::{Str,Eager}`. That would require changing the `From<&'a str>` impls to `From<&'static str>`, which is doable, but I'm not yet sure if it's worthwhile. r? `@davidtwco`
2 parents 76c4936 + bea7822 commit eaa0d57

File tree

1 file changed

+7
-5
lines changed

1 file changed

+7
-5
lines changed

src/diagnostics.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -409,30 +409,32 @@ pub fn report_msg<'tcx>(
409409
} else {
410410
// Make sure we show the message even when it is a dummy span.
411411
for line in span_msg {
412-
err.note(&line);
412+
err.note(line);
413413
}
414414
err.note("(no span available)");
415415
}
416416

417417
// Show note and help messages.
418418
let mut extra_span = false;
419-
for (span_data, note) in &notes {
419+
let notes_len = notes.len();
420+
for (span_data, note) in notes {
420421
if let Some(span_data) = span_data {
421422
err.span_note(span_data.span(), note);
422423
extra_span = true;
423424
} else {
424425
err.note(note);
425426
}
426427
}
427-
for (span_data, help) in &helps {
428+
let helps_len = helps.len();
429+
for (span_data, help) in helps {
428430
if let Some(span_data) = span_data {
429431
err.span_help(span_data.span(), help);
430432
extra_span = true;
431433
} else {
432434
err.help(help);
433435
}
434436
}
435-
if notes.len() + helps.len() > 0 {
437+
if notes_len + helps_len > 0 {
436438
// Add visual separator before backtrace.
437439
err.note(if extra_span { "BACKTRACE (of the first span):" } else { "BACKTRACE:" });
438440
}
@@ -441,7 +443,7 @@ pub fn report_msg<'tcx>(
441443
let is_local = machine.is_local(frame_info);
442444
// No span for non-local frames and the first frame (which is the error site).
443445
if is_local && idx > 0 {
444-
err.span_note(frame_info.span, &frame_info.to_string());
446+
err.span_note(frame_info.span, frame_info.to_string());
445447
} else {
446448
let sm = sess.source_map();
447449
let span = sm.span_to_embeddable_string(frame_info.span);

0 commit comments

Comments
 (0)