Closure-consuming helper functions for fmt::Debug helpers

[jmillikin]

ACP: rust-lang/libs-team#288

Tracking issue: #117729

[rustbot]

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

[jmillikin]

A quick note regarding FormatterFn:

With its current definition, the call site must provide a signature for the closure:

pub struct FormatterFn<F>(pub F);

let value = 'a';
let wrapped = fmt::FormatterFn(|f: &mut fmt::Formatter<'_>| {
    write!(f, "{:?}", &value)
});
assert_eq!(format!("{}", wrapped), "'a'");

If the type definition were changed to include a bound on F, then the call site can be simplified but attempts to reference FormatterFn itself as a type might become more verbose:

pub struct FormatterFn<F>(pub F)
where
    F: Fn(&mut fmt::Formatter<'_>) -> fmt::Result;

let value = 'a';
let wrapped = fmt::FormatterFn(|f| write!(f, "{:?}", &value));
assert_eq!(format!("{}", wrapped), "'a'");

Does anyone have thoughts on this?

[cuviper]

If the type definition were changed to include a bound on F, then the call site can be simplified but attempts to reference FormatterFn itself as a type might become more verbose:

Can you give an example of the latter kind of use that would become more verbose? It seems to me that either way, such uses are going to need to spell out the constraints in order to do anything interesting.

The local call site is probably the more important use to optimize for anyway.

[jmillikin]

Can you give an example of the latter kind of use that would become more verbose? It seems to me that either way, such uses are going to need to spell out the constraints in order to do anything interesting.

The local call site is probably the more important use to optimize for anyway.

After sleeping on it, I think my worry here was misplaced. You're correct that optimizing for call site readability is more important, and also there's an easy solution to what I was thinking of.

PR updated (diff)

---

Some time ago I implemented code that looked somewhat like this:

pub(crate) struct DebugHexU32(u32);

pub(crate) fn debug_hex_u32(value: u32) -> DebugAsHexU32 {
	DebugHexU32(value)
}

impl fmt::Debug for DebugHexU32 {
	fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
		// 8 hex digits + 2 for leading "0x".
		write!(fmt, "{:#010X}", self.0)
	}
}

I was imagining that the debug_hex_u32() return value might become complex, have a type bound, etc.

However, the actual way I'd implemented that code is with -> impl fmt::Debug, and the following snippet works just fine, so I have no concerns:

fn hex_u32(value: u32) -> impl fmt::Debug {
	FormatterFn(move |f| write!(f, "{:#010X}", value))
}

[cuviper]

Sounds good!

@bors r+

[bors]

📌 Commit 82a9f94 has been approved by cuviper

It is now in the queue for this repository.

[bors]

⌛ Testing commit 82a9f94 with merge cbc679c...

[rust-log-analyzer]

The job armhf-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[bors]

💔 Test failed - checks-actions

[cuviper]

Connection failure in remote-test-client...

@bors retry