[PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[Roland Xu]

Make the impl_has_work macro compatible with more complex generics such as lifetimes and const generic arguments.
See more in https://github.com/Rust-for-Linux/linux/issues/1077

Signed-off-by: Roland Xu <mu001999@outlook.com>
---
 rust/kernel/workqueue.rs | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 1cec63a2aea8..1ff81d88b61d 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -482,24 +482,25 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
 /// use kernel::sync::Arc;
 /// use kernel::workqueue::{self, impl_has_work, Work};
 ///
-/// struct MyStruct {
-///     work_field: Work<MyStruct, 17>,
+/// struct MyStruct<'a, T, const N: usize> {
+///     work_field: Work<MyStruct<'a, T, N>, 17>,
+///     f: fn(&'a [T; N]),
 /// }
 ///
 /// impl_has_work! {
-///     impl HasWork<MyStruct, 17> for MyStruct { self.work_field }
+///     impl{'a, T, const N: usize} HasWork<MyStruct<'a, T, N>, 17> for MyStruct<'a, T, N> { self.work_field }
 /// }
 /// ```
 #[macro_export]
 macro_rules! impl_has_work {
-    ($(impl$(<$($implarg:ident),*>)?
+    ($(impl$({$($generics:tt)*})?
        HasWork<$work_type:ty $(, $id:tt)?>
-       for $self:ident $(<$($selfarg:ident),*>)?
+       for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_work` only compiles if the field has the right
         // type.
-        unsafe impl$(<$($implarg),*>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self $(<$($selfarg),*>)? {
+        unsafe impl$(<$($generics)+>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]
@@ -515,7 +516,7 @@ unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_typ
 pub use impl_has_work;
 
 impl_has_work! {
-    impl<T> HasWork<Self> for ClosureWork<T> { self.work }
+    impl{T} HasWork<Self> for ClosureWork<T> { self.work }
 }
 
 unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
-- 
2.34.1

Re: [PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[Greg KH]

On Wed, May 22, 2024 at 09:16:33PM +0800, Roland Xu wrote:
> Make the impl_has_work macro compatible with more complex generics such as lifetimes and const generic arguments.

Can you wrap your lines at 72 columns like checkpatch asks for?

> See more in https://github.com/Rust-for-Linux/linux/issues/1077

Please don't point to external sites for "more information", include it
here in the changelog text as this is where it is going to live for
"forever", random external sites hosted by others usually have short
lifespans.

thanks,

greg k-h

Re: [PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[Roland Xu]

Fixes these in the v2 patch, thanks!

Greg KH:
> Can you wrap your lines at 72 columns like checkpatch asks for?

Greg KH:
> Please don't point to external sites for "more information", include it
here in the changelog text as this is where it is going to live for
"forever", random external sites hosted by others usually have short
lifespans.

________________________________________
From: Greg KH <gregkh@linuxfoundation.org>
Sent: Wednesday, May 22, 2024 22:47
To: Roland Xu
Cc: ojeda@kernel.org; boqun.feng@gmail.com; rust-for-linux@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rust: kernel: make impl_has_work compatible with more complex generics

On Wed, May 22, 2024 at 09:16:33PM +0800, Roland Xu wrote:
> Make the impl_has_work macro compatible with more complex generics such as lifetimes and const generic arguments.

Can you wrap your lines at 72 columns like checkpatch asks for?

> See more in https://github.com/Rust-for-Linux/linux/issues/1077

Please don't point to external sites for "more information", include it
here in the changelog text as this is where it is going to live for
"forever", random external sites hosted by others usually have short
lifespans.

thanks,

greg k-h

[PATCH v2] rust: kernel: make impl_has_work compatible with more generics

[Roland Xu]

v2: apply comments, wrap lines at 72 columns
---
Make the impl_has_work macro compatible with more complex generics such as lifetimes and const generic arguments.

Signed-off-by: Roland Xu <mu001999@outlook.com>
---
 rust/kernel/workqueue.rs | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 1cec63a2a..553a5cba2 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -482,24 +482,26 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
 /// use kernel::sync::Arc;
 /// use kernel::workqueue::{self, impl_has_work, Work};
 ///
-/// struct MyStruct {
-///     work_field: Work<MyStruct, 17>,
+/// struct MyStruct<'a, T, const N: usize> {
+///     work_field: Work<MyStruct<'a, T, N>, 17>,
+///     f: fn(&'a [T; N]),
 /// }
 ///
 /// impl_has_work! {
-///     impl HasWork<MyStruct, 17> for MyStruct { self.work_field }
+///     impl{'a, T, const N: usize} HasWork<MyStruct<'a, T, N>, 17>
+///     for MyStruct<'a, T, N> { self.work_field }
 /// }
 /// ```
 #[macro_export]
 macro_rules! impl_has_work {
-    ($(impl$(<$($implarg:ident),*>)?
+    ($(impl$({$($generics:tt)*})?
        HasWork<$work_type:ty $(, $id:tt)?>
-       for $self:ident $(<$($selfarg:ident),*>)?
+       for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_work` only compiles if the field has the right
         // type.
-        unsafe impl$(<$($implarg),*>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self $(<$($selfarg),*>)? {
+        unsafe impl$(<$($generics)+>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]
@@ -515,7 +517,7 @@ unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_typ
 pub use impl_has_work;
 
 impl_has_work! {
-    impl<T> HasWork<Self> for ClosureWork<T> { self.work }
+    impl{T} HasWork<Self> for ClosureWork<T> { self.work }
 }
 
 unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
-- 
2.43.0

Re: [PATCH v2] rust: kernel: make impl_has_work compatible with more generics

[Greg KH]

On Wed, May 22, 2024 at 11:45:33PM +0800, Roland Xu wrote:
> v2: apply comments, wrap lines at 72 columns
> ---

That goes below the --- line.

> Make the impl_has_work macro compatible with more complex generics such as lifetimes and const generic arguments.

Columns are still not wrapped :(

Look at example submissions on the mailing list for how to structure
this.

thanks,

greg k-h

[PATCH v3] rust: kernel: make impl_has_work compatible with more generics

[Roland Xu]

Make the impl_has_work macro compatible
with more complex generics such as
lifetimes and const generic arguments.

Signed-off-by: Roland Xu <mu001999@outlook.com>
---
 rust/kernel/workqueue.rs | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 1cec63a2a..553a5cba2 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -482,24 +482,26 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
 /// use kernel::sync::Arc;
 /// use kernel::workqueue::{self, impl_has_work, Work};
 ///
-/// struct MyStruct {
-///     work_field: Work<MyStruct, 17>,
+/// struct MyStruct<'a, T, const N: usize> {
+///     work_field: Work<MyStruct<'a, T, N>, 17>,
+///     f: fn(&'a [T; N]),
 /// }
 ///
 /// impl_has_work! {
-///     impl HasWork<MyStruct, 17> for MyStruct { self.work_field }
+///     impl{'a, T, const N: usize} HasWork<MyStruct<'a, T, N>, 17>
+///     for MyStruct<'a, T, N> { self.work_field }
 /// }
 /// ```
 #[macro_export]
 macro_rules! impl_has_work {
-    ($(impl$(<$($implarg:ident),*>)?
+    ($(impl$({$($generics:tt)*})?
        HasWork<$work_type:ty $(, $id:tt)?>
-       for $self:ident $(<$($selfarg:ident),*>)?
+       for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_work` only compiles if the field has the right
         // type.
-        unsafe impl$(<$($implarg),*>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self $(<$($selfarg),*>)? {
+        unsafe impl$(<$($generics)+>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]
@@ -515,7 +517,7 @@ unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_typ
 pub use impl_has_work;
 
 impl_has_work! {
-    impl<T> HasWork<Self> for ClosureWork<T> { self.work }
+    impl{T} HasWork<Self> for ClosureWork<T> { self.work }
 }
 
 unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
-- 
2.45.0

Re: [PATCH v3] rust: kernel: make impl_has_work compatible with more generics

[Miguel Ojeda]

On Wed, May 22, 2024 at 6:09 PM Roland Xu <mu001999@outlook.com> wrote:
>
> Make the impl_has_work macro compatible
> with more complex generics such as
> lifetimes and const generic arguments.
>
> Signed-off-by: Roland Xu <mu001999@outlook.com>

Applied to `rust-next` -- thanks!

I thought about keeping the original example, i.e. non-generic, but it
is a minor thing and we test the non-generic case anyway in another
documentation block above.

[ Wrapped message to 72 columns. - Miguel ]

Cheers,
Miguel

Re: [PATCH v2] rust: kernel: make impl_has_work compatible with more generics

[Roland Xu]

Greg KH:
> Columns are still not wrapped :(

Oh, sorry. I used `scripts/checkpatch.pl` and it only told me the commit title is too long, that's why I only change it.

________________________________________
From: Greg KH <gregkh@linuxfoundation.org>
Sent: Wednesday, May 22, 2024 23:57
To: Roland Xu
Cc: ojeda@kernel.org; boqun.feng@gmail.com; rust-for-linux@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] rust: kernel: make impl_has_work compatible with more generics

On Wed, May 22, 2024 at 11:45:33PM +0800, Roland Xu wrote:
> v2: apply comments, wrap lines at 72 columns
> ---

That goes below the --- line.

> Make the impl_has_work macro compatible with more complex generics such as lifetimes and const generic arguments.

Columns are still not wrapped :(

Look at example submissions on the mailing list for how to structure
this.

thanks,

greg k-h

[PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[mu001999]

Make the impl_has_work macro compatible with more complex generics such as lifetimes and const generic arguments.
See more in https://github.com/Rust-for-Linux/linux/issues/1077

Signed-off-by: mu001999 <mu001999@outlook.com>
---
 rust/kernel/workqueue.rs | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 1cec63a2aea8..1ff81d88b61d 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -482,24 +482,25 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
 /// use kernel::sync::Arc;
 /// use kernel::workqueue::{self, impl_has_work, Work};
 ///
-/// struct MyStruct {
-///     work_field: Work<MyStruct, 17>,
+/// struct MyStruct<'a, T, const N: usize> {
+///     work_field: Work<MyStruct<'a, T, N>, 17>,
+///     f: fn(&'a [T; N]),
 /// }
 ///
 /// impl_has_work! {
-///     impl HasWork<MyStruct, 17> for MyStruct { self.work_field }
+///     impl{'a, T, const N: usize} HasWork<MyStruct<'a, T, N>, 17> for MyStruct<'a, T, N> { self.work_field }
 /// }
 /// ```
 #[macro_export]
 macro_rules! impl_has_work {
-    ($(impl$(<$($implarg:ident),*>)?
+    ($(impl$({$($generics:tt)*})?
        HasWork<$work_type:ty $(, $id:tt)?>
-       for $self:ident $(<$($selfarg:ident),*>)?
+       for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_work` only compiles if the field has the right
         // type.
-        unsafe impl$(<$($implarg),*>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self $(<$($selfarg),*>)? {
+        unsafe impl$(<$($generics)+>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]
@@ -515,7 +516,7 @@ unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_typ
 pub use impl_has_work;
 
 impl_has_work! {
-    impl<T> HasWork<Self> for ClosureWork<T> { self.work }
+    impl{T} HasWork<Self> for ClosureWork<T> { self.work }
 }
 
 unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
-- 
2.34.1

[PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[mu001999]

Signed-off-by: mu001999 <mu001999@outlook.com>
---
 rust/kernel/workqueue.rs | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 1cec63a2aea8..1ff81d88b61d 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -482,24 +482,25 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
 /// use kernel::sync::Arc;
 /// use kernel::workqueue::{self, impl_has_work, Work};
 ///
-/// struct MyStruct {
-///     work_field: Work<MyStruct, 17>,
+/// struct MyStruct<'a, T, const N: usize> {
+///     work_field: Work<MyStruct<'a, T, N>, 17>,
+///     f: fn(&'a [T; N]),
 /// }
 ///
 /// impl_has_work! {
-///     impl HasWork<MyStruct, 17> for MyStruct { self.work_field }
+///     impl{'a, T, const N: usize} HasWork<MyStruct<'a, T, N>, 17> for MyStruct<'a, T, N> { self.work_field }
 /// }
 /// ```
 #[macro_export]
 macro_rules! impl_has_work {
-    ($(impl$(<$($implarg:ident),*>)?
+    ($(impl$({$($generics:tt)*})?
        HasWork<$work_type:ty $(, $id:tt)?>
-       for $self:ident $(<$($selfarg:ident),*>)?
+       for $self:ty
        { self.$field:ident }
     )*) => {$(
         // SAFETY: The implementation of `raw_get_work` only compiles if the field has the right
         // type.
-        unsafe impl$(<$($implarg),*>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self $(<$($selfarg),*>)? {
+        unsafe impl$(<$($generics)+>)? $crate::workqueue::HasWork<$work_type $(, $id)?> for $self {
             const OFFSET: usize = ::core::mem::offset_of!(Self, $field) as usize;
 
             #[inline]
@@ -515,7 +516,7 @@ unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_typ
 pub use impl_has_work;
 
 impl_has_work! {
-    impl<T> HasWork<Self> for ClosureWork<T> { self.work }
+    impl{T} HasWork<Self> for ClosureWork<T> { self.work }
 }
 
 unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
-- 
2.34.1

Re: [PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[Alice Ryhl]

On Wed, May 22, 2024 at 2:27 PM mu001999 <mu001999@outlook.com> wrote:
>
> Signed-off-by: mu001999 <mu001999@outlook.com>

Please provide a more complete commit description. What user motivates
this change?

>  impl_has_work! {
> -    impl<T> HasWork<Self> for ClosureWork<T> { self.work }
> +    impl{T} HasWork<Self> for ClosureWork<T> { self.work }
>  }

I ended up doing something similar for the generics in some of the
linked list patches. Does anyone know if it's possible to support this
without giving up the <T> syntax?

Alice

Re: [PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[Benno Lossin]

On 22.05.24 14:37, Alice Ryhl wrote:
> On Wed, May 22, 2024 at 2:27 PM mu001999 <mu001999@outlook.com> wrote:
>>  impl_has_work! {
>> -    impl<T> HasWork<Self> for ClosureWork<T> { self.work }
>> +    impl{T} HasWork<Self> for ClosureWork<T> { self.work }
>>  }
> 
> I ended up doing something similar for the generics in some of the
> linked list patches. Does anyone know if it's possible to support this
> without giving up the <T> syntax?

I tried to come up with something some time ago, but it was not really
nice. You have to parse the entire generics manually, which ends up
looking horrible. I have been thinking some time now that a `generics`
fragment would actually be really useful in declarative macros.

I also thought that if we get even more `Has*` traits for intrusive
datastructures, we could add a unified derive macro that allows you to
just do:

    #[derive(Intrusive)]
    struct MyStruct {
        #[intrusive]
        work: Work<Self>,
        #[intrusive]
        timer: Timer<Self>,
        /* ... */
    }

But I thought that as long as we have only two intrusive structures, we
don't need this.

---
Cheers,
Benno

Re: [PATCH] rust: kernel: make impl_has_work compatible with more complex generics

[Miguel Ojeda]

On Wed, May 22, 2024 at 2:27 PM mu001999 <mu001999@outlook.com> wrote:
>
> Signed-off-by: mu001999 <mu001999@outlook.com>

The kernel requires a "known identity" (it does not accept anonymous
contributions) -- please see
https://docs.kernel.org/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin.

Cheers,
Miguel