Customizing #[derive(Debug)]

[clubby789]

Proposal

Problem statement

The current behaviour of the #[derive(Debug)] macro can be overly broad and not work for some use cases. Users with types that do not fit the derive are forced to either write verbose custom implementations (which they must remember to update when the type is updated), or a 3rd-party derive. These approaches both mean missing out on upstream optimizations to the macro.

Motivating examples or use cases

#[derive(Debug)]
struct FunctionBuilder<'cx> {
  cx: &'cx SomeVeryLargeContextType,  // Required for the struct, but unecessarily bloats debug output
  name: String,
  instructions: Vec<Instruction>,
}

#[derive(Debug)]
struct MyListForTraitImpls<T>(Vec<T>); // Should be displayed as a list of elements like `Vec`, but includes the `MyListForTraitImpls` type name

#[derive(Debug)]
struct CallbackRunner {
	f: Box<dyn Fn() -> usize>, // We don't want this displayed, but it will fail to compile as `dyn Fn() -> usize` is not `Debug`
	name: &'static str,
}

Solution sketch

Introducing a new #[debug(..)] attribute which can be applied to fields or types to customize the behavior of the derive.

skip

#[debug(skip)] on a field will cause it to not be emitted, and not require the type of that field to implement Debug.

Ideally,

#[derive(Debug)]
struct Xxx<T> {
	#[debug(skip)]
	something: T,
    /* other fields that don't use T */
}

should not place a : Debug bound on T.

transparent

Similarly to repr(transparent), #[debug(transparent)] may be placed on any type containing a single field to use the debug implementation of that field directly. The macro should expand to something like

/* ... */
Debug::fmt(self.0)
/* ... */

Other ideas

Other attributes that I am less commited to, but could be of use:

• #[debug(type)] - expands to the std::any::type_name of the field - useful for function pointers to see the source location
• #[debug(format_with = "path")] - a more general version of type, allowing a custom function to be used to format the field
• #[debug(rename = "identifier")] - changes the name of a field in the debug output

Alternatives

The main alternative is using a 3rd-party crate such as derivative. The reasoning against this is explained in the problem statement.

Links and related work

Derivative - One of the top 350 most downloaded crates on crates.io.

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

Internals thread

[joshtriplett]

Copying a suggestion from the internals thread:

If skip is added, it should probably be spelled #[skip(Debug)] and be applicable to Eq, Ord and other "field-wise derivable" traits as well.

[clubby789]

I went with #[debug] because of the precedent with #[default], but I think there's definitely a good case for that ordering if skip is extended to other derives.

[jhpratt]

Procedurally, is an ACP sufficient? This feels like a relatively large change.

[Veykril]

Copying a suggestion from the internals thread:

If skip is added, it should probably be spelled #[skip(Debug)] and be applicable to Eq, Ord and other "field-wise derivable" traits as well.

Note that the choice here has certain implications for the ecosystem. Going with a skip derive helper would imply that third-party derives should likely adopt this form as well over them using their own#[my_derive(skip)] helper.

[clubby789]

Procedurally, is an ACP sufficient? This feels like a relatively large change.

I've added a link to the Pre-RFC I made on IRLO; Josh suggested filing an ACP so it could be nominated for discussion.

[joshtriplett]

@jhpratt I think an ACP is sufficient; this seems like a small change, which would be a relatively small PR, and the primary challenge is deciding "should we do this", which is what we have ACPs for.

[scottmcm]

Copying from IRLO:

jdahlstrom: If skip is added, it should probably be spelled #[skip(Debug)]

TBH, I prefer this phrasing. It makes the proposal "add a common skip attribute" instead of "add a bunch of customization knobs".

Then #[skip(Hash)] works for fields not worth hashing, and the compiler can do things like deny if you derive(PartialEq, Hash) but have skip(PartialEq), since that means that the Hash implementation is just wrong.

---

But that direction would implicitly be a "also, ecosystem, maybe you should support skip like this too", at which point an RFC becomes more useful.

[dvdsk]

I like the idea of a common skip attribute.

I am not sure about transparent for Debug. I like the Debug fmt to show the 'ugly' truth. In the mentioned use case it saves two lines. It might also look cleaner, but for a clean look shouldn't we rely on Display?

A similar concern around skipping. That is easily resolved by still showing the name of the field.

[joshtriplett]

We discussed this in today's @rust-lang/libs-api meeting.

