Skip to content

Implement Debug helpers via Cell #159302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
connortsui20 wants to merge 1 commit into
base: main
Choose a base branch
from
+92 −64
Open

Implement Debug helpers via Cell #159302

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 92 additions & 64 deletions library/core/src/fmt/builders.rs
View file
Open in desktop

@tgross35 tgross35 Jul 16, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way we could reasonably cover this in a codegen test?

View changes since the review

@hanna-kruppe hanna-kruppe Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've asked myself that on other changes to reduce monomorphization, and I never came up with a satisfying answer. Matching against the structure of calling some shared monomorphic function is the closest thing, but that's not really reasonable because it's either way too unspecific or way too sensitive to details like function names.

@tgross35 tgross35 Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably be a good idea to add a benchmark using these functions to https://github.com/rust-lang/rustc-perf/tree/2e1f10b49224c069b9fdca7fe8d1e78790e8c62b/collector, assuming the differences here make it to optimized binary size.

@hanna-kruppe hanna-kruppe Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think this is worth it in the grand scheme of things. We shouldn’t have an API that’s an unavoidable code size footgun to use at all, but also, this one API isn’t likely to be a sufficiently interesting benchmark for the purpose of rustc-perf, compared to everything else we could be measuring with the limited benchmarking budget. If this API eventually makes its way into a popular application/library, we may measure it that way in a more realistic setting.

Also, yeah, it doesn’t necessarily show up in optimized binary size. The examples we’ve had so far get handled well by function merging so they act mostly like a single field_with call. It’s likely possible to write a microbenchmark that doesn’t have so much obviously identical code, but the real risk is death by a thousand cuts across a large code base, which is hard to model in an artificial benchmark.

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![allow(unused_imports)]

use crate::cell::Cell;
use crate::fmt::{self, Debug, Formatter};

struct PadAdapter<'buf, 'state> {
Expand Down Expand Up @@ -50,6 +51,29 @@ impl fmt::Write for PadAdapter<'_, '_> {
}
}

/// Wraps an `FnOnce` formatting closure in a type that implements [`fmt::Debug`] by calling the
/// closure, allowing the `*_with` builder methods to forward to their `&dyn fmt::Debug`
/// counterparts.
Comment thread
hanna-kruppe marked this conversation as resolved.
///
/// By doing this, the builder logic is monomorphized only once and not for every closure type
/// (see #149745).
///
/// Formatting a `DebugOnce` consumes the closure, so attempting to format it more than once
/// panics. This never happens because the debug builders format each value exactly once.
struct DebugOnce<F>(Cell<Option<F>>);

impl<F> fmt::Debug for DebugOnce<F>
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.0.take() {
Some(value_fmt) => value_fmt(f),
None => panic!("formatting closure called more than once"),

@camsteffen camsteffen Jul 16, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that the panic is optimized out? If not, this should probably return Ok? Even if by some chance this branch were reached, probably better not to panic anyways.

View changes since the review

}
}
}

/// A struct to help with [`fmt::Debug`](Debug) implementations.
///
/// This is useful when you wish to output a formatted struct as a part of your
Expand Down Expand Up @@ -130,18 +154,6 @@ impl<'a, 'b: 'a> DebugStruct<'a, 'b> {
/// ```
#[stable(feature = "debug_builders", since = "1.2.0")]
pub fn field(&mut self, name: &str, value: &dyn fmt::Debug) -> &mut Self {
self.field_with(name, |f| value.fmt(f))
}

/// Adds a new field to the generated struct output.
///
/// This method is equivalent to [`DebugStruct::field`], but formats the
/// value using a provided closure rather than by calling [`Debug::fmt`].
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
pub fn field_with<F>(&mut self, name: &str, value_fmt: F) -> &mut Self
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.result = self.result.and_then(|_| {
if self.is_pretty() {
if !self.has_fields {
Expand All @@ -152,21 +164,33 @@ impl<'a, 'b: 'a> DebugStruct<'a, 'b> {
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut state);
writer.write_str(name)?;
writer.write_str(": ")?;
value_fmt(&mut writer)?;
value.fmt(&mut writer)?;
writer.write_str(",\n")
} else {
let prefix = if self.has_fields { ", " } else { " { " };
self.fmt.write_str(prefix)?;
self.fmt.write_str(name)?;
self.fmt.write_str(": ")?;
value_fmt(self.fmt)
value.fmt(self.fmt)
}
});

self.has_fields = true;
self
}

/// Adds a new field to the generated struct output.
///
/// This method is equivalent to [`DebugStruct::field`], but formats the
/// value using a provided closure rather than by calling [`Debug::fmt`].
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
pub fn field_with<F>(&mut self, name: &str, value_fmt: F) -> &mut Self
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.field(name, &DebugOnce(Cell::new(Some(value_fmt))))
}

/// Marks the struct as non-exhaustive, indicating to the reader that there are some other
/// fields that are not shown in the debug representation.
///
Expand Down Expand Up @@ -327,18 +351,6 @@ impl<'a, 'b: 'a> DebugTuple<'a, 'b> {
/// ```
#[stable(feature = "debug_builders", since = "1.2.0")]
pub fn field(&mut self, value: &dyn fmt::Debug) -> &mut Self {
self.field_with(|f| value.fmt(f))
}

