Skip to content

Commit d8d7a18

Browse files
authored
Unrolled build for #159302
Rollup merge of #159302 - connortsui20:dyn-debug-helpers, r=hanna-kruppe Implement `Debug` helpers via `Cell` Related to #149745, but does not fix it (yet). Following @jmillikin's [suggestion](#159302 (comment)), the builder logic now lives in the pre-existing non-generic methods taking `&dyn fmt::Debug` (where it used to exist), and each `*_with` method wraps its closure in a private `DebugOnce` struct that implements `Debug` by calling the closure, so the body is compiled once. Just for context: A `dyn FnOnce` [can't be called behind a reference](#149745 (comment)), hence the `Cell<Option<..>>` stuff in `DebugOnce`. ###### repro.rs ```rust #![feature(debug_closure_helpers)] #![crate_type = "lib"] use core::fmt; pub struct Point { pub x: u32, pub y: u32, } impl fmt::Debug for Point { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Point") .field_with("x", |f| self.x.fmt(f)) .field_with("y", |f| self.y.fmt(f)) .finish() } } ``` ```bash rustc +nightly --edition=2024 --emit=llvm-ir -Copt-level=0 -o before.ll repro.rs rustc +stage1 --edition=2024 --emit=llvm-ir -Copt-level=0 -o after.ll repro.rs ``` On the repro above, I saw the LLVM IR drop from 739 to 268 lines (an earlier version of this PR using `&mut dyn FnMut` measured 372). Since the stable `&dyn fmt::Debug` methods no longer go through any closure indirection, this should also avoid the potential runtime regression in the `derive(Debug)` microbenchmark. This was the only remaining [blocker](#117729 (comment)) for stabilizing `debug_closure_helpers`, so it should unblock #146099.
2 parents 656ccbe + b727274 commit d8d7a18

1 file changed

Lines changed: 92 additions & 64 deletions

File tree

library/core/src/fmt/builders.rs

Lines changed: 92 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#![allow(unused_imports)]
22

3+
use crate::cell::Cell;
34
use crate::fmt::{self, Debug, Formatter};
45

56
struct PadAdapter<'buf, 'state> {
@@ -50,6 +51,29 @@ impl fmt::Write for PadAdapter<'_, '_> {
5051
}
5152
}
5253

54+
/// Wraps an `FnOnce` formatting closure in a type that implements [`fmt::Debug`] by calling the
55+
/// closure, allowing the `*_with` builder methods to forward to their `&dyn fmt::Debug`
56+
/// counterparts.
57+
///
58+
/// By doing this, the builder logic is monomorphized only once and not for every closure type
59+
/// (see #149745).
60+
///
61+
/// Formatting a `DebugOnce` consumes the closure, so attempting to format it more than once
62+
/// panics. This never happens because the debug builders format each value exactly once.
63+
struct DebugOnce<F>(Cell<Option<F>>);
64+
65+
impl<F> fmt::Debug for DebugOnce<F>
66+
where
67+
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
68+
{
69+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
70+
match self.0.take() {
71+
Some(value_fmt) => value_fmt(f),
72+
None => panic!("formatting closure called more than once"),
73+
}
74+
}
75+
}
76+
5377
/// A struct to help with [`fmt::Debug`](Debug) implementations.
5478
///
5579
/// This is useful when you wish to output a formatted struct as a part of your
@@ -130,18 +154,6 @@ impl<'a, 'b: 'a> DebugStruct<'a, 'b> {
130154
/// ```
131155
#[stable(feature = "debug_builders", since = "1.2.0")]
132156
pub fn field(&mut self, name: &str, value: &dyn fmt::Debug) -> &mut Self {
133-
self.field_with(name, |f| value.fmt(f))
134-
}
135-
136-
/// Adds a new field to the generated struct output.
137-
///
138-
/// This method is equivalent to [`DebugStruct::field`], but formats the
139-
/// value using a provided closure rather than by calling [`Debug::fmt`].
140-
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
141-
pub fn field_with<F>(&mut self, name: &str, value_fmt: F) -> &mut Self
142-
where
143-
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
144-
{
145157
self.result = self.result.and_then(|_| {
146158
if self.is_pretty() {
147159
if !self.has_fields {
@@ -152,21 +164,33 @@ impl<'a, 'b: 'a> DebugStruct<'a, 'b> {
152164
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut state);
153165
writer.write_str(name)?;
154166
writer.write_str(": ")?;
155-
value_fmt(&mut writer)?;
167+
value.fmt(&mut writer)?;
156168
writer.write_str(",\n")
157169
} else {
158170
let prefix = if self.has_fields { ", " } else { " { " };
159171
self.fmt.write_str(prefix)?;
160172
self.fmt.write_str(name)?;
161173
self.fmt.write_str(": ")?;
162-
value_fmt(self.fmt)
174+
value.fmt(self.fmt)
163175
}
164176
});
165177

166178
self.has_fields = true;
167179
self
168180
}
169181

