[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260105142733.3d4ecfde.gary@garyguo.net>
Date: Mon, 5 Jan 2026 14:27:33 +0000
From: Gary Guo <gary@...yguo.net>
To: Tamir Duberstein <tamird@...il.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>, Benno Lossin
<lossin@...nel.org>, Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl
<aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich
<dakr@...nel.org>, Guilherme Giacomo Simoes <trintaeoitogc@...il.com>,
José Expósito <jose.exposito89@...il.com>,
rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/11] rust: macros: convert `#[vtable]` macro to use
`syn`
On Mon, 5 Jan 2026 06:02:35 -0500
Tamir Duberstein <tamird@...il.com> wrote:
> On Sun, Jan 4, 2026 at 9:18 PM Gary Guo <gary@...yguo.net> wrote:
> >
> > On Sun, 4 Jan 2026 18:44:43 -0500
> > Tamir Duberstein <tamird@...il.com> wrote:
> >
> > > On Thu, Dec 11, 2025 at 2:29 PM Gary Guo <gary@...nel.org> wrote:
> > > >
> > > > From: Gary Guo <gary@...yguo.net>
> > > >
> > > > `#[vtable]` is converted to use syn. This is more robust than the
> > > > previous heuristic-based searching of defined methods and functions.
> > > >
> > > > When doing so, the trait and impl are split into two code paths as the
> > > > types are distinct when parsed by `syn`.
> > > >
> > > > Signed-off-by: Gary Guo <gary@...yguo.net>
> > >
> > > Logic looks correct, but the duplication between handle_trait and
> > > handle_impl is unfortunate. I golfed on this a bit, see if you like
> > > it: https://github.com/tamird/linux/commit/8354c5a48769f5e1e52963d19ca57c31e5926b08.
> >
> > I very much prefer the code to be separate. The trait and impl *should*
> > be different. It's just that they *look* similar.
> >
> > Defining
> >
> > const HAS_FOO: bool = false;
> >
> > in trait is defining a new contract while providing *default* value,
> > while
> >
> > const HAS_FOO: bool = true;
> >
> > is implementing such contract with a specific value. They look the
> > same but I think the way `syn` treats them differently is justified.
>
> Yes, the similarity is perhaps superficial, but the duplication in the
> current patch goes a good bit further because all the surrounding
> ceremony is also duplicated.
What do you mean? The only code that is really duplicated is the gathering
of names from items (which have to be duplicate due to type difference
anyway, and they're still duplicate in your linked commit above.
>
> >
> > I think the fact that existing code has a boolean and do different
> > things based on it is a good enough supporting reason to handle
> > different code path.
> >
> > For some new `vtable` features that I am working on would require quite
> > different impl between the two.
>
> It seems unusual to justify current changes with future changes.
I gave both reasons that why I think it should be distinct code path today
and also why I prefer it to be it for the future. I did not justify the
current change with future changes.
Powered by blists - more mailing lists