/// Adds a new field to the generated tuple struct output.
///
/// This method is equivalent to [`DebugTuple::field`], but formats the
/// value using a provided closure rather than by calling [`Debug::fmt`].
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
pub fn field_with<F>(&mut self, value_fmt: F) -> &mut Self
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.result = self.result.and_then(|_| {
if self.is_pretty() {
if self.fields == 0 {
Expand All @@ -347,19 +359,31 @@ impl<'a, 'b: 'a> DebugTuple<'a, 'b> {
let mut slot = None;
let mut state = Default::default();
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut state);
value_fmt(&mut writer)?;
value.fmt(&mut writer)?;
writer.write_str(",\n")
} else {
let prefix = if self.fields == 0 { "(" } else { ", " };
self.fmt.write_str(prefix)?;
value_fmt(self.fmt)
value.fmt(self.fmt)
}
});

self.fields += 1;
self
}

/// Adds a new field to the generated tuple struct output.
///
/// This method is equivalent to [`DebugTuple::field`], but formats the
/// value using a provided closure rather than by calling [`Debug::fmt`].
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
pub fn field_with<F>(&mut self, value_fmt: F) -> &mut Self
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.field(&DebugOnce(Cell::new(Some(value_fmt))))
}

/// Marks the tuple struct as non-exhaustive, indicating to the reader that there are some
/// other fields that are not shown in the debug representation.
///
Expand Down Expand Up @@ -453,10 +477,7 @@ struct DebugInner<'a, 'b: 'a> {
}

impl<'a, 'b: 'a> DebugInner<'a, 'b> {
fn entry_with<F>(&mut self, entry_fmt: F)
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
fn entry(&mut self, entry: &dyn fmt::Debug) {
self.result = self.result.and_then(|_| {
if self.is_pretty() {
if !self.has_fields {
Expand All @@ -465,19 +486,26 @@ impl<'a, 'b: 'a> DebugInner<'a, 'b> {
let mut slot = None;
let mut state = Default::default();
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut state);
entry_fmt(&mut writer)?;
entry.fmt(&mut writer)?;
writer.write_str(",\n")
} else {
if self.has_fields {
self.fmt.write_str(", ")?
}
entry_fmt(self.fmt)
entry.fmt(self.fmt)
}
});

self.has_fields = true;
}

fn entry_with<F>(&mut self, entry_fmt: F)
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.entry(&DebugOnce(Cell::new(Some(entry_fmt))));
}

fn is_pretty(&self) -> bool {
self.fmt.alternate()
}
Expand Down Expand Up @@ -546,7 +574,7 @@ impl<'a, 'b: 'a> DebugSet<'a, 'b> {
/// ```
#[stable(feature = "debug_builders", since = "1.2.0")]
pub fn entry(&mut self, entry: &dyn fmt::Debug) -> &mut Self {
self.inner.entry_with(|f| entry.fmt(f));
self.inner.entry(entry);
self
}

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

Expand Down Expand Up @@ -969,18 +997,6 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
/// ```
#[stable(feature = "debug_map_key_value", since = "1.42.0")]
pub fn key(&mut self, key: &dyn fmt::Debug) -> &mut Self {
self.key_with(|f| key.fmt(f))
}

/// Adds the key part of a new entry to the map output.
///
/// This method is equivalent to [`DebugMap::key`], but formats the
/// key using a provided closure rather than by calling [`Debug::fmt`].
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
pub fn key_with<F>(&mut self, key_fmt: F) -> &mut Self
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.result = self.result.and_then(|_| {
assert!(
!self.has_key,
Expand All @@ -995,13 +1011,13 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
let mut slot = None;
self.state = Default::default();
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut self.state);
key_fmt(&mut writer)?;
key.fmt(&mut writer)?;
writer.write_str(": ")?;
} else {
if self.has_fields {
self.fmt.write_str(", ")?
}
key_fmt(self.fmt)?;
key.fmt(self.fmt)?;
self.fmt.write_str(": ")?;
}

Expand All @@ -1012,6 +1028,18 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
self
}

/// Adds the key part of a new entry to the map output.
///
/// This method is equivalent to [`DebugMap::key`], but formats the
/// key using a provided closure rather than by calling [`Debug::fmt`].
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
pub fn key_with<F>(&mut self, key_fmt: F) -> &mut Self
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.key(&DebugOnce(Cell::new(Some(key_fmt))))
}

/// Adds the value part of a new entry to the map output.
///
/// This method, together with `key`, is an alternative to `entry` that
Expand Down Expand Up @@ -1045,28 +1073,16 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> {
/// ```
#[stable(feature = "debug_map_key_value", since = "1.42.0")]
pub fn value(&mut self, value: &dyn fmt::Debug) -> &mut Self {
self.value_with(|f| value.fmt(f))
}

/// Adds the value part of a new entry to the map output.
///
/// This method is equivalent to [`DebugMap::value`], but formats the
/// value using a provided closure rather than by calling [`Debug::fmt`].
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
pub fn value_with<F>(&mut self, value_fmt: F) -> &mut Self
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.result = self.result.and_then(|_| {
assert!(self.has_key, "attempted to format a map value before its key");

if self.is_pretty() {
let mut slot = None;
let mut writer = PadAdapter::wrap(self.fmt, &mut slot, &mut self.state);
value_fmt(&mut writer)?;
value.fmt(&mut writer)?;
writer.write_str(",\n")?;
} else {
value_fmt(self.fmt)?;
value.fmt(self.fmt)?;
}

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

/// Adds the value part of a new entry to the map output.
///
/// This method is equivalent to [`DebugMap::value`], but formats the
/// value using a provided closure rather than by calling [`Debug::fmt`].
#[unstable(feature = "debug_closure_helpers", issue = "117729")]
pub fn value_with<F>(&mut self, value_fmt: F) -> &mut Self
where
F: FnOnce(&mut fmt::Formatter<'_>) -> fmt::Result,
{
self.value(&DebugOnce(Cell::new(Some(value_fmt))))
}

/// Adds the contents of an iterator of entries to the map output.
///
/// # Examples
Expand Down
Loading