public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Benno Lossin" <lossin@kernel.org>
To: "Danilo Krummrich" <dakr@kernel.org>
Cc: "Maurice Hieronymus" <mhi@mailbox.org>, <aliceryhl@google.com>,
	<acourbot@nvidia.com>, <airlied@gmail.com>, <simona@ffwll.ch>,
	<nouveau@lists.freedesktop.org>,
	<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
	<ojeda@kernel.org>, <boqun.feng@gmail.com>, <gary@garyguo.net>,
	<bjorn3_gh@protonmail.com>, <a.hindborg@kernel.org>,
	<tmgross@umich.edu>, <rust-for-linux@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] rust: macros: Add derive Display for enums
Date: Mon, 05 Jan 2026 15:42:50 +0100	[thread overview]
Message-ID: <DFGQH1FOS007.3IG8XIBOBWTZI@kernel.org> (raw)
In-Reply-To: <DFGL2QTNH7FE.93EN71L7BXFM@kernel.org>

On Mon Jan 5, 2026 at 11:29 AM CET, Danilo Krummrich wrote:
> On Mon Jan 5, 2026 at 10:02 AM CET, Benno Lossin wrote:
>> On Sun Jan 4, 2026 at 9:07 PM CET, Maurice Hieronymus wrote:
>>> Add a derive macro that implements kernel::fmt::Display for enums.
>>> The macro outputs the exact variant name as written, preserving case.
>>>
>>> This supports all enum variant types: unit, tuple, and struct variants.
>>> For variants with data, only the variant name is displayed.
>>
>> I don't think we should be adding this. Display is designed for
>> user-facing output and so it should always be carefully designed and no
>> automation should exist for it.
>
> In general I agree, but simple stringification of an enum variant for a Display
> implementation is a very common use-case and it seems pretty unfortunate to have
> to fall back to either do the below (especially if there are a lot of enum
> variants) or having to go the declarative path of doing something as in [1].
>
> Especially in combination with things like FromPrimitive and ToPrimitive it gets
> us rid of the cases where we need such declarative macro mess.
>
> Eventually, drivers will most likely implement their own proc macro for this or
> repeat the declarative macro pattern over and over again.

When the definition already uses declarative macros, adding the Display
impl there is the correct way to do it IMO. If it is just a normal
definition, then a match is annoying when you have many variants.

> Maybe we should just pick a more specific name for such a derive macro than
> macros::Display.
>
> Maybe something along the lines of macros::EnumVariantDisplay? We could also
> have an optional argument indicating whether it should be converted to lower /
> upper case.

I'm still skeptical about having a derive macro for `Display`. What
about adding & deriving the following trait instead:

    pub trait EnumVariantName {
        fn variant_name(&self) -> &'static str;
    }

To print this, you of course need to call the function, but it is much
more self-descriptive than printing the `Chipset` directly.

Cheers,
Benno

  reply	other threads:[~2026-01-05 14:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-04 20:07 [PATCH v2 0/2] rust: macros: Add derive Display for enums Maurice Hieronymus
2026-01-04 20:07 ` [PATCH v2 1/2] " Maurice Hieronymus
2026-01-05  9:02   ` Benno Lossin
2026-01-05 10:29     ` Danilo Krummrich
2026-01-05 14:42       ` Benno Lossin [this message]
2026-01-05 15:00         ` Danilo Krummrich
2026-01-05 15:23           ` Maurice Hieronymus
2026-01-05 16:11       ` Gary Guo
2026-01-05 21:11         ` Maurice Hieronymus
2026-01-05 22:03           ` Danilo Krummrich
2026-01-06  5:56             ` Maurice Hieronymus
2026-01-06 12:56               ` Benno Lossin
2026-01-04 20:07 ` [PATCH v2 2/2] gpu: nova-core: Use derive Display for Chipset enum Maurice Hieronymus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DFGQH1FOS007.3IG8XIBOBWTZI@kernel.org \
    --to=lossin@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhi@mailbox.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox