bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Restrict From<S> for {D,Subd}iagnosticMessage.

Browse source 
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.
This commit is contained in:
Nicholas Nethercote 2023-04-20 13:26:58 +10:00
parent a368898de7
commit 6b62f37402
177 changed files with 791 additions and 787 deletions
Download patch file Download diff file Expand all files Collapse all files

24
compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file

@ -158,7 +158,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
} else if reinits > 1 {
err.span_note(
MultiSpan::from_spans(reinit_spans),
&if reinits <= 3 {
if reinits <= 3 {
format!("these {reinits} reinitializations might get skipped")
} else {
format!(
@ -253,7 +253,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// We have a `&mut` ref, we need to reborrow on each iteration (#62112).
err.span_suggestion_verbose(
span.shrink_to_lo(),
&format!(
format!(
"consider creating a fresh reborrow of {} here",
self.describe_place(moved_place)
.map(|n| format!("`{n}`"))
@ -304,7 +304,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
..
} = use_spans
{
err.note(&format!(
err.note(format!(
"{} occurs due to deref coercion to `{deref_target_ty}`",
desired_action.as_noun(),
));
@ -586,7 +586,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// _ => {} // We don't want to point to this.
// };
// ```
err.span_label(sp, &label);
err.span_label(sp, label);
shown = true;
}
}
@ -1139,7 +1139,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
if union_type_name != "" {
err.note(&format!(
err.note(format!(
"{} is a field of the union `{}`, so it overlaps the field {}",
msg_place, union_type_name, msg_borrow,
));
@ -1238,14 +1238,14 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
err.span_help(
inner_call_span,
&format!(
format!(
"try adding a local storing this{}...",
if use_span.is_some() { "" } else { " argument" }
),
);
err.span_help(
outer_call_span,
&format!(
format!(
"...and then using that local {}",
if use_span.is_some() { "here" } else { "as the argument to this call" }
),
@ -2281,7 +2281,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
err.span_suggestion_verbose(
sugg_span,
&format!(
format!(
"to force the {} to take ownership of {} (and any \
other referenced variables), use the `move` keyword",
kind, captured_var
@ -2293,7 +2293,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
match category {
ConstraintCategory::Return(_) | ConstraintCategory::OpaqueType => {
let msg = format!("{} is returned here", kind);
err.span_note(constraint_span, &msg);
err.span_note(constraint_span, msg);
}
ConstraintCategory::CallArgument(_) => {
fr_name.highlight_region_name(&mut err);
@ -2304,7 +2304,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
} else {
let msg = format!("{scope} requires argument type to outlive `{fr_name}`");
err.span_note(constraint_span, &msg);
err.span_note(constraint_span, msg);
}
}
_ => bug!(
@ -2626,7 +2626,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
});
if let Some(Ok(instance)) = deref_target {
let deref_target_ty = instance.ty(tcx, self.param_env);
err.note(&format!(
err.note(format!(
"borrow occurs due to deref coercion to `{}`",
deref_target_ty
));
@ -3180,7 +3180,7 @@ impl<'tcx> AnnotatedBorrowFnSignature<'tcx> {
diag.span_label(*return_span, format!("also has lifetime `{}`", region_name,));
diag.help(&format!(
diag.help(format!(
"use data from the highlighted arguments which match the `{}` lifetime of \
the return type",
region_name,

4
compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
View file

@ -90,7 +90,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
{
err.span_label(
pat.span,
&format!("binding `{ident}` declared here"),
format!("binding `{ident}` declared here"),
);
}
}
@ -323,7 +323,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
err.span_suggestion_verbose(
span.shrink_to_hi(),
&msg,
msg,
format!(" + {suggestable_name}"),
Applicability::Unspecified,
);

2
compiler/rustc_borrowck/src/diagnostics/mod.rs
View file

@ -1073,7 +1073,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if !is_loop_move {
err.span_suggestion_verbose(
move_span.shrink_to_lo(),
&format!(
format!(
"consider creating a fresh reborrow of {} here",
self.describe_place(moved_place.as_ref())
.map(|n| format!("`{n}`"))

2
compiler/rustc_borrowck/src/diagnostics/move_errors.rs
View file

@ -533,7 +533,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
suggestions.sort_unstable_by_key(|&(span, _, _)| span);
suggestions.dedup_by_key(|&mut (span, _, _)| span);
for (span, msg, suggestion) in suggestions {
err.span_suggestion_verbose(span, &msg, suggestion, Applicability::MachineApplicable);
err.span_suggestion_verbose(span, msg, suggestion, Applicability::MachineApplicable);
}
}

18
compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
View file

@ -573,7 +573,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
if !is_trait_sig {
err.span_suggestion_verbose(
err_help_span,
&format!(
format!(
"consider changing this to be a mutable {pointer_desc}"
),
suggested_code,
@ -582,7 +582,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
} else if let Some(x) = local_trait {
err.span_suggestion_verbose(
x,
&format!(
format!(
"consider changing that to be a mutable {pointer_desc}"
),
suggested_code,
@ -636,14 +636,14 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
};
err.span_suggestion_verbose(
span,
&format!("consider {changing} this binding's type"),
format!("consider {changing} this binding's type"),
sugg,
Applicability::HasPlaceholders,
);
} else {
err.span_label(
err_label_span,
&format!(
format!(
"consider changing this binding's type to be: `{message}`"
),
);
@ -679,13 +679,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
match opt_source {
Some(BorrowedContentSource::OverloadedDeref(ty)) => {
err.help(&format!(
err.help(format!(
"trait `DerefMut` is required to modify through a dereference, \
but it is not implemented for `{ty}`",
));
}
Some(BorrowedContentSource::OverloadedIndex(ty)) => {
err.help(&format!(
err.help(format!(
"trait `IndexMut` is required to modify indexed content, \
but it is not implemented for `{ty}`",
));
@ -736,7 +736,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
// val[index] = rv;
// ---------- place
self.err.multipart_suggestions(
&format!(
format!(
"to modify a `{}`, use `.get_mut()`, `.insert()` or the entry API",
self.ty,
),
@ -788,7 +788,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
{
// val[index].path(args..);
self.err.multipart_suggestion(
&format!("to modify a `{}` use `.get_mut()`", self.ty),
format!("to modify a `{}` use `.get_mut()`", self.ty),
vec![
(
val.span.shrink_to_hi().with_hi(index.span.lo()),
@ -822,7 +822,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let mut v = V { assign_span, err, ty, suggested: false };
v.visit_body(body);
if !v.suggested {
err.help(&format!(
err.help(format!(
"to modify a `{ty}`, use `.get_mut()`, `.insert()` or the entry API",
));
}

10
compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs
View file

@ -171,7 +171,7 @@ impl OutlivesSuggestionBuilder {
if let (Some(fr_name), Some(outlived_fr_name)) = (fr_name, outlived_fr_name)
&& !matches!(outlived_fr_name.source, RegionNameSource::Static)
{
diag.help(&format!(
diag.help(format!(
"consider adding the following bound: `{fr_name}: {outlived_fr_name}`",
));
}
@ -207,7 +207,7 @@ impl OutlivesSuggestionBuilder {
// If there is exactly one suggestable constraints, then just suggest it. Otherwise, emit a
// list of diagnostics.
let mut diag = if suggested.len() == 1 {
mbcx.infcx.tcx.sess.diagnostic().struct_help(&match suggested.last().unwrap() {
mbcx.infcx.tcx.sess.diagnostic().struct_help(match suggested.last().unwrap() {
SuggestedConstraint::Outlives(a, bs) => {
let bs: SmallVec<[String; 2]> = bs.iter().map(|r| r.to_string()).collect();
format!("add bound `{a}: {}`", bs.join(" + "))
@ -232,15 +232,15 @@ impl OutlivesSuggestionBuilder {
match constraint {
SuggestedConstraint::Outlives(a, bs) => {
let bs: SmallVec<[String; 2]> = bs.iter().map(|r| r.to_string()).collect();
diag.help(&format!("add bound `{a}: {}`", bs.join(" + ")));
diag.help(format!("add bound `{a}: {}`", bs.join(" + ")));
}
SuggestedConstraint::Equal(a, b) => {
diag.help(&format!(
diag.help(format!(
"`{a}` and `{b}` must be the same: replace one with the other",
));
}
SuggestedConstraint::Static(a) => {
diag.help(&format!("replace `{a}` with `'static`"));
diag.help(format!("replace `{a}` with `'static`"));
}
}
}

6
compiler/rustc_borrowck/src/diagnostics/region_errors.rs
View file

@ -533,8 +533,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
_ => panic!("Unexpected type {ty:?}"),
};
diag.note(&format!("requirement occurs because of {desc}",));
diag.note(&note);
diag.note(format!("requirement occurs because of {desc}",));
diag.note(note);
diag.help("see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance");
}
}
@ -863,7 +863,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}
spans_suggs.push((alias_span.shrink_to_hi(), "<'a>".to_string()));
diag.multipart_suggestion_verbose(
&format!(
format!(
"to declare that the trait object {captures}, you can add a lifetime parameter `'a` in the type alias"
),
spans_suggs,

2
compiler/rustc_borrowck/src/diagnostics/region_name.rs
View file

@ -622,7 +622,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
// programs, so we need to use delay_span_bug here. See #82126.
self.infcx.tcx.sess.delay_span_bug(
hir_arg.span(),
&format!("unmatched subst and hir arg: found {kind:?} vs {hir_arg:?}"),
format!("unmatched subst and hir arg: found {kind:?} vs {hir_arg:?}"),
);
}
}

4
compiler/rustc_borrowck/src/lib.rs
View file

@ -2022,7 +2022,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// been emitted (#52262).
self.infcx.tcx.sess.delay_span_bug(
span,
&format!(
format!(
"Accessing `{:?}` with the kind `{:?}` shouldn't be possible",
place, kind,
),
@ -2383,7 +2383,7 @@ mod error {
}
for (_, (mut diag, count)) in std::mem::take(&mut self.errors.buffered_mut_errors) {
if count > 10 {
diag.note(&format!("...and {} other attempted mutable borrows", count - 10));
diag.note(format!("...and {} other attempted mutable borrows", count - 10));
}
diag.buffer(&mut self.errors.buffered);
}

4
compiler/rustc_borrowck/src/nll.rs
View file

@ -399,7 +399,7 @@ pub(super) fn dump_annotation<'tcx>(
regioncx.annotate(tcx, &mut err);
err.note(&format!(
err.note(format!(
"number of external vids: {}",
closure_region_requirements.num_external_vids
));
@ -421,7 +421,7 @@ pub(super) fn dump_annotation<'tcx>(
};
if !opaque_type_values.is_empty() {
err.note(&format!("Inferred opaque type values:\n{:#?}", opaque_type_values));
err.note(format!("Inferred opaque type values:\n{:#?}", opaque_type_values));
}
errors.buffer_non_error_diag(err);

2
compiler/rustc_borrowck/src/region_infer/opaque_types.rs
View file

@ -399,7 +399,7 @@ fn check_opaque_type_parameter_valid(
return Err(tcx
.sess
.struct_span_err(span, "non-defining opaque type use in defining scope")
.span_note(spans, &format!("{} used multiple times", descr))
.span_note(spans, format!("{} used multiple times", descr))
.emit());
}
}

2
compiler/rustc_borrowck/src/type_check/free_region_relations.rs
View file

@ -249,7 +249,7 @@ impl<'tcx> UniversalRegionRelationsBuilder<'_, 'tcx> {
.infcx
.tcx
.sess
.delay_span_bug(span, &format!("failed to normalize {:?}", ty));
.delay_span_bug(span, format!("failed to normalize {:?}", ty));
TypeOpOutput {
output: self.infcx.tcx.ty_error(guar),
constraints: None,

2
compiler/rustc_borrowck/src/type_check/input_output.rs
View file

@ -106,7 +106,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
if body.yield_ty().is_some() != universal_regions.yield_ty.is_some() {
self.tcx().sess.delay_span_bug(
body.span,
&format!(
format!(
"Expected body to have yield_ty ({:?}) iff we have a UR yield_ty ({:?})",
body.yield_ty(),
universal_regions.yield_ty,

2
compiler/rustc_borrowck/src/type_check/mod.rs
View file

@ -236,7 +236,7 @@ pub(crate) fn type_check<'mir, 'tcx>(
if hidden_type.has_non_region_infer() {
let reported = infcx.tcx.sess.delay_span_bug(
decl.hidden_type.span,
&format!("could not resolve {:#?}", hidden_type.ty.kind()),
format!("could not resolve {:#?}", hidden_type.ty.kind()),
);
hidden_type.ty = infcx.tcx.ty_error(reported);
}

14
compiler/rustc_borrowck/src/universal_regions.rs
View file

@ -335,7 +335,7 @@ impl<'tcx> UniversalRegions<'tcx> {
pub(crate) fn annotate(&self, tcx: TyCtxt<'tcx>, err: &mut Diagnostic) {
match self.defining_ty {
DefiningTy::Closure(def_id, substs) => {
err.note(&format!(
err.note(format!(
"defining type: {} with closure substs {:#?}",
tcx.def_path_str_with_substs(def_id, substs),
&substs[tcx.generics_of(def_id).parent_count..],
@ -347,11 +347,11 @@ impl<'tcx> UniversalRegions<'tcx> {
// and other things that are not stable across tests!
// So we just include the region-vid. Annoying.
for_each_late_bound_region_in_recursive_scope(tcx, def_id.expect_local(), |r| {
err.note(&format!("late-bound region is {:?}", self.to_region_vid(r)));
err.note(format!("late-bound region is {:?}", self.to_region_vid(r)));
});
}
DefiningTy::Generator(def_id, substs, _) => {
err.note(&format!(
err.note(format!(
"defining type: {} with generator substs {:#?}",
tcx.def_path_str_with_substs(def_id, substs),
&substs[tcx.generics_of(def_id).parent_count..],
@ -361,23 +361,23 @@ impl<'tcx> UniversalRegions<'tcx> {
// `r` but doing so is not stable across architectures
// and so forth.
for_each_late_bound_region_in_recursive_scope(tcx, def_id.expect_local(), |r| {
err.note(&format!("late-bound region is {:?}", self.to_region_vid(r)));
err.note(format!("late-bound region is {:?}", self.to_region_vid(r)));
});
}
DefiningTy::FnDef(def_id, substs) => {
err.note(&format!(
err.note(format!(
"defining type: {}",
tcx.def_path_str_with_substs(def_id, substs),
));
}
DefiningTy::Const(def_id, substs) => {
err.note(&format!(
err.note(format!(
"defining constant type: {}",
tcx.def_path_str_with_substs(def_id, substs),
));
}
DefiningTy::InlineConst(def_id, substs) => {
err.note(&format!(
err.note(format!(
"defining inline constant type: {}",
tcx.def_path_str_with_substs(def_id, substs),
));