Skip to content

Commit

Permalink
Use Cow in {D,Subd}iagnosticMessage.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nnethercote committed May 28, 2023
1 parent 1c53407 commit 781111e
Show file tree
Hide file tree
Showing 45 changed files with 308 additions and 287 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/compile_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub fn expand_compile_error<'cx>(
reason = "diagnostic message is specified by user"
)]
#[expect(rustc::untranslatable_diagnostic, reason = "diagnostic message is specified by user")]
cx.span_err(sp, var.as_str());
cx.span_err(sp, var.to_string());

DummyResult::any(sp)
}
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for EnvNotDefined {
rustc::untranslatable_diagnostic,
reason = "cannot translate user-provided messages"
)]
handler.struct_diagnostic(msg.as_str())
handler.struct_diagnostic(msg.to_string())
} else {
handler.struct_diagnostic(crate::fluent_generated::builtin_macros_env_not_defined)
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -893,7 +893,7 @@ fn link_natively<'a>(
linker_path: &linker_path,
exit_status: prog.status,
command: &cmd,
escaped_output: &escaped_output,
escaped_output,
};
sess.diagnostic().emit_err(err);
// If MSVC's `link.exe` was expected but the return code
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1800,7 +1800,7 @@ impl SharedEmitterMain {
handler.emit_diagnostic(&mut d);
}
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
let msg = msg.strip_prefix("error: ").unwrap_or(&msg);
let msg = msg.strip_prefix("error: ").unwrap_or(&msg).to_string();

let mut err = match level {
Level::Error { lint: false } => sess.struct_err(msg).forget_guarantee(),
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ pub struct LinkingFailed<'a> {
pub linker_path: &'a PathBuf,
pub exit_status: ExitStatus,
pub command: &'a Command,
pub escaped_output: &'a str,
pub escaped_output: String,
}

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

diag.note(format!("{:?}", self.command)).note(self.escaped_output);
let contains_undefined_ref = self.escaped_output.contains("undefined reference to");

diag.note(format!("{:?}", self.command)).note(self.escaped_output.to_string());

