Skip to content

Commit 4b043fa

Browse files
borsbors
committed
Auto merge of rust-lang#94131 - Mark-Simulacrum:fmt-string, r=oli-obk
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.
2 parents 3d127e2 + 2ee6d55 commit 4b043fa

File tree

10 files changed

+84
-80
lines changed

10 files changed

+84
-80
lines changed

‎compiler/rustc_borrowck/src/diagnostics/mod.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -488,8 +488,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
488488
/// Return the name of the provided `Ty` (that must be a reference) with a synthesized lifetime
489489
/// name where required.
490490
pub(super) fn get_name_for_ty(&self, ty: Ty<'tcx>, counter: usize) -> String {
491-
let mut s = String::new();
492-
let mut printer = ty::print::FmtPrinter::new(self.infcx.tcx, &mut s, Namespace::TypeNS);
491+
let mut printer = ty::print::FmtPrinter::new(self.infcx.tcx, Namespace::TypeNS);
493492

494493
// We need to add synthesized lifetimes where appropriate. We do
495494
// this by hooking into the pretty printer and telling it to label the
@@ -504,15 +503,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
504503
}
505504
}
506505

507-
let _ = ty.print(printer);
508-
s
506+
ty.print(printer).unwrap().into_buffer()
509507
}
510508

511509
/// Returns the name of the provided `Ty` (that must be a reference)'s region with a
512510
/// synthesized lifetime name where required.
513511
pub(super) fn get_region_name_for_ty(&self, ty: Ty<'tcx>, counter: usize) -> String {
514-
let mut s = String::new();
515-
let mut printer = ty::print::FmtPrinter::new(self.infcx.tcx, &mut s, Namespace::TypeNS);
512+
let mut printer = ty::print::FmtPrinter::new(self.infcx.tcx, Namespace::TypeNS);
516513

517514
let region = if let ty::Ref(region, ..) = ty.kind() {
518515
match **region {
@@ -527,8 +524,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
527524
bug!("ty for annotation of borrow region is not a reference");
528525
};
529526

530-
let _ = region.print(printer);
531-
s
527+
region.print(printer).unwrap().into_buffer()
532528
}
533529
}
534530

