From 431cce15401f9dc99ac9ac08551d803130896b83 Mon Sep 17 00:00:00 2001
From: Nicholas Nethercote <n.nethercote@gmail.com>
Date: Thu, 20 Apr 2023 13:26:58 +1000
Subject: [PATCH] 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.
---
clippy_lints/src/future_not_send.rs | 2 +-
clippy_lints/src/lib.rs | 2 +-
clippy_lints/src/methods/str_splitn.rs | 4 ++--
clippy_lints/src/non_send_fields_in_send_ty.rs | 6 +++---
clippy_utils/src/attrs.rs | 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/clippy_lints/src/future_not_send.rs b/clippy_lints/src/future_not_send.rs
index ff838c2d56e..d1314795f58 100644
--- a/clippy_lints/src/future_not_send.rs
+++ b/clippy_lints/src/future_not_send.rs
@@ -96,7 +96,7 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
if let PredicateKind::Clause(Clause::Trait(trait_pred)) =
obligation.predicate.kind().skip_binder()
{
- db.note(&format!(
+ db.note(format!(
"`{}` doesn't implement `{}`",
trait_pred.self_ty(),
trait_pred.trait_ref.print_only_trait_path(),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 573ffe349ec..9e65f9ecd16 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -353,7 +353,7 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, sess: &Se
pub fn read_conf(sess: &Session, path: &io::Result<(Option<PathBuf>, Vec<String>)>) -> Conf {
if let Ok((_, warnings)) = path {
for warning in warnings {
- sess.warn(warning);
+ sess.warn(warning.clone());
}
}
let file_name = match path {
diff --git a/clippy_lints/src/methods/str_splitn.rs b/clippy_lints/src/methods/str_splitn.rs
index d00708e828e..91f7ce1dbe5 100644
--- a/clippy_lints/src/methods/str_splitn.rs
+++ b/clippy_lints/src/methods/str_splitn.rs
@@ -175,13 +175,13 @@ fn check_manual_split_once_indirect(
let remove_msg = format!("remove the `{iter_ident}` usages");
diag.span_suggestion(
first.span,
- &remove_msg,
+ remove_msg.clone(),
"",
app,
);
diag.span_suggestion(
second.span,
- &remove_msg,
+ remove_msg,
"",
app,
);
diff --git a/clippy_lints/src/non_send_fields_in_send_ty.rs b/clippy_lints/src/non_send_fields_in_send_ty.rs
index 839c3a3815c..7eaa7db78a4 100644
--- a/clippy_lints/src/non_send_fields_in_send_ty.rs
+++ b/clippy_lints/src/non_send_fields_in_send_ty.rs
@@ -131,13 +131,13 @@ impl<'tcx> LateLintPass<'tcx> for NonSendFieldInSendTy {
for field in non_send_fields {
diag.span_note(
field.def.span,
- &format!("it is not safe to send field `{}` to another thread", field.def.ident.name),
+ format!("it is not safe to send field `{}` to another thread", field.def.ident.name),
);
match field.generic_params.len() {
0 => diag.help("use a thread-safe type that implements `Send`"),
- 1 if is_ty_param(field.ty) => diag.help(&format!("add `{}: Send` bound in `Send` impl", field.ty)),
- _ => diag.help(&format!(
+ 1 if is_ty_param(field.ty) => diag.help(format!("add `{}: Send` bound in `Send` impl", field.ty)),
+ _ => diag.help(format!(
"add bounds on type parameter{} `{}` that satisfy `{}: Send`",
if field.generic_params.len() > 1 { "s" } else { "" },
field.generic_params_string(),
diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs
index b4ad42a5027..49cb9718ef6 100644
--- a/clippy_utils/src/attrs.rs
+++ b/clippy_utils/src/attrs.rs
@@ -133,7 +133,7 @@ pub fn get_unique_attr<'a>(
let mut unique_attr: Option<&ast::Attribute> = None;
for attr in get_attr(sess, attrs, name) {
if let Some(duplicate) = unique_attr {
- sess.struct_span_err(attr.span, &format!("`{name}` is defined multiple times"))
+ sess.struct_span_err(attr.span, format!("`{name}` is defined multiple times"))
.span_note(duplicate.span, "first definition found here")
.emit();
} else {