// Trying to match an error from OS linkers
// which by now we have no way to translate.
if self.escaped_output.contains("undefined reference to") {
if contains_undefined_ref {
diag.note(fluent::codegen_ssa_extern_funcs_not_found)
.note(fluent::codegen_ssa_specify_libraries_to_link)
.note(fluent::codegen_ssa_use_cargo_directive);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_driver_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1258,7 +1258,7 @@ pub fn install_ice_hook(bug_report_url: &'static str, extra_info: fn(&Handler))
if let Some(msg) = info.payload().downcast_ref::<String>() {
if msg.starts_with("failed printing to stdout: ") && msg.ends_with("(os error 232)") {
// the error code is already going to be reported when the panic unwinds up the stack
let _ = early_error_no_abort(ErrorOutputType::default(), msg.as_str());
let _ = early_error_no_abort(ErrorOutputType::default(), msg.clone());
return;
}
};
Expand Down
32 changes: 14 additions & 18 deletions compiler/rustc_error_messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,7 @@ type FluentId = Cow<'static, str>;
#[rustc_diagnostic_item = "SubdiagnosticMessage"]
pub enum SubdiagnosticMessage {
/// Non-translatable diagnostic message.
// FIXME(davidtwco): can a `Cow<'static, str>` be used here?
Str(String),
Str(Cow<'static, str>),
/// Translatable message which has already been translated eagerly.
///
/// Some diagnostics have repeated subdiagnostics where the same interpolated variables would
Expand All @@ -275,8 +274,7 @@ pub enum SubdiagnosticMessage {
/// incorrect diagnostics. Eager translation results in translation for a subdiagnostic
/// happening immediately after the subdiagnostic derive's logic has been run. This variant
/// stores messages which have been translated eagerly.
// FIXME(#100717): can a `Cow<'static, str>` be used here?
Eager(String),
Eager(Cow<'static, str>),
/// Identifier of a Fluent message. Instances of this variant are generated by the
/// `Subdiagnostic` derive.
FluentIdentifier(FluentId),
Expand All @@ -290,17 +288,17 @@ pub enum SubdiagnosticMessage {

impl From<String> for SubdiagnosticMessage {
fn from(s: String) -> Self {
SubdiagnosticMessage::Str(s)
SubdiagnosticMessage::Str(Cow::Owned(s))
}
}
impl<'a> From<&'a str> for SubdiagnosticMessage {
fn from(s: &'a str) -> Self {
SubdiagnosticMessage::Str(s.to_string())
impl From<&'static str> for SubdiagnosticMessage {
fn from(s: &'static str) -> Self {
SubdiagnosticMessage::Str(Cow::Borrowed(s))
}
}
impl From<Cow<'static, str>> for SubdiagnosticMessage {
fn from(s: Cow<'static, str>) -> Self {
SubdiagnosticMessage::Str(s.to_string())
SubdiagnosticMessage::Str(s)
}
}

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

impl From<String> for DiagnosticMessage {
fn from(s: String) -> Self {
DiagnosticMessage::Str(s)
DiagnosticMessage::Str(Cow::Owned(s))
}
}
impl<'a> From<&'a str> for DiagnosticMessage {
fn from(s: &'a str) -> Self {
DiagnosticMessage::Str(s.to_string())
impl From<&'static str> for DiagnosticMessage {
fn from(s: &'static str) -> Self {
DiagnosticMessage::Str(Cow::Borrowed(s))
}
}
impl From<Cow<'static, str>> for DiagnosticMessage {
fn from(s: Cow<'static, str>) -> Self {
DiagnosticMessage::Str(s.to_string())
DiagnosticMessage::Str(s)
}
}

Expand Down
24 changes: 10 additions & 14 deletions compiler/rustc_errors/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,9 @@ impl Diagnostic {

/// Labels all the given spans with the provided label.
/// See [`Self::span_label()`] for more information.
pub fn span_labels(
&mut self,
spans: impl IntoIterator<Item = Span>,
label: impl AsRef<str>,
) -> &mut Self {
let label = label.as_ref();
pub fn span_labels(&mut self, spans: impl IntoIterator<Item = Span>, label: &str) -> &mut Self {
for span in spans {
self.span_label(span, label);
self.span_label(span, label.to_string());
}
self
}
Expand Down Expand Up @@ -394,17 +389,18 @@ impl Diagnostic {
expected: DiagnosticStyledString,
found: DiagnosticStyledString,
) -> &mut Self {
let mut msg: Vec<_> = vec![("required when trying to coerce from type `", Style::NoStyle)];
let mut msg: Vec<_> =
vec![(Cow::from("required when trying to coerce from type `"), Style::NoStyle)];
msg.extend(expected.0.iter().map(|x| match *x {
StringPart::Normal(ref s) => (s.as_str(), Style::NoStyle),
StringPart::Highlighted(ref s) => (s.as_str(), Style::Highlight),
StringPart::Normal(ref s) => (Cow::from(s.clone()), Style::NoStyle),
StringPart::Highlighted(ref s) => (Cow::from(s.clone()), Style::Highlight),
}));
msg.push(("` to type '", Style::NoStyle));
msg.push((Cow::from("` to type '"), Style::NoStyle));
msg.extend(found.0.iter().map(|x| match *x {
StringPart::Normal(ref s) => (s.as_str(), Style::NoStyle),
StringPart::Highlighted(ref s) => (s.as_str(), Style::Highlight),
StringPart::Normal(ref s) => (Cow::from(s.clone()), Style::NoStyle),
StringPart::Highlighted(ref s) => (Cow::from(s.clone()), Style::Highlight),
}));
msg.push(("`", Style::NoStyle));
msg.push((Cow::from("`"), Style::NoStyle));

// For now, just attach these as notes
self.highlighted_note(msg);
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_errors/src/diagnostic_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> {
}

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

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

forward!(pub fn note_expected_found(
Expand Down Expand Up @@ -781,8 +781,8 @@ impl Drop for DiagnosticBuilderInner<'_> {
if !panicking() {
handler.emit_diagnostic(&mut Diagnostic::new(
Level::Bug,
DiagnosticMessage::Str(
"the following error was constructed but not emitted".to_string(),
DiagnosticMessage::from(
"the following error was constructed but not emitted",
),
));
handler.emit_diagnostic(&mut self.diagnostic);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ pub trait Emitter: Translate {

children.push(SubDiagnostic {
level: Level::Note,
message: vec![(DiagnosticMessage::Str(msg), Style::NoStyle)],
message: vec![(DiagnosticMessage::from(msg), Style::NoStyle)],
span: MultiSpan::new(),
render_span: None,
});
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ impl Handler {
message: DiagnosticMessage,
args: impl Iterator<Item = DiagnosticArg<'a, 'static>>,
) -> SubdiagnosticMessage {
SubdiagnosticMessage::Eager(self.eagerly_translate_to_string(message, args))
SubdiagnosticMessage::Eager(Cow::from(self.eagerly_translate_to_string(message, args)))
}

/// Translate `message` eagerly with `args` to `String`.
Expand Down Expand Up @@ -1450,14 +1450,14 @@ impl HandlerInner {
self.emit_stashed_diagnostics();

let warnings = match self.deduplicated_warn_count {
0 => String::new(),
1 => "1 warning emitted".to_string(),
count => format!("{count} warnings emitted"),
0 => Cow::from(""),
1 => Cow::from("1 warning emitted"),
count => Cow::from(format!("{count} warnings emitted")),
};
let errors = match self.deduplicated_err_count {
0 => String::new(),
1 => "aborting due to previous error".to_string(),
count => format!("aborting due to {count} previous errors"),
0 => Cow::from(""),
1 => Cow::from("aborting due to previous error"),
count => Cow::from(format!("aborting due to {count} previous errors")),
};
if self.treat_err_as_bug() {
return;
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_expand/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@ impl<'a> ExtCtxt<'a> {
// Fixme: does this result in errors?
self.expansions.clear();
}
pub fn bug(&self, msg: &str) -> ! {
pub fn bug(&self, msg: &'static str) -> ! {
self.sess.parse_sess.span_diagnostic.bug(msg);
}
pub fn trace_macros(&self) -> bool {
Expand Down Expand Up @@ -1224,7 +1224,7 @@ pub fn resolve_path(
pub fn expr_to_spanned_string<'a>(
cx: &'a mut ExtCtxt<'_>,
expr: P<ast::Expr>,
err_msg: &str,
err_msg: &'static str,
) -> Result<(Symbol, ast::StrStyle, Span), Option<(DiagnosticBuilder<'a, ErrorGuaranteed>, bool)>> {
// Perform eager expansion on the expression.
// We want to be able to handle e.g., `concat!("foo", "bar")`.
Expand Down Expand Up @@ -1262,7 +1262,7 @@ pub fn expr_to_spanned_string<'a>(
pub fn expr_to_string(
cx: &mut ExtCtxt<'_>,
expr: P<ast::Expr>,
err_msg: &str,
err_msg: &'static str,
) -> Option<(Symbol, ast::StrStyle)> {
expr_to_spanned_string(cx, expr, err_msg)
.map_err(|err| {
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_expand/src/mbe/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl<'a, 'cx, 'matcher> Tracker<'matcher> for CollectTrackerAndEmitter<'a, 'cx,
}
Error(err_sp, msg) => {
let span = err_sp.substitute_dummy(self.root_span);
self.cx.struct_span_err(span, msg.as_str()).emit();
self.cx.struct_span_err(span, msg.clone()).emit();
self.result = Some(DummyResult::any(span));
}
ErrorReported(_) => self.result = Some(DummyResult::any(self.root_span)),
Expand Down Expand Up @@ -222,7 +222,7 @@ pub(super) fn emit_frag_parse_err(
{
let msg = &e.message[0];
e.message[0] = (
DiagnosticMessage::Str(format!(
DiagnosticMessage::from(format!(
"macro expansion ends with an incomplete expression: {}",
message.replace(", found `<eof>`", ""),
)),
Expand Down Expand Up @@ -313,9 +313,9 @@ pub(super) fn annotate_doc_comment(err: &mut Diagnostic, sm: &SourceMap, span: S

/// Generates an appropriate parsing failure message. For EOF, this is "unexpected end...". For
/// other tokens, this is "unexpected token...".
pub(super) fn parse_failure_msg(tok: &Token) -> String {
pub(super) fn parse_failure_msg(tok: &Token) -> Cow<'static, str> {
match tok.kind {
token::Eof => "unexpected end of macro invocation".to_string(),
_ => format!("no rules expected the token `{}`", pprust::token_to_string(tok),),
token::Eof => Cow::from("unexpected end of macro invocation"),
_ => Cow::from(format!("no rules expected the token `{}`", pprust::token_to_string(tok))),
}
}
7 changes: 4 additions & 3 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _;
use rustc_trait_selection::traits::{
self, ObligationCause, ObligationCauseCode, ObligationCtxt, Reveal,
};
use std::borrow::Cow;
use std::iter;

/// Checks that a method from an impl conforms to the signature of
Expand Down Expand Up @@ -684,7 +685,7 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>(
&cause,
hir.get_if_local(impl_m.def_id)
.and_then(|node| node.fn_decl())
.map(|decl| (decl.output.span(), "return type in trait".to_owned())),
.map(|decl| (decl.output.span(), Cow::from("return type in trait"))),
Some(infer::ValuePairs::Terms(ExpectedFound {
expected: trait_return_ty.into(),
found: impl_return_ty.into(),
Expand Down Expand Up @@ -963,7 +964,7 @@ fn report_trait_method_mismatch<'tcx>(
infcx.err_ctxt().note_type_err(
&mut diag,
&cause,
trait_err_span.map(|sp| (sp, "type in trait".to_owned())),
trait_err_span.map(|sp| (sp, Cow::from("type in trait"))),
Some(infer::ValuePairs::Sigs(ExpectedFound { expected: trait_sig, found: impl_sig })),
terr,
false,
Expand Down Expand Up @@ -1731,7 +1732,7 @@ pub(super) fn compare_impl_const_raw(
infcx.err_ctxt().note_type_err(
&mut diag,
&cause,
trait_c_span.map(|span| (span, "type in trait".to_owned())),
trait_c_span.map(|span| (span, Cow::from("type in trait"))),
Some(infer::ValuePairs::Terms(ExpectedFound {
expected: trait_ty.into(),
found: impl_ty.into(),
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_hir_analysis/src/check/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,11 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: DefId) -> hir
if has_safe_attr != is_in_list {
tcx.sess.struct_span_err(
tcx.def_span(intrinsic_id),
DiagnosticMessage::Str(format!(
"intrinsic safety mismatch between list of intrinsics within the compiler and core library intrinsics for intrinsic `{}`",
tcx.item_name(intrinsic_id)
))).emit();
DiagnosticMessage::from(format!(
"intrinsic safety mismatch between list of intrinsics within the compiler and core library intrinsics for intrinsic `{}`",
tcx.item_name(intrinsic_id)
)
)).emit();
}

is_in_list
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/generator_interior/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl<'a, 'tcx> InteriorVisitor<'a, 'tcx> {

self.fcx
.need_type_info_err_in_generator(self.kind, span, unresolved_term)
.span_note(yield_data.span, &*note)
.span_note(yield_data.span, note)
.emit();
}
} else {
Expand Down Expand Up @@ -686,7 +686,7 @@ fn check_must_not_suspend_def(
// Add optional reason note
if let Some(note) = attr.value_str() {
// FIXME(guswynn): consider formatting this better
lint.span_note(data.source_span, note.as_str());
lint.span_note(data.source_span, note.to_string());
}

// Add some quick suggestions on what to do
Expand Down
Loading

0 comments on commit 781111e

Please sign in to comment.