Skip to content

Commit

Permalink
Auto merge of rust-lang#110579 - nnethercote:restrict-From-for-Diagno…
Browse files Browse the repository at this point in the history
…stics, 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`
  • Loading branch information
bors committed May 3, 2023
2 parents b4571be + 6b62f37 commit 71af5c4
Show file tree
Hide file tree
Showing 177 changed files with 791 additions and 787 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

self.diagnostic().span_bug(
p.span,
&format!(
format!(
"lower_qpath: no final extension segment in {}..{}",
proj_start,
p.segments.len()
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl<'a> PostExpansionVisitor<'a> {
&self,
const_extern_fn,
span,
&format!("`{}` as a `const fn` ABI is unstable", abi)
format!("`{}` as a `const fn` ABI is unstable", abi)
),
}
}
Expand All @@ -104,7 +104,7 @@ impl<'a> PostExpansionVisitor<'a> {
if self.sess.opts.pretty.map_or(true, |ppm| ppm.needs_hir()) {
self.sess.parse_sess.span_diagnostic.delay_span_bug(
span,
&format!(
format!(
"unrecognized ABI not caught in lowering: {}",
symbol_unescaped.as_str()
),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ fn gate_cfg(gated_cfg: &GatedCfg, cfg_span: Span, sess: &ParseSess, features: &F
let (cfg, feature, has_feature) = gated_cfg;
if !has_feature(features) && !cfg_span.allows_unstable(*feature) {
let explain = format!("`cfg({cfg})` is experimental and subject to change");
feature_err(sess, *feature, cfg_span, &explain).emit();
feature_err(sess, *feature, cfg_span, explain).emit();
}
}

Expand Down
24 changes: 12 additions & 12 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -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}`"))
Expand Down Expand Up @@ -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(),
));
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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,
));
Expand Down Expand Up @@ -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" }
),
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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!(
Expand Down Expand Up @@ -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
));
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
{
err.span_label(
pat.span,
&format!("binding `{ident}` declared here"),
format!("binding `{ident}` declared here"),
);
}
}
Expand Down Expand Up @@ -323,7 +323,7 @@ impl<'tcx> BorrowExplanation<'tcx> {

err.span_suggestion_verbose(
span.shrink_to_hi(),
&msg,
msg,
format!(" + {suggestable_name}"),
Applicability::Unspecified,
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}`"))
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
18 changes: 9 additions & 9 deletions compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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}`"
),
);
Expand Down Expand Up @@ -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}`",
));
Expand Down Expand Up @@ -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,
),
Expand Down Expand Up @@ -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()),
Expand Down Expand Up @@ -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",
));
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}`",
));
}
Expand Down Expand Up @@ -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(" + "))
Expand All @@ -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`"));
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}"),
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/nll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
));
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/type_check/input_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Loading

0 comments on commit 71af5c4

Please sign in to comment.