182+
/// Adds a new field to the generated struct output.
183+
///
184+
/// This method is equivalent to [`DebugStruct::field`], but formats the
185+
/// value using a provided closure rather than by calling [`Debug::fmt`].
186+
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
187+
pub fn field_with<F>(&mut self, name: &str, value_fmt: F) -> &mut Self
188+
where
189+
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
190+
{
191+
self.field(name, &DebugOnce(Cell::new(Some(value_fmt))))
192+
}
193+
170194
/// Marks the struct as non-exhaustive, indicating to the reader that there are some other
171195
/// fields that are not shown in the debug representation.
172196
///
@@ -327,18 +351,6 @@ impl<'a, 'b: 'a> DebugTuple<'a, 'b> {
327351
/// ```
328352
#[stable(feature = "debug_builders", since = "1.2.0")]
329353
pub fn field(&mut self, value: &dyn fmt::Debug) -> &mut Self {
330-
self.field_with(|f| value.fmt(f))
331-
}
332-
333-
/// Adds a new field to the generated tuple struct output.
334-
///
335-
/// This method is equivalent to [`DebugTuple::field`], but formats the
336-
/// value using a provided closure rather than by calling [`Debug::fmt`].
337-
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
338-
pub fn field_with<F>(&mut self, value_fmt: F) -> &mut Self
339-
where
340-
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
341-
{
342354
self.result = self.result.and_then(|_| {
343355
if self.is_pretty() {
344356
if self.fields == 0 {
@@ -347,19 +359,31 @@ impl<'a, 'b: 'a> DebugTuple<'a, 'b> {
347359
let mut slot = None;
348360
let mut state = Default::default();
349361
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut state);
350-
value_fmt(&mut writer)?;
362+
value.fmt(&mut writer)?;
351363
writer.write_str(",\n")
352364
} else {
353365
let prefix = if self.fields == 0 { "(" } else { ", " };
354366
self.fmt.write_str(prefix)?;
355-
value_fmt(self.fmt)
367+
value.fmt(self.fmt)
356368
}
357369
});
358370

359371
self.fields += 1;
360372
self
361373
}
362374

375+
/// Adds a new field to the generated tuple struct output.
376+
///
377+
/// This method is equivalent to [`DebugTuple::field`], but formats the
378+
/// value using a provided closure rather than by calling [`Debug::fmt`].
379+
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
380+
pub fn field_with<F>(&mut self, value_fmt: F) -> &mut Self
381+
where
382+
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
383+
{
384+
self.field(&DebugOnce(Cell::new(Some(value_fmt))))
385+
}
386+
363387
/// Marks the tuple struct as non-exhaustive, indicating to the reader that there are some
364388
/// other fields that are not shown in the debug representation.
365389
///
@@ -453,10 +477,7 @@ struct DebugInner<'a, 'b: 'a> {
453477
}
454478

