Skip to content

Commit 781111e

Browse files
committed
Use Cow in {D,Subd}iagnosticMessage.
Each of `{D,Subd}iagnosticMessage::{Str,Eager}` has a comment: ``` // FIXME(davidtwco): can a `Cow<'static, str>` be used here? ``` This commit answers that question in the affirmative. It's not the most compelling change ever, but it might be worth merging. This requires changing the `impl<'a> From<&'a str>` impls to `impl From<&'static str>`, which involves a bunch of knock-on changes that require/result in call sites being a little more precise about exactly what kind of string they use to create errors, and not just `&str`. This will result in fewer unnecessary allocations, though this will not have any notable perf effects given that these are error paths. Note that I was lazy within Clippy, using `to_string` in a few places to preserve the existing string imprecision. I could have used `impl Into<{D,Subd}iagnosticMessage>` in various places as is done in the compiler, but that would have required changes to *many* call sites (mostly changing `&format("...")` to `format!("...")`) which didn't seem worthwhile.
1 parent 1c53407 commit 781111e

File tree

45 files changed

+308
-287
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+308
-287
lines changed

compiler/rustc_builtin_macros/src/compile_error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub fn expand_compile_error<'cx>(
1818
reason = "diagnostic message is specified by user"
1919
)]
2020
#[expect(rustc::untranslatable_diagnostic, reason = "diagnostic message is specified by user")]
21-
cx.span_err(sp, var.as_str());
21+
cx.span_err(sp, var.to_string());
2222

2323
DummyResult::any(sp)
2424
}

compiler/rustc_builtin_macros/src/errors.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for EnvNotDefined {
377377
rustc::untranslatable_diagnostic,
378378
reason = "cannot translate user-provided messages"
379379
)]
380-
handler.struct_diagnostic(msg.as_str())
380+
handler.struct_diagnostic(msg.to_string())
381381
} else {
382382
handler.struct_diagnostic(crate::fluent_generated::builtin_macros_env_not_defined)
383383
};

compiler/rustc_codegen_ssa/src/back/link.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,7 @@ fn link_natively<'a>(
893893
linker_path: &linker_path,
894894
exit_status: prog.status,
895895
command: &cmd,
896-
escaped_output: &escaped_output,
896+
escaped_output,
897897
};
898898
sess.diagnostic().emit_err(err);
899899
// If MSVC's `link.exe` was expected but the return code

compiler/rustc_codegen_ssa/src/back/write.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1800,7 +1800,7 @@ impl SharedEmitterMain {
18001800
handler.emit_diagnostic(&mut d);
18011801
}
18021802
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
1803-
let msg = msg.strip_prefix("error: ").unwrap_or(&msg);
1803+
let msg = msg.strip_prefix("error: ").unwrap_or(&msg).to_string();
18041804

18051805
let mut err = match level {
18061806
Level::Error { lint: false } => sess.struct_err(msg).forget_guarantee(),

compiler/rustc_codegen_ssa/src/errors.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ pub struct LinkingFailed<'a> {
336336
pub linker_path: &'a PathBuf,
337337
pub exit_status: ExitStatus,
338338
pub command: &'a Command,
339-
pub escaped_output: &'a str,
339+
pub escaped_output: String,
340340
}
341341

