| Message ID | 20250324-list-no-offset-v1-3-afd2b7fc442a@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | rust: list: remove HasListLinks::OFFSET | expand |
On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by > the same name used in `impl_list_item!`. > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > --- > rust/kernel/list/impl_list_item_mod.rs | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs > index 5ed66fdce953..9d2102138c48 100644 > --- a/rust/kernel/list/impl_list_item_mod.rs > +++ b/rust/kernel/list/impl_list_item_mod.rs > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> { > /// Implements the [`HasListLinks`] trait for the given type. > #[macro_export] > macro_rules! impl_has_list_links { > - ($(impl$(<$($implarg:ident),*>)? > + ($(impl$(<$($generics:ident),*>)? > HasListLinks$(<$id:tt>)? > for $self:ty > { self$(.$field:ident)* } > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links { > // > // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is > // equivalent to the pointer offset operation in the trait definition. > - unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > + unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize; > > #[inline] > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. > #[macro_export] > macro_rules! impl_has_list_links_self_ptr { > - ($(impl$({$($implarg:tt)*})? > + ($(impl$({$($generics:tt)*})? While you're at it, can you also change this to be ($(impl$(<$($generics:tt)*>)? ? I don't know why we chose <> for impl_has_list_links, but {} for impl_has_list_links_self_ptr ;-) Regards, Boqun > HasSelfPtr<$item_type:ty $(, $id:tt)?> > for $self:ty > { self.$field:ident } > )*) => {$( > // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the > // right type. > - unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {} > + unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {} > > - unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for $self { > + unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self { > const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize; > > #[inline] > > -- > 2.48.1 >
On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: > > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by > > the same name used in `impl_list_item!`. > > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > --- > > rust/kernel/list/impl_list_item_mod.rs | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs > > index 5ed66fdce953..9d2102138c48 100644 > > --- a/rust/kernel/list/impl_list_item_mod.rs > > +++ b/rust/kernel/list/impl_list_item_mod.rs > > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> { > > /// Implements the [`HasListLinks`] trait for the given type. > > #[macro_export] > > macro_rules! impl_has_list_links { > > - ($(impl$(<$($implarg:ident),*>)? > > + ($(impl$(<$($generics:ident),*>)? > > HasListLinks$(<$id:tt>)? > > for $self:ty > > { self$(.$field:ident)* } > > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links { > > // > > // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is > > // equivalent to the pointer offset operation in the trait definition. > > - unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > > + unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > > const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize; > > > > #[inline] > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> > > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. > > #[macro_export] > > macro_rules! impl_has_list_links_self_ptr { > > - ($(impl$({$($implarg:tt)*})? > > + ($(impl$({$($generics:tt)*})? > > While you're at it, can you also change this to be > > ($(impl$(<$($generics:tt)*>)? > > ? > > I don't know why we chose <> for impl_has_list_links, but {} for > impl_has_list_links_self_ptr ;-) This doesn't work in all cases: error: local ambiguity when calling macro `impl_has_work`: multiple parsing options: built-in NTs tt ('generics') or 1 other option. --> ../rust/kernel/workqueue.rs:522:11 | 522 | impl<T> HasWork<Self> for ClosureWork<T> { self.work } The reason that `impl_has_list_links` uses <> and all others use {} is that `impl_has_list_links` is the only one that captures the generic parameter as an `ident`, the rest use `tt`. So we could change `impl_has_list_links` to use {}, but not the other way around.
On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote: > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: > > > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by > > > the same name used in `impl_list_item!`. > > > > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > > --- > > > rust/kernel/list/impl_list_item_mod.rs | 10 +++++----- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs > > > index 5ed66fdce953..9d2102138c48 100644 > > > --- a/rust/kernel/list/impl_list_item_mod.rs > > > +++ b/rust/kernel/list/impl_list_item_mod.rs > > > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> { > > > /// Implements the [`HasListLinks`] trait for the given type. > > > #[macro_export] > > > macro_rules! impl_has_list_links { > > > - ($(impl$(<$($implarg:ident),*>)? > > > + ($(impl$(<$($generics:ident),*>)? > > > HasListLinks$(<$id:tt>)? > > > for $self:ty > > > { self$(.$field:ident)* } > > > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links { > > > // > > > // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is > > > // equivalent to the pointer offset operation in the trait definition. > > > - unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > > > + unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > > > const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize; > > > > > > #[inline] > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> > > > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. > > > #[macro_export] > > > macro_rules! impl_has_list_links_self_ptr { > > > - ($(impl$({$($implarg:tt)*})? > > > + ($(impl$({$($generics:tt)*})? > > > > While you're at it, can you also change this to be > > > > ($(impl$(<$($generics:tt)*>)? > > > > ? > > > > I don't know why we chose <> for impl_has_list_links, but {} for > > impl_has_list_links_self_ptr ;-) > > This doesn't work in all cases: > > error: local ambiguity when calling macro `impl_has_work`: multiple > parsing options: built-in NTs tt ('generics') or 1 other option. > --> ../rust/kernel/workqueue.rs:522:11 > | > 522 | impl<T> HasWork<Self> for ClosureWork<T> { self.work } > > The reason that `impl_has_list_links` uses <> and all others use {} is > that `impl_has_list_links` is the only one that captures the generic > parameter as an `ident`, the rest use `tt`. So we could change > `impl_has_list_links` to use {}, but not the other way around. I've changed it to `{}` so it's consistent everywhere.
On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote: > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: > > > > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by > > > > the same name used in `impl_list_item!`. > > > > > > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > > > --- > > > > rust/kernel/list/impl_list_item_mod.rs | 10 +++++----- > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs > > > > index 5ed66fdce953..9d2102138c48 100644 > > > > --- a/rust/kernel/list/impl_list_item_mod.rs > > > > +++ b/rust/kernel/list/impl_list_item_mod.rs > > > > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> { > > > > /// Implements the [`HasListLinks`] trait for the given type. > > > > #[macro_export] > > > > macro_rules! impl_has_list_links { > > > > - ($(impl$(<$($implarg:ident),*>)? > > > > + ($(impl$(<$($generics:ident),*>)? > > > > HasListLinks$(<$id:tt>)? > > > > for $self:ty > > > > { self$(.$field:ident)* } > > > > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links { > > > > // > > > > // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is > > > > // equivalent to the pointer offset operation in the trait definition. > > > > - unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > > > > + unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > > > > const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize; > > > > > > > > #[inline] > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> > > > > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. > > > > #[macro_export] > > > > macro_rules! impl_has_list_links_self_ptr { > > > > - ($(impl$({$($implarg:tt)*})? > > > > + ($(impl$({$($generics:tt)*})? > > > > > > While you're at it, can you also change this to be > > > > > > ($(impl$(<$($generics:tt)*>)? > > > > > > ? > > > > > > I don't know why we chose <> for impl_has_list_links, but {} for > > > impl_has_list_links_self_ptr ;-) > > > > This doesn't work in all cases: > > > > error: local ambiguity when calling macro `impl_has_work`: multiple > > parsing options: built-in NTs tt ('generics') or 1 other option. > > --> ../rust/kernel/workqueue.rs:522:11 > > | > > 522 | impl<T> HasWork<Self> for ClosureWork<T> { self.work } > > > > The reason that `impl_has_list_links` uses <> and all others use {} is > > that `impl_has_list_links` is the only one that captures the generic > > parameter as an `ident`, the rest use `tt`. So we could change Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm a bit curious. Regards, Boqun > > `impl_has_list_links` to use {}, but not the other way around. > > I've changed it to `{}` so it's consistent everywhere.
On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote: > > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: > > > > > Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by > > > > > the same name used in `impl_list_item!`. > > > > > > > > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > > > > --- > > > > > rust/kernel/list/impl_list_item_mod.rs | 10 +++++----- > > > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > > > > > diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs > > > > > index 5ed66fdce953..9d2102138c48 100644 > > > > > --- a/rust/kernel/list/impl_list_item_mod.rs > > > > > +++ b/rust/kernel/list/impl_list_item_mod.rs > > > > > @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> { > > > > > /// Implements the [`HasListLinks`] trait for the given type. > > > > > #[macro_export] > > > > > macro_rules! impl_has_list_links { > > > > > - ($(impl$(<$($implarg:ident),*>)? > > > > > + ($(impl$(<$($generics:ident),*>)? > > > > > HasListLinks$(<$id:tt>)? > > > > > for $self:ty > > > > > { self$(.$field:ident)* } > > > > > @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links { > > > > > // > > > > > // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is > > > > > // equivalent to the pointer offset operation in the trait definition. > > > > > - unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > > > > > + unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self { > > > > > const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize; > > > > > > > > > > #[inline] > > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> > > > > > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. > > > > > #[macro_export] > > > > > macro_rules! impl_has_list_links_self_ptr { > > > > > - ($(impl$({$($implarg:tt)*})? > > > > > + ($(impl$({$($generics:tt)*})? > > > > > > > > While you're at it, can you also change this to be > > > > > > > > ($(impl$(<$($generics:tt)*>)? > > > > > > > > ? > > > > > > > > I don't know why we chose <> for impl_has_list_links, but {} for > > > > impl_has_list_links_self_ptr ;-) > > > > > > This doesn't work in all cases: > > > > > > error: local ambiguity when calling macro `impl_has_work`: multiple > > > parsing options: built-in NTs tt ('generics') or 1 other option. > > > --> ../rust/kernel/workqueue.rs:522:11 > > > | > > > 522 | impl<T> HasWork<Self> for ClosureWork<T> { self.work } > > > > > > The reason that `impl_has_list_links` uses <> and all others use {} is > > > that `impl_has_list_links` is the only one that captures the generic > > > parameter as an `ident`, the rest use `tt`. So we could change > > Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm > a bit curious. I think it's because `ident` cannot deal with lifetimes or const generics - or at least I was not able to make it work with them.
On Tue Mar 25, 2025 at 10:52 AM CET, Tamir Duberstein wrote: > On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote: >> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote: >> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote: >> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote: >> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: >> > > > > #[inline] >> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> >> > > > > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. >> > > > > #[macro_export] >> > > > > macro_rules! impl_has_list_links_self_ptr { >> > > > > - ($(impl$({$($implarg:tt)*})? >> > > > > + ($(impl$({$($generics:tt)*})? >> > > > >> > > > While you're at it, can you also change this to be >> > > > >> > > > ($(impl$(<$($generics:tt)*>)? >> > > > >> > > > ? >> > > > >> > > > I don't know why we chose <> for impl_has_list_links, but {} for >> > > > impl_has_list_links_self_ptr ;-) >> > > >> > > This doesn't work in all cases: >> > > >> > > error: local ambiguity when calling macro `impl_has_work`: multiple >> > > parsing options: built-in NTs tt ('generics') or 1 other option. >> > > --> ../rust/kernel/workqueue.rs:522:11 >> > > | >> > > 522 | impl<T> HasWork<Self> for ClosureWork<T> { self.work } >> > > >> > > The reason that `impl_has_list_links` uses <> and all others use {} is >> > > that `impl_has_list_links` is the only one that captures the generic >> > > parameter as an `ident`, the rest use `tt`. So we could change >> >> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm >> a bit curious. > > I think it's because `ident` cannot deal with lifetimes or const > generics - or at least I was not able to make it work with them. If you use `ident`, you can use the normal `<>` as the delimiters of generics. For `tt`, you have to use `{}` (or `()`/`[]`). --- Cheers, Benno
On Tue, Mar 25, 2025 at 6:37 AM Benno Lossin <benno.lossin@proton.me> wrote: > > On Tue Mar 25, 2025 at 10:52 AM CET, Tamir Duberstein wrote: > > On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote: > >> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote: > >> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote: > >> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote: > >> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: > >> > > > > #[inline] > >> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> > >> > > > > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. > >> > > > > #[macro_export] > >> > > > > macro_rules! impl_has_list_links_self_ptr { > >> > > > > - ($(impl$({$($implarg:tt)*})? > >> > > > > + ($(impl$({$($generics:tt)*})? > >> > > > > >> > > > While you're at it, can you also change this to be > >> > > > > >> > > > ($(impl$(<$($generics:tt)*>)? > >> > > > > >> > > > ? > >> > > > > >> > > > I don't know why we chose <> for impl_has_list_links, but {} for > >> > > > impl_has_list_links_self_ptr ;-) > >> > > > >> > > This doesn't work in all cases: > >> > > > >> > > error: local ambiguity when calling macro `impl_has_work`: multiple > >> > > parsing options: built-in NTs tt ('generics') or 1 other option. > >> > > --> ../rust/kernel/workqueue.rs:522:11 > >> > > | > >> > > 522 | impl<T> HasWork<Self> for ClosureWork<T> { self.work } > >> > > > >> > > The reason that `impl_has_list_links` uses <> and all others use {} is > >> > > that `impl_has_list_links` is the only one that captures the generic > >> > > parameter as an `ident`, the rest use `tt`. So we could change > >> > >> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm > >> a bit curious. > > > > I think it's because `ident` cannot deal with lifetimes or const > > generics - or at least I was not able to make it work with them. > > If you use `ident`, you can use the normal `<>` as the delimiters of > generics. For `tt`, you have to use `{}` (or `()`/`[]`). Yes I know. But with `ident` you cannot capture lifetimes or const generics.
On Tue Mar 25, 2025 at 11:42 AM CET, Tamir Duberstein wrote: > On Tue, Mar 25, 2025 at 6:37 AM Benno Lossin <benno.lossin@proton.me> wrote: >> On Tue Mar 25, 2025 at 10:52 AM CET, Tamir Duberstein wrote: >> > On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote: >> >> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote: >> >> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote: >> >> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote: >> >> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: >> >> > > > > #[inline] >> >> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> >> >> > > > > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. >> >> > > > > #[macro_export] >> >> > > > > macro_rules! impl_has_list_links_self_ptr { >> >> > > > > - ($(impl$({$($implarg:tt)*})? >> >> > > > > + ($(impl$({$($generics:tt)*})? >> >> > > > >> >> > > > While you're at it, can you also change this to be >> >> > > > >> >> > > > ($(impl$(<$($generics:tt)*>)? >> >> > > > >> >> > > > ? >> >> > > > >> >> > > > I don't know why we chose <> for impl_has_list_links, but {} for >> >> > > > impl_has_list_links_self_ptr ;-) >> >> > > >> >> > > This doesn't work in all cases: >> >> > > >> >> > > error: local ambiguity when calling macro `impl_has_work`: multiple >> >> > > parsing options: built-in NTs tt ('generics') or 1 other option. >> >> > > --> ../rust/kernel/workqueue.rs:522:11 >> >> > > | >> >> > > 522 | impl<T> HasWork<Self> for ClosureWork<T> { self.work } >> >> > > >> >> > > The reason that `impl_has_list_links` uses <> and all others use {} is >> >> > > that `impl_has_list_links` is the only one that captures the generic >> >> > > parameter as an `ident`, the rest use `tt`. So we could change >> >> >> >> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm >> >> a bit curious. >> > >> > I think it's because `ident` cannot deal with lifetimes or const >> > generics - or at least I was not able to make it work with them. >> >> If you use `ident`, you can use the normal `<>` as the delimiters of >> generics. For `tt`, you have to use `{}` (or `()`/`[]`). > > Yes I know. But with `ident` you cannot capture lifetimes or const generics. Why is that required for this macro? I think we could use `tt`. --- Cheers, Benno
On Tue, Mar 25, 2025 at 7:18 AM Benno Lossin <benno.lossin@proton.me> wrote: > > On Tue Mar 25, 2025 at 11:42 AM CET, Tamir Duberstein wrote: > > On Tue, Mar 25, 2025 at 6:37 AM Benno Lossin <benno.lossin@proton.me> wrote: > >> On Tue Mar 25, 2025 at 10:52 AM CET, Tamir Duberstein wrote: > >> > On Tue, Mar 25, 2025 at 12:02 AM Boqun Feng <boqun.feng@gmail.com> wrote: > >> >> On Mon, Mar 24, 2025 at 05:56:57PM -0400, Tamir Duberstein wrote: > >> >> > On Mon, Mar 24, 2025 at 5:51 PM Tamir Duberstein <tamird@gmail.com> wrote: > >> >> > > On Mon, Mar 24, 2025 at 5:42 PM Boqun Feng <boqun.feng@gmail.com> wrote: > >> >> > > > On Mon, Mar 24, 2025 at 05:33:45PM -0400, Tamir Duberstein wrote: > >> >> > > > > #[inline] > >> >> > > > > @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> > >> >> > > > > /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. > >> >> > > > > #[macro_export] > >> >> > > > > macro_rules! impl_has_list_links_self_ptr { > >> >> > > > > - ($(impl$({$($implarg:tt)*})? > >> >> > > > > + ($(impl$({$($generics:tt)*})? > >> >> > > > > >> >> > > > While you're at it, can you also change this to be > >> >> > > > > >> >> > > > ($(impl$(<$($generics:tt)*>)? > >> >> > > > > >> >> > > > ? > >> >> > > > > >> >> > > > I don't know why we chose <> for impl_has_list_links, but {} for > >> >> > > > impl_has_list_links_self_ptr ;-) > >> >> > > > >> >> > > This doesn't work in all cases: > >> >> > > > >> >> > > error: local ambiguity when calling macro `impl_has_work`: multiple > >> >> > > parsing options: built-in NTs tt ('generics') or 1 other option. > >> >> > > --> ../rust/kernel/workqueue.rs:522:11 > >> >> > > | > >> >> > > 522 | impl<T> HasWork<Self> for ClosureWork<T> { self.work } > >> >> > > > >> >> > > The reason that `impl_has_list_links` uses <> and all others use {} is > >> >> > > that `impl_has_list_links` is the only one that captures the generic > >> >> > > parameter as an `ident`, the rest use `tt`. So we could change > >> >> > >> >> Why impl_has_list_links uses generics at `ident` but rest use `tt`? I'm > >> >> a bit curious. > >> > > >> > I think it's because `ident` cannot deal with lifetimes or const > >> > generics - or at least I was not able to make it work with them. > >> > >> If you use `ident`, you can use the normal `<>` as the delimiters of > >> generics. For `tt`, you have to use `{}` (or `()`/`[]`). > > > > Yes I know. But with `ident` you cannot capture lifetimes or const generics. > > Why is that required for this macro? I think we could use `tt`. We're in violent agreement. The change I've made is to use `tt` everywhere, including where `ident` is currently used with `<>`.
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs index 5ed66fdce953..9d2102138c48 100644 --- a/rust/kernel/list/impl_list_item_mod.rs +++ b/rust/kernel/list/impl_list_item_mod.rs @@ -41,7 +41,7 @@ unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> { /// Implements the [`HasListLinks`] trait for the given type. #[macro_export] macro_rules! impl_has_list_links { - ($(impl$(<$($implarg:ident),*>)? + ($(impl$(<$($generics:ident),*>)? HasListLinks$(<$id:tt>)? for $self:ty { self$(.$field:ident)* } @@ -51,7 +51,7 @@ macro_rules! impl_has_list_links { // // The behavior of `raw_get_list_links` is not changed since the `addr_of_mut!` macro is // equivalent to the pointer offset operation in the trait definition. - unsafe impl$(<$($implarg),*>)? $crate::list::HasListLinks$(<$id>)? for $self { + unsafe impl$(<$($generics),*>)? $crate::list::HasListLinks$(<$id>)? for $self { const OFFSET: usize = ::core::mem::offset_of!(Self, $($field).*) as usize; #[inline] @@ -81,16 +81,16 @@ pub unsafe trait HasSelfPtr<T: ?Sized, const ID: u64 = 0> /// Implements the [`HasListLinks`] and [`HasSelfPtr`] traits for the given type. #[macro_export] macro_rules! impl_has_list_links_self_ptr { - ($(impl$({$($implarg:tt)*})? + ($(impl$({$($generics:tt)*})? HasSelfPtr<$item_type:ty $(, $id:tt)?> for $self:ty { self.$field:ident } )*) => {$( // SAFETY: The implementation of `raw_get_list_links` only compiles if the field has the // right type. - unsafe impl$(<$($implarg)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {} + unsafe impl$(<$($generics)*>)? $crate::list::HasSelfPtr<$item_type $(, $id)?> for $self {} - unsafe impl$(<$($implarg)*>)? $crate::list::HasListLinks$(<$id>)? for $self { + unsafe impl$(<$($generics)*>)? $crate::list::HasListLinks$(<$id>)? for $self { const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize; #[inline]
Refer to the type parameters of `impl_has_list_links{,_self_ptr}!` by the same name used in `impl_list_item!`. Signed-off-by: Tamir Duberstein <tamird@gmail.com> --- rust/kernel/list/impl_list_item_mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)