[v1] rust: list: remove HasListLinks::OFFSET

[PATCH 3/5] rust: list: use consistent type parameter names

Posted by Tamir Duberstein 8 months, 3 weeks ago

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)*})?
        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

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Boqun Feng 8 months, 3 weeks ago

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
>

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Tamir Duberstein 8 months, 3 weeks ago

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.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Tamir Duberstein 8 months, 3 weeks ago

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.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Boqun Feng 8 months, 3 weeks ago

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.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Tamir Duberstein 8 months, 3 weeks ago

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.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Benno Lossin 8 months, 3 weeks ago

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

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Tamir Duberstein 8 months, 3 weeks ago

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.

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Benno Lossin 8 months, 3 weeks ago

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

Re: [PATCH 3/5] rust: list: use consistent type parameter names

Posted by Tamir Duberstein 8 months, 3 weeks ago

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 `<>`.