Auto merge of rust-lang#110579 - nnethercote:restrict-From-for-Diagnostics, 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`

Author: bors

compiler/rustc_ast_lowering/src/path.rs

@@ -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()

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -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)
                 ),
             }
         }
@@ -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()
                         ),

compiler/rustc_attr/src/builtin.rs

@@ -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();
     }
 }
 

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -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,

compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs

@@ -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,
             );

compiler/rustc_borrowck/src/diagnostics/mod.rs

@@ -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}`"))

compiler/rustc_borrowck/src/diagnostics/move_errors.rs

@@ -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);
         }
     }
 

compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs

@@ -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",
                 ));
             }

compiler/rustc_borrowck/src/diagnostics/outlives_suggestion.rs

@@ -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`"));
                     }
                 }
             }

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

@@ -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,

compiler/rustc_borrowck/src/diagnostics/region_name.rs

@@ -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:?}"),
                     );
                 }
             }

compiler/rustc_borrowck/src/lib.rs

@@ -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);
             }

compiler/rustc_borrowck/src/nll.rs

@@ -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);

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -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());
         }
     }

compiler/rustc_borrowck/src/type_check/free_region_relations.rs

@@ -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,

compiler/rustc_borrowck/src/type_check/input_output.rs

@@ -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,

compiler/rustc_borrowck/src/type_check/mod.rs

@@ -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);
             }