342342
impl IntoDiagnostic<'_> for LinkingFailed<'_> {
@@ -345,11 +345,13 @@ impl IntoDiagnostic<'_> for LinkingFailed<'_> {
345345
diag.set_arg("linker_path", format!("{}", self.linker_path.display()));
346346
diag.set_arg("exit_status", format!("{}", self.exit_status));
347347

348-
diag.note(format!("{:?}", self.command)).note(self.escaped_output);
348+
let contains_undefined_ref = self.escaped_output.contains("undefined reference to");
349+
350+
diag.note(format!("{:?}", self.command)).note(self.escaped_output.to_string());
349351

350352
// Trying to match an error from OS linkers
351353
// which by now we have no way to translate.
352-
if self.escaped_output.contains("undefined reference to") {
354+
if contains_undefined_ref {
353355
diag.note(fluent::codegen_ssa_extern_funcs_not_found)
354356
.note(fluent::codegen_ssa_specify_libraries_to_link)
355357
.note(fluent::codegen_ssa_use_cargo_directive);

compiler/rustc_driver_impl/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1258,7 +1258,7 @@ pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler))
12581258
if let Some(msg) = info.payload().downcast_ref::<String>() {
12591259
if msg.starts_with("failed printing to stdout: ") && msg.ends_with("(os error 232)") {
12601260
// the error code is already going to be reported when the panic unwinds up the stack
1261-
let _ = early_error_no_abort(ErrorOutputType::default(), msg.as_str());
1261+
let _ = early_error_no_abort(ErrorOutputType::default(), msg.clone());
12621262
return;
12631263
}
12641264
};

compiler/rustc_error_messages/src/lib.rs

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -263,8 +263,7 @@ type FluentId = Cow<'static, str>;
263263
#[rustc_diagnostic_item = "SubdiagnosticMessage"]
264264
pub enum SubdiagnosticMessage {
265265
/// Non-translatable diagnostic message.
266-
// FIXME(davidtwco): can a `Cow<'static, str>` be used here?
267-
Str(String),
266+
Str(Cow<'static, str>),
268267
/// Translatable message which has already been translated eagerly.
269268
///
270269
/// Some diagnostics have repeated subdiagnostics where the same interpolated variables would
@@ -275,8 +274,7 @@ pub enum SubdiagnosticMessage {
275274
/// incorrect diagnostics. Eager translation results in translation for a subdiagnostic
276275
/// happening immediately after the subdiagnostic derive's logic has been run. This variant
277276
/// stores messages which have been translated eagerly.
278-
// FIXME(#100717): can a `Cow<'static, str>` be used here?
279-
Eager(String),
277+
Eager(Cow<'static, str>),
280278
/// Identifier of a Fluent message. Instances of this variant are generated by the
281279
/// `Subdiagnostic` derive.
282280
FluentIdentifier(FluentId),
@@ -290,17 +288,17 @@ pub enum SubdiagnosticMessage {
290288

291289
impl From<String> for SubdiagnosticMessage {
292290
fn from(s: String) -> Self {
293-
SubdiagnosticMessage::Str(s)
291+
SubdiagnosticMessage::Str(Cow::Owned(s))
294292
}
295293
}
296-
impl<'a> From<&'a str> for SubdiagnosticMessage {
297-
fn from(s: &'a str) -> Self {
298-
SubdiagnosticMessage::Str(s.to_string())
294+
impl From<&'static str> for SubdiagnosticMessage {
295+
fn from(s: &'static str) -> Self {
296+
SubdiagnosticMessage::Str(Cow::Borrowed(s))
299297
}
300298
}
301299
impl From<Cow<'static, str>> for SubdiagnosticMessage {
302300
fn from(s: Cow<'static, str>) -> Self {
303-
SubdiagnosticMessage::Str(s.to_string())
301+
SubdiagnosticMessage::Str(s)
304302
}
305303
}
306304

@@ -312,8 +310,7 @@ impl From<Cow<'static, str>> for SubdiagnosticMessage {
312310
#[rustc_diagnostic_item = "DiagnosticMessage"]
313311
pub enum DiagnosticMessage {
314312
/// Non-translatable diagnostic message.
315-
// FIXME(#100717): can a `Cow<'static, str>` be used here?
316-
Str(String),
313+
Str(Cow<'static, str>),
317314
/// Translatable message which has already been translated eagerly.
318315
///
319316
/// Some diagnostics have repeated subdiagnostics where the same interpolated variables would
@@ -324,8 +321,7 @@ pub enum DiagnosticMessage {
324321
/// incorrect diagnostics. Eager translation results in translation for a subdiagnostic
325322
/// happening immediately after the subdiagnostic derive's logic has been run. This variant
326323
/// stores messages which have been translated eagerly.
327-
// FIXME(#100717): can a `Cow<'static, str>` be used here?
328-
Eager(String),
324+
Eager(Cow<'static, str>),
329325
/// Identifier for a Fluent message (with optional attribute) corresponding to the diagnostic
330326
/// message.
331327
///
@@ -363,17 +359,17 @@ impl DiagnosticMessage {
363359

364360
impl From<String> for DiagnosticMessage {
365361
fn from(s: String) -> Self {
366-
DiagnosticMessage::Str(s)
362+
DiagnosticMessage::Str(Cow::Owned(s))
367363
}
368364
}
369-
impl<'a> From<&'a str> for DiagnosticMessage {
370-
fn from(s: &'a str) -> Self {
371-
DiagnosticMessage::Str(s.to_string())
365+
impl From<&'static str> for DiagnosticMessage {
366+
fn from(s: &'static str) -> Self {
367+
DiagnosticMessage::Str(Cow::Borrowed(s))
372368
}
373369
}
374370
impl From<Cow<'static, str>> for DiagnosticMessage {
375371
fn from(s: Cow<'static, str>) -> Self {
376-
DiagnosticMessage::Str(s.to_string())
372+
DiagnosticMessage::Str(s)
377373
}
378374
}
379375

compiler/rustc_errors/src/diagnostic.rs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -352,14 +352,9 @@ impl Diagnostic {
352352

353353
/// Labels all the given spans with the provided label.
354354
/// See [`Self::span_label()`] for more information.
355-
pub fn span_labels(
356-
&mut self,
357-
spans: impl IntoIterator<Item = Span>,
358-
label: impl AsRef<str>,
359-
) -> &mut Self {
360-
let label = label.as_ref();
355+
pub fn span_labels(&mut self, spans: impl IntoIterator<Item = Span>, label: &str) -> &mut Self {
361356
for span in spans {
362-
self.span_label(span, label);
357+
self.span_label(span, label.to_string());
363358
}
364359
self
365360
}
@@ -394,17 +389,18 @@ impl Diagnostic {
394389
expected: DiagnosticStyledString,
395390
found: DiagnosticStyledString,
396391
) -> &mut Self {
397-
let mut msg: Vec<_> = vec![("required when trying to coerce from type `", Style::NoStyle)];
392+
let mut msg: Vec<_> =
393+
vec![(Cow::from("required when trying to coerce from type `"), Style::NoStyle)];
398394
msg.extend(expected.0.iter().map(|x| match *x {
399-
StringPart::Normal(ref s) => (s.as_str(), Style::NoStyle),
400-
StringPart::Highlighted(ref s) => (s.as_str(), Style::Highlight),
395+
StringPart::Normal(ref s) => (Cow::from(s.clone()), Style::NoStyle),
396+
StringPart::Highlighted(ref s) => (Cow::from(s.clone()), Style::Highlight),
401397
}));
402-
msg.push(("` to type '", Style::NoStyle));
398+
msg.push((Cow::from("` to type '"), Style::NoStyle));
403399
msg.extend(found.0.iter().map(|x| match *x {
404-
StringPart::Normal(ref s) => (s.as_str(), Style::NoStyle),
405-
StringPart::Highlighted(ref s) => (s.as_str(), Style::Highlight),
400+
StringPart::Normal(ref s) => (Cow::from(s.clone()), Style::NoStyle),
401+
StringPart::Highlighted(ref s) => (Cow::from(s.clone()), Style::Highlight),
406402
}));
407-
msg.push(("`", Style::NoStyle));
403+
msg.push((Cow::from("`"), Style::NoStyle));
408404

409405
// For now, just attach these as notes
410406
self.highlighted_note(msg);

compiler/rustc_errors/src/diagnostic_builder.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -558,7 +558,7 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
558558
}
559559

560560
// Take the `Diagnostic` by replacing it with a dummy.
561-
let dummy = Diagnostic::new(Level::Allow, DiagnosticMessage::Str("".to_string()));
561+
let dummy = Diagnostic::new(Level::Allow, DiagnosticMessage::from(""));
562562
let diagnostic = std::mem::replace(&mut *self.inner.diagnostic, dummy);
563563

564564
// Disable the ICE on `Drop`.
@@ -627,7 +627,7 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
627627
pub fn span_labels(
628628
&mut self,
629629
spans: impl IntoIterator<Item = Span>,
630-
label: impl AsRef<str>,
630+
label: &str,
631631
) -> &mut Self);
632632

633633
forward!(pub fn note_expected_found(
@@ -781,8 +781,8 @@ impl Drop for DiagnosticBuilderInner<'_> {
781781
if !panicking() {
782782
handler.emit_diagnostic(&mut Diagnostic::new(
783783
Level::Bug,
784-
DiagnosticMessage::Str(
785-
"the following error was constructed but not emitted".to_string(),
784+
DiagnosticMessage::from(
785+
"the following error was constructed but not emitted",
786786
),
787787
));
788788
handler.emit_diagnostic(&mut self.diagnostic);

compiler/rustc_errors/src/emitter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ pub trait Emitter: Translate {
367367

368368
children.push(SubDiagnostic {
369369
level: Level::Note,
370-
message: vec![(DiagnosticMessage::Str(msg), Style::NoStyle)],
370+
message: vec![(DiagnosticMessage::from(msg), Style::NoStyle)],
371371
span: MultiSpan::new(),
372372
render_span: None,
373373
});

0 commit comments

Comments
 (0)