455479
impl<'a, 'b: 'a> DebugInner<'a, 'b> {
456-
fn entry_with<F>(&mut self, entry_fmt: F)
457-
where
458-
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
459-
{
480+
fn entry(&mut self, entry: &dyn fmt::Debug) {
460481
self.result = self.result.and_then(|_| {
461482
if self.is_pretty() {
462483
if !self.has_fields {
@@ -465,19 +486,26 @@ impl<'a, 'b: 'a> DebugInner<'a, 'b> {
465486
let mut slot = None;
466487
let mut state = Default::default();
467488
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut state);
468-
entry_fmt(&mut writer)?;
489+
entry.fmt(&mut writer)?;
469490
writer.write_str(",\n")
470491
} else {
471492
if self.has_fields {
472493
self.fmt.write_str(", ")?
473494
}
474-
entry_fmt(self.fmt)
495+
entry.fmt(self.fmt)
475496
}
476497
});
477498

478499
self.has_fields = true;
479500
}
480501

502+
fn entry_with<F>(&mut self, entry_fmt: F)
503+
where
504+
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
505+
{
506+
self.entry(&DebugOnce(Cell::new(Some(entry_fmt))));
507+
}
508+
481509
fn is_pretty(&self) -> bool {
482510
self.fmt.alternate()
483511
}
@@ -546,7 +574,7 @@ impl<'a, 'b: 'a> DebugSet<'a, 'b> {
546574
/// ```
547575
#[stable(feature = "debug_builders", since = "1.2.0")]
548576
pub fn entry(&mut self, entry: &dyn fmt::Debug) -> &mut Self {
549-
self.inner.entry_with(|f| entry.fmt(f));
577+
self.inner.entry(entry);
550578
self
551579
}
552580

@@ -738,7 +766,7 @@ impl<'a, 'b: 'a> DebugList<'a, 'b> {
738766
/// ```
739767
#[stable(feature = "debug_builders", since = "1.2.0")]
740768
pub fn entry(&mut self, entry: &dyn fmt::Debug) -> &mut Self {
741-
self.inner.entry_with(|f| entry.fmt(f));
769+
self.inner.entry(entry);
742770
self
743771
}
744772

@@ -969,18 +997,6 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
969997
/// ```
970998
#[stable(feature = "debug_map_key_value", since = "1.42.0")]
971999
pub fn key(&mut self, key: &dyn fmt::Debug) -> &mut Self {
972-
self.key_with(|f| key.fmt(f))
973-
}
974-
975-
/// Adds the key part of a new entry to the map output.
976-
///
977-
/// This method is equivalent to [`DebugMap::key`], but formats the
978-
/// key using a provided closure rather than by calling [`Debug::fmt`].
979-
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
980-
pub fn key_with<F>(&mut self, key_fmt: F) -> &mut Self
981-
where
982-
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
983-
{
9841000
self.result = self.result.and_then(|_| {
9851001
assert!(
9861002
!self.has_key,
@@ -995,13 +1011,13 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
9951011
let mut slot = None;
9961012
self.state = Default::default();
9971013
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut self.state);
998-
key_fmt(&mut writer)?;
1014+
key.fmt(&mut writer)?;
9991015
writer.write_str(": ")?;
10001016
} else {
10011017
if self.has_fields {
10021018
self.fmt.write_str(", ")?
10031019
}
1004-
key_fmt(self.fmt)?;
1020+
key.fmt(self.fmt)?;
10051021
self.fmt.write_str(": ")?;
10061022
}
10071023

@@ -1012,6 +1028,18 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
10121028
self
10131029
}
10141030

1031+
/// Adds the key part of a new entry to the map output.
1032+
///
1033+
/// This method is equivalent to [`DebugMap::key`], but formats the
1034+
/// key using a provided closure rather than by calling [`Debug::fmt`].
1035+
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
1036+
pub fn key_with<F>(&mut self, key_fmt: F) -> &mut Self
1037+
where
1038+
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
1039+
{
1040+
self.key(&DebugOnce(Cell::new(Some(key_fmt))))
1041+
}
1042+
10151043
/// Adds the value part of a new entry to the map output.
10161044
///
10171045
/// This method, together with `key`, is an alternative to `entry` that
@@ -1045,28 +1073,16 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
10451073
/// ```
10461074
#[stable(feature = "debug_map_key_value", since = "1.42.0")]
10471075
pub fn value(&mut self, value: &dyn fmt::Debug) -> &mut Self {
1048-
self.value_with(|f| value.fmt(f))
1049-
}
1050-
1051-
/// Adds the value part of a new entry to the map output.
1052-
///
1053-
/// This method is equivalent to [`DebugMap::value`], but formats the
1054-
/// value using a provided closure rather than by calling [`Debug::fmt`].
1055-
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
1056-
pub fn value_with<F>(&mut self, value_fmt: F) -> &mut Self
1057-
where
1058-
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
1059-
{
10601076
self.result = self.result.and_then(|_| {
10611077
assert!(self.has_key, "attempted to format a map value before its key");
10621078

10631079
if self.is_pretty() {
10641080
let mut slot = None;
10651081
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut self.state);
1066-
value_fmt(&mut writer)?;
1082+
value.fmt(&mut writer)?;
10671083
writer.write_str(",\n")?;
10681084
} else {
1069-
value_fmt(self.fmt)?;
1085+
value.fmt(self.fmt)?;
10701086
}
10711087

10721088
self.has_key = false;
@@ -1077,6 +1093,18 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
10771093
self
10781094
}
10791095

1096+
/// Adds the value part of a new entry to the map output.
1097+
///
1098+
/// This method is equivalent to [`DebugMap::value`], but formats the
1099+
/// value using a provided closure rather than by calling [`Debug::fmt`].
1100+
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
1101+
pub fn value_with<F>(&mut self, value_fmt: F) -> &mut Self
1102+
where
1103+
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
1104+
{
1105+
self.value(&DebugOnce(Cell::new(Some(value_fmt))))
1106+
}
1107+
10801108
/// Adds the contents of an iterator of entries to the map output.
10811109
///
10821110
/// # Examples

0 commit comments

Comments
 (0)