‎compiler/rustc_const_eval/src/interpret/operand.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,11 +109,11 @@ rustc_data_structures::static_assert_size!(ImmTy<'_>, 72);
109109
impl<Tag: Provenance> std::fmt::Display for ImmTy<'_, Tag> {
110110
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
111111
/// Helper function for printing a scalar to a FmtPrinter
112-
fn p<'a, 'tcx, F: std::fmt::Write, Tag: Provenance>(
113-
cx: FmtPrinter<'a, 'tcx, F>,
112+
fn p<'a, 'tcx, Tag: Provenance>(
113+
cx: FmtPrinter<'a, 'tcx>,
114114
s: ScalarMaybeUninit<Tag>,
115115
ty: Ty<'tcx>,
116-
) -> Result<FmtPrinter<'a, 'tcx, F>, std::fmt::Error> {
116+
) -> Result<FmtPrinter<'a, 'tcx>, std::fmt::Error> {
117117
match s {
118118
ScalarMaybeUninit::Scalar(Scalar::Int(int)) => {
119119
cx.pretty_print_const_scalar_int(int, ty, true)
@@ -138,8 +138,8 @@ impl<Tag: Provenance> std::fmt::Display for ImmTy<'_, Tag> {
138138
match self.imm {
139139
Immediate::Scalar(s) => {
140140
if let Some(ty) = tcx.lift(self.layout.ty) {
141-
let cx = FmtPrinter::new(tcx, f, Namespace::ValueNS);
142-
p(cx, s, ty)?;
141+
let cx = FmtPrinter::new(tcx, Namespace::ValueNS);
142+
f.write_str(&p(cx, s, ty)?.into_buffer())?;
143143
return Ok(());
144144
}
145145
write!(f, "{:x}: {}", s, self.layout.ty)

‎compiler/rustc_infer/src/infer/error_reporting/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -988,8 +988,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
988988
) -> (DiagnosticStyledString, DiagnosticStyledString) {
989989
let get_lifetimes = |sig| {
990990
use rustc_hir::def::Namespace;
991-
let mut s = String::new();
992-
let (_, sig, reg) = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS)
991+
let (_, sig, reg) = ty::print::FmtPrinter::new(self.tcx, Namespace::TypeNS)
993992
.name_all_regions(sig)
994993
.unwrap();
995994
let lts: Vec<String> = reg.into_iter().map(|(_, kind)| kind.to_string()).collect();

‎compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -397,14 +397,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
397397
}
398398
}
399399

400-
let mut s = String::new();
401-
let mut printer = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS);
400+
let mut printer = ty::print::FmtPrinter::new(self.tcx, Namespace::TypeNS);
402401
if let Some(highlight) = highlight {
403402
printer.region_highlight_mode = highlight;
404403
}
405-
let _ = ty.print(printer);
404+
let name = ty.print(printer).unwrap().into_buffer();
406405
InferenceDiagnosticsData {
407-
name: s,
406+
name,
408407
span: None,
409408
kind: UnderspecifiedArgKind::Type { prefix: ty.prefix_string(self.tcx) },
410409
parent: None,
@@ -433,15 +432,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
433432
}
434433

435434
debug_assert!(!origin.span.is_dummy());
436-
let mut s = String::new();
437-
let mut printer =
438-
ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::ValueNS);
435+
let mut printer = ty::print::FmtPrinter::new(self.tcx, Namespace::ValueNS);
439436
if let Some(highlight) = highlight {
440437
printer.region_highlight_mode = highlight;
441438
}
442-
let _ = ct.print(printer);
439+
let name = ct.print(printer).unwrap().into_buffer();
443440
InferenceDiagnosticsData {
444-
name: s,
441+
name,
445442
span: Some(origin.span),
446443
kind: UnderspecifiedArgKind::Const { is_parameter: false },
447444
parent: None,
@@ -497,8 +494,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
497494

498495
let mut local_visitor = FindHirNodeVisitor::new(&self, arg, span);
499496
let ty_to_string = |ty: Ty<'tcx>| -> String {
500-
let mut s = String::new();
501-
let mut printer = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS);
497+
let mut printer = ty::print::FmtPrinter::new(self.tcx, Namespace::TypeNS);
502498
let ty_getter = move |ty_vid| {
503499
if let TypeVariableOriginKind::TypeParameterDefinition(name, _) =
504500
self.inner.borrow_mut().type_variables().var_origin(ty_vid).kind
@@ -525,14 +521,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
525521
};
526522
printer.const_infer_name_resolver = Some(Box::new(const_getter));
527523

528-
let _ = if let ty::FnDef(..) = ty.kind() {
524+
if let ty::FnDef(..) = ty.kind() {
529525
// We don't want the regular output for `fn`s because it includes its path in
530526
// invalid pseudo-syntax, we want the `fn`-pointer output instead.
531-
ty.fn_sig(self.tcx).print(printer)
527+
ty.fn_sig(self.tcx).print(printer).unwrap().into_buffer()
532528
} else {
533-
ty.print(printer)
534-
};
535-
s
529+
ty.print(printer).unwrap().into_buffer()
530+
}
536531
};
537532

538533
if let Some(body_id) = body_id {

‎compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -335,18 +335,19 @@ impl<'tcx> NiceRegionError<'_, 'tcx> {
335335

336336
impl<'tcx, T> fmt::Display for Highlighted<'tcx, T>
337337
where
338-
T: for<'a, 'b, 'c> Print<
338+
T: for<'a> Print<
339339
'tcx,
340-
FmtPrinter<'a, 'tcx, &'b mut fmt::Formatter<'c>>,
340+
FmtPrinter<'a, 'tcx>,
341341
Error = fmt::Error,
342+
Output = FmtPrinter<'a, 'tcx>,
342343
>,
343344
{
344345
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
345-
let mut printer = ty::print::FmtPrinter::new(self.tcx, f, Namespace::TypeNS);
346+
let mut printer = ty::print::FmtPrinter::new(self.tcx, Namespace::TypeNS);
346347
printer.region_highlight_mode = self.highlight;
347348

348-
self.value.print(printer)?;
349-
Ok(())
349+
let s = self.value.print(printer)?.into_buffer();
350+
f.write_str(&s)
350351
}
351352
}
352353

‎compiler/rustc_middle/src/mir/mod.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2421,11 +2421,11 @@ impl<'tcx> Debug for Rvalue<'tcx> {
24212421

24222422
AggregateKind::Adt(adt_did, variant, substs, _user_ty, _) => {
24232423
ty::tls::with(|tcx| {
2424-
let mut name = String::new();
24252424
let variant_def = &tcx.adt_def(adt_did).variants[variant];
24262425
let substs = tcx.lift(substs).expect("could not lift for printing");
2427-
FmtPrinter::new(tcx, &mut name, Namespace::ValueNS)
2428-
.print_def_path(variant_def.def_id, substs)?;
2426+
let name = FmtPrinter::new(tcx, Namespace::ValueNS)
2427+
.print_def_path(variant_def.def_id, substs)?
2428+
.into_buffer();
24292429

24302430
match variant_def.ctor_kind {
24312431
CtorKind::Const => fmt.write_str(&name),
@@ -2847,9 +2847,10 @@ fn pretty_print_const<'tcx>(
28472847
use crate::ty::print::PrettyPrinter;
28482848
ty::tls::with(|tcx| {
28492849
let literal = tcx.lift(c).unwrap();
2850-
let mut cx = FmtPrinter::new(tcx, fmt, Namespace::ValueNS);
2850+
let mut cx = FmtPrinter::new(tcx, Namespace::ValueNS);
28512851
cx.print_alloc_ids = true;
2852-
cx.pretty_print_const(literal, print_types)?;
2852+
let cx = cx.pretty_print_const(literal, print_types)?;
2853+
fmt.write_str(&cx.into_buffer())?;
28532854
Ok(())
28542855
})
28552856
}
@@ -2864,9 +2865,10 @@ fn pretty_print_const_value<'tcx>(
28642865
ty::tls::with(|tcx| {
28652866
let val = tcx.lift(val).unwrap();
28662867
let ty = tcx.lift(ty).unwrap();
2867-
let mut cx = FmtPrinter::new(tcx, fmt, Namespace::ValueNS);
2868+
let mut cx = FmtPrinter::new(tcx, Namespace::ValueNS);
28682869
cx.print_alloc_ids = true;
2869-
cx.pretty_print_const_value(val, ty, print_types)?;
2870+
let cx = cx.pretty_print_const_value(val, ty, print_types)?;
2871+
fmt.write_str(&cx.into_buffer())?;
28702872
Ok(())
28712873
})
28722874
}

‎compiler/rustc_middle/src/ty/error.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -983,10 +983,9 @@ fn foo(&self) -> Self::T { String::new() }
983983
}
984984

985985
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)
988987
.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()
991990
}
992991
}

‎compiler/rustc_middle/src/ty/instance.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,10 @@ impl<'tcx> fmt::Display for Instance<'tcx> {
279279
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
280280
ty::tls::with(|tcx| {
281281
let substs = tcx.lift(self.substs).expect("could not lift for printing");
282-
FmtPrinter::new(tcx, &mut *f, Namespace::ValueNS)
283-
.print_def_path(self.def_id(), substs)?;
284-
Ok(())
282+
let s = FmtPrinter::new(tcx, Namespace::ValueNS)
283+
.print_def_path(self.def_id(), substs)?
284+
.into_buffer();
285+
f.write_str(&s)
285286
})?;
286287

287288
match self.def {

‎compiler/rustc_middle/src/ty/print/pretty.rs

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1543,11 +1543,11 @@ pub trait PrettyPrinter<'tcx>:
15431543
}
15441544

15451545
// HACK(eddyb) boxed to avoid moving around a large struct by-value.
1546-
pub struct FmtPrinter<'a, 'tcx, F>(Box<FmtPrinterData<'a, 'tcx, F>>);
1546+
pub struct FmtPrinter<'a, 'tcx>(Box<FmtPrinterData<'a, 'tcx>>);
15471547

1548-
pub struct FmtPrinterData<'a, 'tcx, F> {
1548+
pub struct FmtPrinterData<'a, 'tcx> {
15491549
tcx: TyCtxt<'tcx>,
1550-
fmt: F,
1550+
fmt: String,
15511551

15521552
empty_path: bool,
15531553
in_value: bool,
@@ -1564,24 +1564,26 @@ pub struct FmtPrinterData<'a, 'tcx, F> {
15641564
pub const_infer_name_resolver: Option<Box<dyn Fn(ty::ConstVid<'tcx>) -> Option<String> + 'a>>,
15651565
}
15661566

1567-
impl<'a, 'tcx, F> Deref for FmtPrinter<'a, 'tcx, F> {
1568-
type Target = FmtPrinterData<'a, 'tcx, F>;
1567+
impl<'a, 'tcx> Deref for FmtPrinter<'a, 'tcx> {
1568+
type Target = FmtPrinterData<'a, 'tcx>;
15691569
fn deref(&self) -> &Self::Target {
15701570
&self.0
15711571
}
15721572
}
15731573

1574-
impl<F> DerefMut for FmtPrinter<'_, '_, F> {
1574+
impl DerefMut for FmtPrinter<'_, '_> {
15751575
fn deref_mut(&mut self) -> &mut Self::Target {
15761576
&mut self.0
15771577
}
15781578
}
15791579

1580-
impl<'a, 'tcx, F> FmtPrinter<'a, 'tcx, F> {
1581-
pub fn new(tcx: TyCtxt<'tcx>, fmt: F, ns: Namespace) -> Self {
1580+
impl<'a, 'tcx> FmtPrinter<'a, 'tcx> {
1581+
pub fn new(tcx: TyCtxt<'tcx>, ns: Namespace) -> Self {
15821582
FmtPrinter(Box::new(FmtPrinterData {
15831583
tcx,
1584-
fmt,
1584+
// Estimated reasonable capacity to allocate upfront based on a few
1585+
// benchmarks.
1586+
fmt: String::with_capacity(64),
15851587
empty_path: false,
15861588
in_value: ns == Namespace::ValueNS,
15871589
print_alloc_ids: false,
@@ -1594,6 +1596,10 @@ impl<'a, 'tcx, F> FmtPrinter<'a, 'tcx, F> {
15941596
const_infer_name_resolver: None,
15951597
}))
15961598
}
1599+
1600+
pub fn into_buffer(self) -> String {
1601+
self.0.fmt
1602+
}
15971603
}
15981604

15991605
// HACK(eddyb) get rid of `def_path_str` and/or pass `Namespace` explicitly always
@@ -1625,19 +1631,18 @@ impl<'t> TyCtxt<'t> {
16251631
pub fn def_path_str_with_substs(self, def_id: DefId, substs: &'t [GenericArg<'t>]) -> String {
16261632
let ns = guess_def_namespace(self, def_id);
16271633
debug!("def_path_str: def_id={:?}, ns={:?}", def_id, ns);
1628-
let mut s = String::new();
1629-
let _ = FmtPrinter::new(self, &mut s, ns).print_def_path(def_id, substs);
1630-
s
1634+
FmtPrinter::new(self, ns).print_def_path(def_id, substs).unwrap().into_buffer()
16311635
}
16321636
}
16331637

1634-
impl<F: fmt::Write> fmt::Write for FmtPrinter<'_, '_, F> {
1638+
impl fmt::Write for FmtPrinter<'_, '_> {
16351639
fn write_str(&mut self, s: &str) -> fmt::Result {
1636-
self.fmt.write_str(s)
1640+
self.fmt.push_str(s);
1641+
Ok(())
16371642
}
16381643
}
16391644

1640-
impl<'tcx, F: fmt::Write> Printer<'tcx> for FmtPrinter<'_, 'tcx, F> {
1645+
impl<'tcx> Printer<'tcx> for FmtPrinter<'_, 'tcx> {
16411646
type Error = fmt::Error;
16421647

16431648
type Path = Self;
@@ -1845,7 +1850,7 @@ impl<'tcx, F: fmt::Write> Printer<'tcx> for FmtPrinter<'_, 'tcx, F> {
18451850
}
18461851
}
18471852

1848-
impl<'tcx, F: fmt::Write> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx, F> {
1853+
impl<'tcx> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx> {
18491854
fn ty_infer_name(&self, id: ty::TyVid) -> Option<String> {
18501855
self.0.ty_infer_name_resolver.as_ref().and_then(|func| func(id))
18511856
}
@@ -1981,7 +1986,7 @@ impl<'tcx, F: fmt::Write> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx, F> {
19811986
}
19821987

19831988
// HACK(eddyb) limited to `FmtPrinter` because of `region_highlight_mode`.
1984-
impl<F: fmt::Write> FmtPrinter<'_, '_, F> {
1989+
impl FmtPrinter<'_, '_> {
19851990
pub fn pretty_print_region(mut self, region: ty::Region<'_>) -> Result<Self, fmt::Error> {
19861991
define_scoped_cx!(self);
19871992

@@ -2115,7 +2120,7 @@ impl<'a, 'tcx> ty::TypeFolder<'tcx> for RegionFolder<'a, 'tcx> {
21152120

21162121
// HACK(eddyb) limited to `FmtPrinter` because of `binder_depth`,
21172122
// `region_index` and `used_region_names`.
2118-
impl<'tcx, F: fmt::Write> FmtPrinter<'_, 'tcx, F> {
2123+
impl<'tcx> FmtPrinter<'_, 'tcx> {
21192124
pub fn name_all_regions<T>(
21202125
mut self,
21212126
value: &ty::Binder<'tcx, T>,
@@ -2367,9 +2372,10 @@ macro_rules! forward_display_to_print {
23672372
$(#[allow(unused_lifetimes)] impl<'tcx> fmt::Display for $ty {
23682373
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
23692374
ty::tls::with(|tcx| {
2370-
tcx.lift(*self)
2375+
let cx = tcx.lift(*self)
23712376
.expect("could not lift for printing")
2372-
.print(FmtPrinter::new(tcx, f, Namespace::TypeNS))?;
2377+
.print(FmtPrinter::new(tcx, Namespace::TypeNS))?;
2378+
f.write_str(&cx.into_buffer())?;
23732379
Ok(())
23742380
})
23752381
}
@@ -2400,8 +2406,7 @@ macro_rules! define_print_and_forward_display {
24002406
impl<'tcx> fmt::Display for ty::Region<'tcx> {
24012407
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
24022408
ty::tls::with(|tcx| {
2403-
self.print(FmtPrinter::new(tcx, f, Namespace::TypeNS))?;
2404-
Ok(())
2409+
f.write_str(&self.print(FmtPrinter::new(tcx, Namespace::TypeNS))?.into_buffer())
24052410
})
24062411
}
24072412
}

0 commit comments

Comments
 (0)