We'd like to go ahead and approve #[skip(...)] to skip a field for field-wise derives.

We also felt that there should be some syntax to skip a field for all field-wise derives, without having to repeat the whole list. #[skip] could do that.

Something, probably rustc, needs to lint against un-handled arguments for skip, such as skip(Degub).

And it would probably also make sense to have a lint for the cases scottmcm mentioned: skipping a field for a subset of derives that seems unlikely to make sense, such as deriving more than one of Hash/PartialOrd/PartialEq` but skipping a field for some-but-not-all of those.

[cuviper]

How do you know it's unhandled if a 3rd party derive might use it? Or is the assertion that #[skip(_)] is only going to be allowed for builtins?

[joshtriplett]

@cuviper If skip is going to be shared across derives, one potential way would be to assume that the only arguments skip takes are the names of traits, so if it has an argument that's not the name of a trait being derived, lint.

Making it a lint and not an error also makes it easy enough to disable.

[fmease]

Technically speaking this is a breaking change though isn't it? If a pre-existing crate exposes a derive macro with a helper attribute skip and errors on #[skip(Debug)] for example, existing #[derive(Debug, Custom)] would suddenly break. Not sure if this would be the fault of the 3rd-party library?

I guess the same could've been said about Default's #[default].

Edit: Ah, furthermore, the feature gate error alone can also break existing packages.

[clubby789]

I'm currently implementing the unsupported skip argument as a lint so it can be disabled if needed (although right now there seem to be issues with that).
As for feature gating, not sure how to handle that 😕 A crater run might be a good idea. Seems like there was some discussion about this on a previous similar proposal https://internals.rust-lang.org/t/add-skip-attribute-to-various-derives/17411/

[fmease]

Revisiting this I think we should be more careful with introducing more and more unnamespaced helper attributes. If we continue this trend (#[default], #[skip], etc.) we end up digging our own grave imo.

As the last comment from the linked IRLO thread mentions, ideally we would have better language support for this: Qualified helper attributes or better yet fully hygienic helper attributes (*). This comes close to a pre-RFC and blocking #[skip] on this would probably delay it close to indefinitely.

(*) If an (attr or derive) proc macro declares a helper attribute helper, no other proc macros will be able to see it inside the token stream they receive. If multiple proc macros declare the same helper attribute helper and the user tries to use it we will reject it as ambiguous and force them to qualify it. E.g., as #[a::helper] / #[a::helper] in the case of attr proc macros and as #[derive::A::helper] / #[derive::B::helper] in the case of derive proc macros, modulo syntax bikeshedding. Skipping further details.

Maybe we should nominate this for T-lang discussion.

I still plan on reviewing the implementation of #[skip] (rust-lang/rust#121053) given I'm assigned to it but personally speaking I'd rather we follow a more principled approach. Not that I necessarily have a say in this, I'm but a member of T-compiler-contributors.

[fmease]

cc @petrochenkov (you might be interested in this topic)

[kennytm]

#[skip] is very different from #[default] though.

#[skip] is basically agnostic from the trait being derived (you write #[skip(Debug)] not #[debug(skip)]), while #[default] can only make sense in the Default trait. The "helper attribute" concern should block #[default] and #[debug] and #[partial_ord] etc. but not #[skip].

Or that #[skip] should be entirely removed in favor of the trait-specific #[default(skip)] and #[debug(skip)] and #[partial_ord(skip)] etc.

[dead-claudia]

Curious question: how should third party libraries integrate with that #[skip(...)]? It's reasonable for a library/framework like Serde to also want to do things like #[skip(Foo)] for some arbitrary derive trait Foo - that library itself already uses #[serde(skip)], #[serde(skip_serializing)], and #[serde(skip_deserializing)] for what's essentially #[skip(Serialize, Deserialize)], #[skip(Serialize)], and #[skip(Deserialize)].

[kennytm]

I think the field that is #[skip]'ed should be entirely deleted from the TokenStream sent to the proc macro. However currently no similar treatment applied to proc macro: all attributes, even helpers from other macros, are visible as-is. To be consistent with the current behavior I'm afraid the custom proc-macro needs to handle the #[skip] themselves.

Ideally,

1. helper attributes that are known to belong to other derive macros should be hidden away from the proc_macro_derive
2. skipped fields should be removed, and if the field survives the #[skip] attribute itself should be hidden away from the proc_macro_derive (similar to #[cfg] and #[cfg_attr])

but it may be too late to change (1) now.

Proof of concept

extern crate proc_macro;
use proc_macro::TokenStream;

#[proc_macro_derive(First, attributes(first_a, first_b, common))]
pub fn derive_first(item: TokenStream) -> TokenStream {
    println!("derive(First): {item}");
    TokenStream::new()
}
#[proc_macro_derive(Second, attributes(second_a, second_b, common))]
pub fn derive_second(item: TokenStream) -> TokenStream {
    println!("derive(Second): {item}");
    TokenStream::new()
}

#[derive(Default, First, Second)]
pub struct F {
    /// documentation
    #[rustfmt::skip]
    #[first_a]
    pub a1: u8,

    #[allow(unknown_lints)]
    #[second_a]
    pub a2: u8,

    #[cfg_attr(any(), first_a)]
    #[cfg_attr(all(), second_a)]
    pub a3: u8,

    #[first_b]
    #[second_b]
    pub b: u8,

    #[common]
    pub c: u8,

    #[cfg(any())]
    pub d: u8,

    #[skip(First)]
    #[first_a]
    pub skip_first: u8,

    #[skip(Second)]
    #[first_a]
    pub skip_second: u8,

    #[skip]
    #[first_a]
    pub skip_all: u8,
}

Current output is something like:

derive(First): pub struct F
{
    #[doc = "documentation"] #[rustfmt :: skip] #[first_a] pub a1 : u8, 
    #[allow(unknown_lints)] #[first_a] pub a2 : u8, 
    #[second_a] pub a3 : u8, 
    #[first_b] #[second_b] pub b : u8, 
    #[common] pub c : u8, 
    #[skip(First)] #[first_a] pub skip_first : u8,
    #[skip(Second)] #[first_a] pub skip_second : u8, 
    #[skip] #[first_a] pub skip_all : u8,
}
derive(Second): pub struct F
{
    #[doc = "documentation"] #[rustfmt :: skip] #[first_a] pub a1 : u8, 
    #[allow(unknown_lints)] #[second_a] pub a2 : u8, 
    #[second_a] pub a3 : u8, 
    #[first_b] #[second_b] pub b : u8, 
    #[common] pub c : u8, 
    #[skip(First)] #[first_a] pub skip_first : u8,
    #[skip(Second)] #[first_a] pub skip_second : u8, 
    #[skip] #[first_a] pub skip_all : u8,
}

Ideal output:

derive(First): pub struct F
{
    #[doc = "documentation"] #[rustfmt :: skip] #[first_a] pub a1 : u8, 
    #[allow(unknown_lints)] pub a2 : u8, 
    pub a3 : u8, 
    #[first_b] pub b : u8, 
    #[common] pub c : u8, 
    #[first_a] pub skip_second : u8, 
}
derive(Second): pub struct F
{
    #[doc = "documentation"] #[rustfmt :: skip] pub a1 : u8, 
    #[allow(unknown_lints)] #[second_a] pub a2 : u8, 
    #[second_a] pub a3 : u8, 
    #[second_b] pub b : u8, 
    #[common] pub c : u8, 
    pub skip_first : u8,
}

This is also the problem that for trait that is going to construct an object like Default, if you #[skip] a field to make it invisible from the proc macro, it becomes impossible to implement that trait.

#[derive(Default)]
struct G {
    #[skip(Default)]
    pub a: u8,
}
// This will generate:
impl Default for G {
    fn default() -> Self {
        Self {
        } // which is a compilation error because the constructor must fill in all fields including `a`!
    }
}

[fmease]

#334 (comment):

Ah, furthermore, the feature gate error alone can also break existing packages.

To give an update (at long last), the crater report did confirm my suspicion. We have 4 confirmed root regressions: [1], [2], [3], [4], [4.1], [4.2].

As such and for the other reasons I gave above, I deem the feature as currently designed unacceptable. I'm willing to work with the author @clubby789 on alternative designs if they agree to it like ones based on "hygienic" helper attributes. In any case, I'm going to submit a T-lang meeting proposal at the end of July — latest — hopefully containing an actionable solution sketch.

[joshtriplett]

Given the crater results showing compatibility hazards, it sounds like this is going to need a design and lang RFC for an approach that avoids those. Happy to work with anyone looking to write one. @fmease?

[fmease]

@joshtriplett Happy to write one this week and open a meeting proposal over at rust-lang/lang-team.