Auto merge of #94131 - Mark-Simulacrum:fmt-string, r=oli-obk

bors · bors · commit 4b043faba34c · 2022-02-24T17:18:07.000Z
Always format to internal String in FmtPrinter

This avoids monomorphizing for different parameters, decreasing generic code
instantiated downstream from rustc_middle -- locally seeing 7% unoptimized LLVM IR
line wins on rustc_borrowck, for example.

We likely can't/shouldn't get rid of the Result-ness on most functions, though some
further cleanup avoiding fmt::Error where we now know it won't occur may be possible,
though somewhat painful -- fmt::Write is a pretty annoying API to work with in practice
when you're trying to use it infallibly.
diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs
@@ -488,8 +488,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
     /// Return the name of the provided `Ty` (that must be a reference) with a synthesized lifetime
     /// name where required.
     pub(super) fn get_name_for_ty(&self, ty: Ty<'tcx>, counter: usize) -> String {
-        let mut s = String::new();
-        let mut printer = ty::print::FmtPrinter::new(self.infcx.tcx, &mut s, Namespace::TypeNS);
+        let mut printer = ty::print::FmtPrinter::new(self.infcx.tcx, Namespace::TypeNS);
 
         // We need to add synthesized lifetimes where appropriate. We do
         // this by hooking into the pretty printer and telling it to label the
@@ -504,15 +503,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             }
         }
 
-        let _ = ty.print(printer);
-        s
+        ty.print(printer).unwrap().into_buffer()
     }
 
     /// Returns the name of the provided `Ty` (that must be a reference)'s region with a
     /// synthesized lifetime name where required.
     pub(super) fn get_region_name_for_ty(&self, ty: Ty<'tcx>, counter: usize) -> String {
-        let mut s = String::new();
-        let mut printer = ty::print::FmtPrinter::new(self.infcx.tcx, &mut s, Namespace::TypeNS);
+        let mut printer = ty::print::FmtPrinter::new(self.infcx.tcx, Namespace::TypeNS);
 
         let region = if let ty::Ref(region, ..) = ty.kind() {
             match **region {
@@ -527,8 +524,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
             bug!("ty for annotation of borrow region is not a reference");
         };
 
-        let _ = region.print(printer);
-        s
+        region.print(printer).unwrap().into_buffer()
     }
 }
 
diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs
@@ -109,11 +109,11 @@ rustc_data_structures::static_assert_size!(ImmTy<'_>, 72);
 impl<Tag: Provenance> std::fmt::Display for ImmTy<'_, Tag> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         /// Helper function for printing a scalar to a FmtPrinter
-        fn p<'a, 'tcx, F: std::fmt::Write, Tag: Provenance>(
-            cx: FmtPrinter<'a, 'tcx, F>,
+        fn p<'a, 'tcx, Tag: Provenance>(
+            cx: FmtPrinter<'a, 'tcx>,
             s: ScalarMaybeUninit<Tag>,
             ty: Ty<'tcx>,
-        ) -> Result<FmtPrinter<'a, 'tcx, F>, std::fmt::Error> {
+        ) -> Result<FmtPrinter<'a, 'tcx>, std::fmt::Error> {
             match s {
                 ScalarMaybeUninit::Scalar(Scalar::Int(int)) => {
                     cx.pretty_print_const_scalar_int(int, ty, true)
@@ -138,8 +138,8 @@ impl<Tag: Provenance> std::fmt::Display for ImmTy<'_, Tag> {
             match self.imm {
                 Immediate::Scalar(s) => {
                     if let Some(ty) = tcx.lift(self.layout.ty) {
-                        let cx = FmtPrinter::new(tcx, f, Namespace::ValueNS);
-                        p(cx, s, ty)?;
+                        let cx = FmtPrinter::new(tcx, Namespace::ValueNS);
+                        f.write_str(&p(cx, s, ty)?.into_buffer())?;
                         return Ok(());
                     }
                     write!(f, "{:x}: {}", s, self.layout.ty)
diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs
@@ -988,8 +988,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     ) -> (DiagnosticStyledString, DiagnosticStyledString) {
         let get_lifetimes = |sig| {
             use rustc_hir::def::Namespace;
-            let mut s = String::new();
-            let (_, sig, reg) = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS)
+            let (_, sig, reg) = ty::print::FmtPrinter::new(self.tcx, Namespace::TypeNS)
                 .name_all_regions(sig)
                 .unwrap();
             let lts: Vec<String> = reg.into_iter().map(|(_, kind)| kind.to_string()).collect();
diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
@@ -397,14 +397,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                     }
                 }
 
-                let mut s = String::new();
-                let mut printer = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS);
+                let mut printer = ty::print::FmtPrinter::new(self.tcx, Namespace::TypeNS);
                 if let Some(highlight) = highlight {
                     printer.region_highlight_mode = highlight;
                 }
-                let _ = ty.print(printer);
+                let name = ty.print(printer).unwrap().into_buffer();
                 InferenceDiagnosticsData {
-                    name: s,
+                    name,
                     span: None,
                     kind: UnderspecifiedArgKind::Type { prefix: ty.prefix_string(self.tcx) },
                     parent: None,
@@ -433,15 +432,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
                         }
 
                         debug_assert!(!origin.span.is_dummy());
-                        let mut s = String::new();
-                        let mut printer =
-                            ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::ValueNS);
+                        let mut printer = ty::print::FmtPrinter::new(self.tcx, Namespace::ValueNS);
                         if let Some(highlight) = highlight {
                             printer.region_highlight_mode = highlight;
                         }
-                        let _ = ct.print(printer);
+                        let name = ct.print(printer).unwrap().into_buffer();
                         InferenceDiagnosticsData {
-                            name: s,
+                            name,
                             span: Some(origin.span),
                             kind: UnderspecifiedArgKind::Const { is_parameter: false },
                             parent: None,
@@ -497,8 +494,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
 
         let mut local_visitor = FindHirNodeVisitor::new(&self, arg, span);
         let ty_to_string = |ty: Ty<'tcx>| -> String {
-            let mut s = String::new();
-            let mut printer = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS);
+            let mut printer = ty::print::FmtPrinter::new(self.tcx, Namespace::TypeNS);
             let ty_getter = move |ty_vid| {
                 if let TypeVariableOriginKind::TypeParameterDefinition(name, _) =
                     self.inner.borrow_mut().type_variables().var_origin(ty_vid).kind
@@ -525,14 +521,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             };
             printer.const_infer_name_resolver = Some(Box::new(const_getter));
 
-            let _ = if let ty::FnDef(..) = ty.kind() {
+            if let ty::FnDef(..) = ty.kind() {
                 // We don't want the regular output for `fn`s because it includes its path in
                 // invalid pseudo-syntax, we want the `fn`-pointer output instead.
-                ty.fn_sig(self.tcx).print(printer)
+                ty.fn_sig(self.tcx).print(printer).unwrap().into_buffer()
             } else {
-                ty.print(printer)
-            };
-            s
+                ty.print(printer).unwrap().into_buffer()
+            }
         };
 
         if let Some(body_id) = body_id {
diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs
@@ -335,18 +335,19 @@ impl<'tcx> NiceRegionError<'_, 'tcx> {
 
         impl<'tcx, T> fmt::Display for Highlighted<'tcx, T>
         where
-            T: for<'a, 'b, 'c> Print<
+            T: for<'a> Print<
                 'tcx,
-                FmtPrinter<'a, 'tcx, &'b mut fmt::Formatter<'c>>,
+                FmtPrinter<'a, 'tcx>,
                 Error = fmt::Error,
+                Output = FmtPrinter<'a, 'tcx>,
             >,
         {
             fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-                let mut printer = ty::print::FmtPrinter::new(self.tcx, f, Namespace::TypeNS);
+                let mut printer = ty::print::FmtPrinter::new(self.tcx, Namespace::TypeNS);
                 printer.region_highlight_mode = self.highlight;
 
-                self.value.print(printer)?;
-                Ok(())
+                let s = self.value.print(printer)?.into_buffer();
+                f.write_str(&s)
             }
         }
 
diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs
@@ -2421,11 +2421,11 @@ impl<'tcx> Debug for Rvalue<'tcx> {
 
                     AggregateKind::Adt(adt_did, variant, substs, _user_ty, _) => {
                         ty::tls::with(|tcx| {
-                            let mut name = String::new();
                             let variant_def = &tcx.adt_def(adt_did).variants[variant];
                             let substs = tcx.lift(substs).expect("could not lift for printing");
-                            FmtPrinter::new(tcx, &mut name, Namespace::ValueNS)
-                                .print_def_path(variant_def.def_id, substs)?;
+                            let name = FmtPrinter::new(tcx, Namespace::ValueNS)
+                                .print_def_path(variant_def.def_id, substs)?
+                                .into_buffer();
 
                             match variant_def.ctor_kind {
                                 CtorKind::Const => fmt.write_str(&name),
@@ -2847,9 +2847,10 @@ fn pretty_print_const<'tcx>(
     use crate::ty::print::PrettyPrinter;
     ty::tls::with(|tcx| {
         let literal = tcx.lift(c).unwrap();
-        let mut cx = FmtPrinter::new(tcx, fmt, Namespace::ValueNS);
+        let mut cx = FmtPrinter::new(tcx, Namespace::ValueNS);
         cx.print_alloc_ids = true;
-        cx.pretty_print_const(literal, print_types)?;
+        let cx = cx.pretty_print_const(literal, print_types)?;
+        fmt.write_str(&cx.into_buffer())?;
         Ok(())
     })
 }
@@ -2864,9 +2865,10 @@ fn pretty_print_const_value<'tcx>(
     ty::tls::with(|tcx| {
         let val = tcx.lift(val).unwrap();
         let ty = tcx.lift(ty).unwrap();
-        let mut cx = FmtPrinter::new(tcx, fmt, Namespace::ValueNS);
+        let mut cx = FmtPrinter::new(tcx, Namespace::ValueNS);
         cx.print_alloc_ids = true;
-        cx.pretty_print_const_value(val, ty, print_types)?;
+        let cx = cx.pretty_print_const_value(val, ty, print_types)?;
+        fmt.write_str(&cx.into_buffer())?;
         Ok(())
     })
 }
diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs
@@ -983,10 +983,9 @@ fn foo(&self) -> Self::T { String::new() }
     }
 
     fn format_generic_args(self, args: &[ty::GenericArg<'tcx>]) -> String {
-        let mut item_args = String::new();
-        FmtPrinter::new(self, &mut item_args, hir::def::Namespace::TypeNS)
+        FmtPrinter::new(self, hir::def::Namespace::TypeNS)
             .path_generic_args(Ok, args)
-            .expect("could not write to `String`.");
-        item_args
+            .expect("could not write to `String`.")
+            .into_buffer()
     }
 }
diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs
@@ -279,9 +279,10 @@ impl<'tcx> fmt::Display for Instance<'tcx> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         ty::tls::with(|tcx| {
             let substs = tcx.lift(self.substs).expect("could not lift for printing");
-            FmtPrinter::new(tcx, &mut *f, Namespace::ValueNS)
-                .print_def_path(self.def_id(), substs)?;
-            Ok(())
+            let s = FmtPrinter::new(tcx, Namespace::ValueNS)
+                .print_def_path(self.def_id(), substs)?
+                .into_buffer();
+            f.write_str(&s)
         })?;
 
         match self.def {
diff --git a/compiler/rustc_middle/src/ty/print/pretty.rs b/compiler/rustc_middle/src/ty/print/pretty.rs
@@ -1543,11 +1543,11 @@ pub trait PrettyPrinter<'tcx>:
 }
 
 // HACK(eddyb) boxed to avoid moving around a large struct by-value.
-pub struct FmtPrinter<'a, 'tcx, F>(Box<FmtPrinterData<'a, 'tcx, F>>);
+pub struct FmtPrinter<'a, 'tcx>(Box<FmtPrinterData<'a, 'tcx>>);
 
-pub struct FmtPrinterData<'a, 'tcx, F> {
+pub struct FmtPrinterData<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
-    fmt: F,
+    fmt: String,
 
     empty_path: bool,
     in_value: bool,
@@ -1564,24 +1564,26 @@ pub struct FmtPrinterData<'a, 'tcx, F> {
     pub const_infer_name_resolver: Option<Box<dyn Fn(ty::ConstVid<'tcx>) -> Option<String> + 'a>>,
 }
 
-impl<'a, 'tcx, F> Deref for FmtPrinter<'a, 'tcx, F> {
-    type Target = FmtPrinterData<'a, 'tcx, F>;
+impl<'a, 'tcx> Deref for FmtPrinter<'a, 'tcx> {
+    type Target = FmtPrinterData<'a, 'tcx>;
     fn deref(&self) -> &Self::Target {
         &self.0
     }
 }
 
-impl<F> DerefMut for FmtPrinter<'_, '_, F> {
+impl DerefMut for FmtPrinter<'_, '_> {
     fn deref_mut(&mut self) -> &mut Self::Target {
         &mut self.0
     }
 }
 
-impl<'a, 'tcx, F> FmtPrinter<'a, 'tcx, F> {
-    pub fn new(tcx: TyCtxt<'tcx>, fmt: F, ns: Namespace) -> Self {
+impl<'a, 'tcx> FmtPrinter<'a, 'tcx> {
+    pub fn new(tcx: TyCtxt<'tcx>, ns: Namespace) -> Self {
         FmtPrinter(Box::new(FmtPrinterData {
             tcx,
-            fmt,
+            // Estimated reasonable capacity to allocate upfront based on a few
+            // benchmarks.
+            fmt: String::with_capacity(64),
             empty_path: false,
             in_value: ns == Namespace::ValueNS,
             print_alloc_ids: false,
@@ -1594,6 +1596,10 @@ impl<'a, 'tcx, F> FmtPrinter<'a, 'tcx, F> {
             const_infer_name_resolver: None,
         }))
     }
+
+    pub fn into_buffer(self) -> String {
+        self.0.fmt
+    }
 }
 
 // HACK(eddyb) get rid of `def_path_str` and/or pass `Namespace` explicitly always
@@ -1625,19 +1631,18 @@ impl<'t> TyCtxt<'t> {
     pub fn def_path_str_with_substs(self, def_id: DefId, substs: &'t [GenericArg<'t>]) -> String {
         let ns = guess_def_namespace(self, def_id);
         debug!("def_path_str: def_id={:?}, ns={:?}", def_id, ns);
-        let mut s = String::new();
-        let _ = FmtPrinter::new(self, &mut s, ns).print_def_path(def_id, substs);
-        s
+        FmtPrinter::new(self, ns).print_def_path(def_id, substs).unwrap().into_buffer()
     }
 }
 
-impl<F: fmt::Write> fmt::Write for FmtPrinter<'_, '_, F> {
+impl fmt::Write for FmtPrinter<'_, '_> {
     fn write_str(&mut self, s: &str) -> fmt::Result {
-        self.fmt.write_str(s)
+        self.fmt.push_str(s);
+        Ok(())
     }
 }
 
-impl<'tcx, F: fmt::Write> Printer<'tcx> for FmtPrinter<'_, 'tcx, F> {
+impl<'tcx> Printer<'tcx> for FmtPrinter<'_, 'tcx> {
     type Error = fmt::Error;
 
     type Path = Self;
@@ -1845,7 +1850,7 @@ impl<'tcx, F: fmt::Write> Printer<'tcx> for FmtPrinter<'_, 'tcx, F> {
     }
 }
 
-impl<'tcx, F: fmt::Write> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx, F> {
+impl<'tcx> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx> {
     fn ty_infer_name(&self, id: ty::TyVid) -> Option<String> {
         self.0.ty_infer_name_resolver.as_ref().and_then(|func| func(id))
     }
@@ -1981,7 +1986,7 @@ impl<'tcx, F: fmt::Write> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx, F> {
 }
 
 // HACK(eddyb) limited to `FmtPrinter` because of `region_highlight_mode`.
-impl<F: fmt::Write> FmtPrinter<'_, '_, F> {
+impl FmtPrinter<'_, '_> {
     pub fn pretty_print_region(mut self, region: ty::Region<'_>) -> Result<Self, fmt::Error> {
         define_scoped_cx!(self);
 
@@ -2115,7 +2120,7 @@ impl<'a, 'tcx> ty::TypeFolder<'tcx> for RegionFolder<'a, 'tcx> {
 
 // HACK(eddyb) limited to `FmtPrinter` because of `binder_depth`,
 // `region_index` and `used_region_names`.
-impl<'tcx, F: fmt::Write> FmtPrinter<'_, 'tcx, F> {
+impl<'tcx> FmtPrinter<'_, 'tcx> {
     pub fn name_all_regions<T>(
         mut self,
         value: &ty::Binder<'tcx, T>,
@@ -2367,9 +2372,10 @@ macro_rules! forward_display_to_print {
         $(#[allow(unused_lifetimes)] impl<'tcx> fmt::Display for $ty {
             fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
                 ty::tls::with(|tcx| {
-                    tcx.lift(*self)
+                    let cx = tcx.lift(*self)
                         .expect("could not lift for printing")
-                        .print(FmtPrinter::new(tcx, f, Namespace::TypeNS))?;
+                        .print(FmtPrinter::new(tcx, Namespace::TypeNS))?;
+                    f.write_str(&cx.into_buffer())?;
                     Ok(())
                 })
             }
@@ -2400,8 +2406,7 @@ macro_rules! define_print_and_forward_display {
 impl<'tcx> fmt::Display for ty::Region<'tcx> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         ty::tls::with(|tcx| {
-            self.print(FmtPrinter::new(tcx, f, Namespace::TypeNS))?;
-            Ok(())
+            f.write_str(&self.print(FmtPrinter::new(tcx, Namespace::TypeNS))?.into_buffer())
         })
     }
 }
diff --git a/compiler/rustc_middle/src/ty/structural_impls.rs b/compiler/rustc_middle/src/ty/structural_impls.rs

Original file line number	Diff line number	Diff line change
`@@ -983,10 +983,9 @@ fn foo(&self) -> Self::T { String::new() }`
`983`	`983`	`}`
`984`	`984`
`985`	`985`	`fn format_generic_args(self, args: &[ty::GenericArg<'tcx>]) -> String {`
`986`		`- let mut item_args = String::new();`
`987`		`- FmtPrinter::new(self, &mut item_args, hir::def::Namespace::TypeNS)`
	`986`	`+ FmtPrinter::new(self, hir::def::Namespace::TypeNS)`
`988`	`987`	`.path_generic_args(Ok, args)`
`989`		- .expect("could not write to `String`.");
`990`		`- item_args`
	`988`	+ .expect("could not write to `String`.")
	`989`	`+ .into_buffer()`
`991`	`990`	`}`
`992`	`991`	`}`