The generic parameters on a type definition can specify default values.
Currently `parse_generics()` cannot handle this though. For example when
parsing the following generics:
<T: Clone, const N: usize = 0>
The `impl_generics` will be set to `T: Clone, const N: usize = 0` and
`ty_generics` will be set to `T, N`. Now using the `impl_generics` on an
impl block:
impl<$($impl_generics)*> Foo {}
will result in invalid Rust code, because default values are only
available on type definitions.
Therefore add parsing support for generic parameter default values using
a new kind of generics called `decl_generics` and change the old
behavior of `impl_generics` to not contain the generic parameter default
values.
Now `Generics` has three fields:
- `impl_generics`: the generics with bounds
(e.g. `T: Clone, const N: usize`)
- `decl_generics`: the generics with bounds and default values
(e.g. `T: Clone, const N: usize = 0`)
- `ty_generics`: contains the generics without bounds and without
default values (e.g. `T, N`)
`impl_generics` is designed to be used on `impl<$impl_generics>`,
`decl_generics` for the type definition, so `struct Foo<$decl_generics>`
and `ty_generics` whenever you use the type, so `Foo<$ty_generics>`.
Here is an example that uses all three different types of generics:
let (Generics { decl_generics, impl_generics, ty_generics }, rest) = parse_generics(input);
quote! {
struct Foo<$($decl_generics)*> {
// ...
}
impl<$impl_generics> Foo<$ty_generics> {
fn foo() {
// ...
}
}
}
The next commit contains a fix to the `#[pin_data]` macro making it
compatible with generic parameter default values by relying on this new
behavior.
Signed-off-by: Benno Lossin <[email protected]>
---
v1 -> v2:
- improve documentation and commit message explanation
- add motivation to commit message
rust/macros/helpers.rs | 122 ++++++++++++++++++++++++++++++----------
rust/macros/pin_data.rs | 1 +
rust/macros/zeroable.rs | 1 +
3 files changed, 94 insertions(+), 30 deletions(-)
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index afb0f2e3a36a..3f50a5c847c8 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-use proc_macro::{token_stream, Group, Punct, Spacing, TokenStream, TokenTree};
+use proc_macro::{token_stream, Group, TokenStream, TokenTree};
pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
if let Some(TokenTree::Ident(ident)) = it.next() {
@@ -70,8 +70,40 @@ pub(crate) fn expect_end(it: &mut token_stream::IntoIter) {
}
}
+/// Parsed generics.
+///
+/// See the field documentation for an explanation what each of the fields represents.
+///
+/// # Examples
+///
+/// ```rust,ignore
+/// # let input = todo!();
+/// let (Generics { decl_generics, impl_generics, ty_generics }, rest) = parse_generics(input);
+/// quote! {
+/// struct Foo<$($decl_generics)*> {
+/// // ...
+/// }
+///
+/// impl<$impl_generics> Foo<$ty_generics> {
+/// fn foo() {
+/// // ...
+/// }
+/// }
+/// }
+/// ```
pub(crate) struct Generics {
+ /// The generics with bounds and default values (e.g. `T: Clone, const N: usize = 0`).
+ ///
+ /// Use this on type definitions e.g. `struct Foo<$decl_generics> ...` (or `union`/`enum`).
+ pub(crate) decl_generics: Vec<TokenTree>,
+ /// The generics with bounds (e.g. `T: Clone, const N: usize`).
+ ///
+ /// Use this on `impl` blocks e.g. `impl<$impl_generics> Trait for ...`.
pub(crate) impl_generics: Vec<TokenTree>,
+ /// The generics without bounds and without default values (e.g. `T, N`).
+ ///
+ /// Use this when you use the type that is declared with these generics e.g.
+ /// `Foo<$ty_generics>`.
pub(crate) ty_generics: Vec<TokenTree>,
}
@@ -81,6 +113,8 @@ pub(crate) struct Generics {
pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
// `impl_generics`, the declared generics with their bounds.
let mut impl_generics = vec![];
+ // The generics with bounds and default values.
+ let mut decl_generics = vec![];
// Only the names of the generics, without any bounds.
let mut ty_generics = vec![];
// Tokens not related to the generics e.g. the `where` token and definition.
@@ -90,10 +124,17 @@ pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
let mut toks = input.into_iter();
// If we are at the beginning of a generic parameter.
let mut at_start = true;
- for tt in &mut toks {
+ let mut skip_until_comma = false;
+ while let Some(tt) = toks.next() {
+ if nesting == 1 && matches!(&tt, TokenTree::Punct(p) if p.as_char() == '>') {
+ // Found the end of the generics.
+ break;
+ } else if nesting >= 1 {
+ decl_generics.push(tt.clone());
+ }
match tt.clone() {
TokenTree::Punct(p) if p.as_char() == '<' => {
- if nesting >= 1 {
+ if nesting >= 1 && !skip_until_comma {
// This is inside of the generics and part of some bound.
impl_generics.push(tt);
}
@@ -105,49 +146,70 @@ pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
break;
} else {
nesting -= 1;
- if nesting >= 1 {
+ if nesting >= 1 && !skip_until_comma {
// We are still inside of the generics and part of some bound.
impl_generics.push(tt);
}
- if nesting == 0 {
- break;
- }
}
}
- tt => {
+ TokenTree::Punct(p) if skip_until_comma && p.as_char() == ',' => {
if nesting == 1 {
- // Here depending on the token, it might be a generic variable name.
- match &tt {
- // Ignore const.
- TokenTree::Ident(i) if i.to_string() == "const" => {}
- TokenTree::Ident(_) if at_start => {
- ty_generics.push(tt.clone());
- // We also already push the `,` token, this makes it easier to append
- // generics.
- ty_generics.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
- at_start = false;
- }
- TokenTree::Punct(p) if p.as_char() == ',' => at_start = true,
- // Lifetimes begin with `'`.
- TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
- ty_generics.push(tt.clone());
- }
- _ => {}
- }
+ impl_generics.push(TokenTree::Punct(p.clone()));
+ ty_generics.push(TokenTree::Punct(p));
+ skip_until_comma = false;
}
- if nesting >= 1 {
- impl_generics.push(tt);
- } else if nesting == 0 {
+ }
+ tt if !skip_until_comma => {
+ match nesting {
// If we haven't entered the generics yet, we still want to keep these tokens.
- rest.push(tt);
+ 0 => rest.push(tt),
+ 1 => {
+ // Here depending on the token, it might be a generic variable name.
+ match tt {
+ TokenTree::Ident(i) if at_start && i.to_string() == "const" => {
+ let Some(name) = toks.next() else {
+ // Parsing error.
+ break;
+ };
+ impl_generics.push(TokenTree::Ident(i));
+ impl_generics.push(name.clone());
+ ty_generics.push(name.clone());
+ decl_generics.push(name);
+ at_start = false;
+ }
+ tt @ TokenTree::Ident(_) if at_start => {
+ impl_generics.push(tt.clone());
+ ty_generics.push(tt);
+ at_start = false;
+ }
+ TokenTree::Punct(p) if p.as_char() == ',' => {
+ impl_generics.push(TokenTree::Punct(p.clone()));
+ ty_generics.push(TokenTree::Punct(p));
+ at_start = true;
+ }
+ // Lifetimes begin with `'`.
+ TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
+ ty_generics.push(TokenTree::Punct(p.clone()));
+ impl_generics.push(TokenTree::Punct(p));
+ }
+ // Generics can have default values, we skip these.
+ TokenTree::Punct(p) if p.as_char() == '=' => {
+ skip_until_comma = true;
+ }
+ tt => impl_generics.push(tt),
+ }
+ }
+ _ => impl_generics.push(tt),
}
}
+ _ => {}
}
}
rest.extend(toks);
(
Generics {
impl_generics,
+ decl_generics,
ty_generics,
},
rest,
diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
index 6d58cfda9872..022e68e9720d 100644
--- a/rust/macros/pin_data.rs
+++ b/rust/macros/pin_data.rs
@@ -10,6 +10,7 @@ pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
let (
Generics {
impl_generics,
+ decl_generics: _,
ty_generics,
},
rest,
diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
index 0d605c46ab3b..cfee2cec18d5 100644
--- a/rust/macros/zeroable.rs
+++ b/rust/macros/zeroable.rs
@@ -7,6 +7,7 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
let (
Generics {
impl_generics,
+ decl_generics: _,
ty_generics,
},
mut rest,
base-commit: d9857c16cfc6bce7764e1b79956c6a028f97f4d0
--
2.42.0
Add support for generic parameters defaults in `#[pin_data]` by using
the newly introduced `decl_generics` instead of the `impl_generics`.
Before this would not compile:
#[pin_data]
struct Foo<const N: usize = 0> {
// ...
}
because it would be expanded to this:
struct Foo<const N: usize = 0> {
// ...
}
const _: () = {
struct __ThePinData<const N: usize = 0> {
__phantom: ::core::marker::PhantomData<fn(Foo<N>) -> Foo<N>>,
}
impl<const N: usize = 0> ::core::clone::Clone for __ThePinData<N> {
fn clone(&self) -> Self {
*self
}
}
// [...] rest of expansion omitted
};
The problem is with the `impl<const N: usize = 0>`, since that is
invalid Rust syntax. It should not mention the default value at all,
since default values only make sense on type definitions.
The new `impl_generics` do not contain the default values, thus
generating correct Rust code.
This is used by the next commit that puts `#[pin_data]` on
`kernel::workqueue::Work`.
Signed-off-by: Benno Lossin <[email protected]>
---
v1 -> v2:
- clarify the change in the commit message
- add motivation to the commit message
rust/kernel/init/macros.rs | 19 ++++++++++++++++++-
rust/macros/pin_data.rs | 3 ++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index cb6e61b6c50b..624e9108e3b4 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -538,6 +538,7 @@ macro_rules! __pin_data {
),
@impl_generics($($impl_generics:tt)*),
@ty_generics($($ty_generics:tt)*),
+ @decl_generics($($decl_generics:tt)*),
@body({ $($fields:tt)* }),
) => {
// We now use token munching to iterate through all of the fields. While doing this we
@@ -560,6 +561,9 @@ macro_rules! __pin_data {
@impl_generics($($impl_generics)*),
// The 'ty generics', the generics that will need to be specified on the impl blocks.
@ty_generics($($ty_generics)*),
+ // The 'decl generics', the generics that need to be specified on the struct
+ // definition.
+ @decl_generics($($decl_generics)*),
// The where clause of any impl block and the declaration.
@where($($($whr)*)?),
// The remaining fields tokens that need to be processed.
@@ -585,6 +589,7 @@ macro_rules! __pin_data {
@name($name:ident),
@impl_generics($($impl_generics:tt)*),
@ty_generics($($ty_generics:tt)*),
+ @decl_generics($($decl_generics:tt)*),
@where($($whr:tt)*),
// We found a PhantomPinned field, this should generally be pinned!
@fields_munch($field:ident : $($($(::)?core::)?marker::)?PhantomPinned, $($rest:tt)*),
@@ -607,6 +612,7 @@ macro_rules! __pin_data {
@name($name),
@impl_generics($($impl_generics)*),
@ty_generics($($ty_generics)*),
+ @decl_generics($($decl_generics)*),
@where($($whr)*),
@fields_munch($($rest)*),
@pinned($($pinned)* $($accum)* $field: ::core::marker::PhantomPinned,),
@@ -623,6 +629,7 @@ macro_rules! __pin_data {
@name($name:ident),
@impl_generics($($impl_generics:tt)*),
@ty_generics($($ty_generics:tt)*),
+ @decl_generics($($decl_generics:tt)*),
@where($($whr:tt)*),
// We reached the field declaration.
@fields_munch($field:ident : $type:ty, $($rest:tt)*),
@@ -640,6 +647,7 @@ macro_rules! __pin_data {
@name($name),
@impl_generics($($impl_generics)*),
@ty_generics($($ty_generics)*),
+ @decl_generics($($decl_generics)*),
@where($($whr)*),
@fields_munch($($rest)*),
@pinned($($pinned)* $($accum)* $field: $type,),
@@ -656,6 +664,7 @@ macro_rules! __pin_data {
@name($name:ident),
@impl_generics($($impl_generics:tt)*),
@ty_generics($($ty_generics:tt)*),
+ @decl_generics($($decl_generics:tt)*),
@where($($whr:tt)*),
// We reached the field declaration.
@fields_munch($field:ident : $type:ty, $($rest:tt)*),
@@ -673,6 +682,7 @@ macro_rules! __pin_data {
@name($name),
@impl_generics($($impl_generics)*),
@ty_generics($($ty_generics)*),
+ @decl_generics($($decl_generics)*),
@where($($whr)*),
@fields_munch($($rest)*),
@pinned($($pinned)*),
@@ -689,6 +699,7 @@ macro_rules! __pin_data {
@name($name:ident),
@impl_generics($($impl_generics:tt)*),
@ty_generics($($ty_generics:tt)*),
+ @decl_generics($($decl_generics:tt)*),
@where($($whr:tt)*),
// We found the `#[pin]` attr.
@fields_munch(#[pin] $($rest:tt)*),
@@ -705,6 +716,7 @@ macro_rules! __pin_data {
@name($name),
@impl_generics($($impl_generics)*),
@ty_generics($($ty_generics)*),
+ @decl_generics($($decl_generics)*),
@where($($whr)*),
@fields_munch($($rest)*),
// We do not include `#[pin]` in the list of attributes, since it is not actually an
@@ -724,6 +736,7 @@ macro_rules! __pin_data {
@name($name:ident),
@impl_generics($($impl_generics:tt)*),
@ty_generics($($ty_generics:tt)*),
+ @decl_generics($($decl_generics:tt)*),
@where($($whr:tt)*),
// We reached the field declaration with visibility, for simplicity we only munch the
// visibility and put it into `$accum`.
@@ -741,6 +754,7 @@ macro_rules! __pin_data {
@name($name),
@impl_generics($($impl_generics)*),
@ty_generics($($ty_generics)*),
+ @decl_generics($($decl_generics)*),
@where($($whr)*),
@fields_munch($field $($rest)*),
@pinned($($pinned)*),
@@ -757,6 +771,7 @@ macro_rules! __pin_data {
@name($name:ident),
@impl_generics($($impl_generics:tt)*),
@ty_generics($($ty_generics:tt)*),
+ @decl_generics($($decl_generics:tt)*),
@where($($whr:tt)*),
// Some other attribute, just put it into `$accum`.
@fields_munch(#[$($attr:tt)*] $($rest:tt)*),
@@ -773,6 +788,7 @@ macro_rules! __pin_data {
@name($name),
@impl_generics($($impl_generics)*),
@ty_generics($($ty_generics)*),
+ @decl_generics($($decl_generics)*),
@where($($whr)*),
@fields_munch($($rest)*),
@pinned($($pinned)*),
@@ -789,6 +805,7 @@ macro_rules! __pin_data {
@name($name:ident),
@impl_generics($($impl_generics:tt)*),
@ty_generics($($ty_generics:tt)*),
+ @decl_generics($($decl_generics:tt)*),
@where($($whr:tt)*),
// We reached the end of the fields, plus an optional additional comma, since we added one
// before and the user is also allowed to put a trailing comma.
@@ -802,7 +819,7 @@ macro_rules! __pin_data {
) => {
// Declare the struct with all fields in the correct order.
$($struct_attrs)*
- $vis struct $name <$($impl_generics)*>
+ $vis struct $name <$($decl_generics)*>
where $($whr)*
{
$($fields)*
diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
index 022e68e9720d..1d4a3547c684 100644
--- a/rust/macros/pin_data.rs
+++ b/rust/macros/pin_data.rs
@@ -10,7 +10,7 @@ pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
let (
Generics {
impl_generics,
- decl_generics: _,
+ decl_generics,
ty_generics,
},
rest,
@@ -77,6 +77,7 @@ pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
@sig(#(#rest)*),
@impl_generics(#(#impl_generics)*),
@ty_generics(#(#ty_generics)*),
+ @decl_generics(#(#decl_generics)*),
@body(#last),
});
quoted.extend(errs);
--
2.42.0
The previous two patches made it possible to add `#[pin_data]` on
structs with default generic parameter values.
This patch makes `Work` use `#[pin_data]` and removes an invocation of
`pin_init_from_closure`. This function is intended as a low level manual
escape hatch, so it is better to rely on the safe `pin_init!` macro.
Signed-off-by: Benno Lossin <[email protected]>
---
v1 -> v2:
- improve commit message wording
- change `:` to `<-` in `pin_init!` invocation
rust/kernel/workqueue.rs | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index b67fb1ba168e..4a9fb7d06d20 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -334,8 +334,10 @@ pub trait WorkItem<const ID: u64 = 0> {
/// Wraps the kernel's C `struct work_struct`.
///
/// This is a helper type used to associate a `work_struct` with the [`WorkItem`] that uses it.
+#[pin_data]
#[repr(transparent)]
pub struct Work<T: ?Sized, const ID: u64 = 0> {
+ #[pin]
work: Opaque<bindings::work_struct>,
_inner: PhantomData<T>,
}
@@ -357,21 +359,22 @@ pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self
where
T: WorkItem<ID>,
{
- // SAFETY: The `WorkItemPointer` implementation promises that `run` can be used as the work
- // item function.
- unsafe {
- kernel::init::pin_init_from_closure(move |slot| {
- let slot = Self::raw_get(slot);
- bindings::init_work_with_key(
- slot,
- Some(T::Pointer::run),
- false,
- name.as_char_ptr(),
- key.as_ptr(),
- );
- Ok(())
- })
- }
+ pin_init!(Self {
+ work <- Opaque::ffi_init(|slot| {
+ // SAFETY: The `WorkItemPointer` implementation promises that `run` can be used as
+ // the work item function.
+ unsafe {
+ bindings::init_work_with_key(
+ slot,
+ Some(T::Pointer::run),
+ false,
+ name.as_char_ptr(),
+ key.as_ptr(),
+ )
+ }
+ }),
+ _inner: PhantomData,
+ })
}
/// Get a pointer to the inner `work_struct`.
--
2.42.0
On 12/13/23 19:08, Benno Lossin wrote:
> Add support for generic parameters defaults in `#[pin_data]` by using
> the newly introduced `decl_generics` instead of the `impl_generics`.
>
> Before this would not compile:
>
> #[pin_data]
> struct Foo<const N: usize = 0> {
> // ...
> }
>
> because it would be expanded to this:
>
> struct Foo<const N: usize = 0> {
> // ...
> }
>
> const _: () = {
> struct __ThePinData<const N: usize = 0> {
> __phantom: ::core::marker::PhantomData<fn(Foo<N>) -> Foo<N>>,
> }
> impl<const N: usize = 0> ::core::clone::Clone for __ThePinData<N> {
> fn clone(&self) -> Self {
> *self
> }
> }
>
> // [...] rest of expansion omitted
> };
>
> The problem is with the `impl<const N: usize = 0>`, since that is
> invalid Rust syntax. It should not mention the default value at all,
> since default values only make sense on type definitions.
>
> The new `impl_generics` do not contain the default values, thus
> generating correct Rust code.
>
> This is used by the next commit that puts `#[pin_data]` on
> `kernel::workqueue::Work`.
>
> Signed-off-by: Benno Lossin <[email protected]>
> ---
> [...]
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
On 12/13/23 19:08, Benno Lossin wrote:
> The generic parameters on a type definition can specify default values.
> Currently `parse_generics()` cannot handle this though. For example when
> parsing the following generics:
>
> <T: Clone, const N: usize = 0>
>
> The `impl_generics` will be set to `T: Clone, const N: usize = 0` and
> `ty_generics` will be set to `T, N`. Now using the `impl_generics` on an
> impl block:
>
> impl<$($impl_generics)*> Foo {}
>
> will result in invalid Rust code, because default values are only
> available on type definitions.
>
> Therefore add parsing support for generic parameter default values using
> a new kind of generics called `decl_generics` and change the old
> behavior of `impl_generics` to not contain the generic parameter default
> values.
>
> Now `Generics` has three fields:
> - `impl_generics`: the generics with bounds
> (e.g. `T: Clone, const N: usize`)
> - `decl_generics`: the generics with bounds and default values
> (e.g. `T: Clone, const N: usize = 0`)
> - `ty_generics`: contains the generics without bounds and without
> default values (e.g. `T, N`)
>
> `impl_generics` is designed to be used on `impl<$impl_generics>`,
> `decl_generics` for the type definition, so `struct Foo<$decl_generics>`
> and `ty_generics` whenever you use the type, so `Foo<$ty_generics>`.
>
> Here is an example that uses all three different types of generics:
>
> let (Generics { decl_generics, impl_generics, ty_generics }, rest) = parse_generics(input);
> quote! {
> struct Foo<$($decl_generics)*> {
> // ...
> }
>
> impl<$impl_generics> Foo<$ty_generics> {
> fn foo() {
> // ...
> }
> }
> }
>
> The next commit contains a fix to the `#[pin_data]` macro making it
> compatible with generic parameter default values by relying on this new
> behavior.
>
> Signed-off-by: Benno Lossin <[email protected]>
> ---
> [...]
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
On 12/14/23 04:13, Martin Rodriguez Reboredo wrote:
> On 12/13/23 19:09, Benno Lossin wrote:
>> The previous two patches made it possible to add `#[pin_data]` on
>> structs with default generic parameter values.
>> This patch makes `Work` use `#[pin_data]` and removes an invocation of
>> `pin_init_from_closure`. This function is intended as a low level manual
>> escape hatch, so it is better to rely on the safe `pin_init!` macro.
>>
>> Signed-off-by: Benno Lossin <[email protected]>
>> ---
>> [...]
Did you mean to add your reviewed by? Because I received only
the quote.
--
Cheers,
Benno
The logic looks good to me, some nits:
> +/// Parsed generics.
> +///
> +/// See the field documentation for an explanation what each of the fields represents.
> +///
> +/// # Examples
> +///
> +/// ```rust,ignore
> +/// # let input = todo!();
> +/// let (Generics { decl_generics, impl_generics, ty_generics }, rest) = parse_generics(input);
> +/// quote! {
> +/// struct Foo<$($decl_generics)*> {
> +/// // ...
> +/// }
> +///
> +/// impl<$impl_generics> Foo<$ty_generics> {
> +/// fn foo() {
> +/// // ...
> +/// }
> +/// }
> +/// }
> +/// ```
> pub(crate) struct Generics {
> + /// The generics with bounds and default values (e.g. `T: Clone, const N: usize = 0`).
> + ///
> + /// Use this on type definitions e.g. `struct Foo<$decl_generics> ...` (or `union`/`enum`).
> + pub(crate) decl_generics: Vec<TokenTree>,
> + /// The generics with bounds (e.g. `T: Clone, const N: usize`).
> + ///
> + /// Use this on `impl` blocks e.g. `impl<$impl_generics> Trait for ...`.
> pub(crate) impl_generics: Vec<TokenTree>,
> + /// The generics without bounds and without default values (e.g. `T, N`).
> + ///
> + /// Use this when you use the type that is declared with these generics e.g.
> + /// `Foo<$ty_generics>`.
> pub(crate) ty_generics: Vec<TokenTree>,
> }
>
> @@ -81,6 +113,8 @@ pub(crate) struct Generics {
> pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
> // `impl_generics`, the declared generics with their bounds.
> let mut impl_generics = vec![];
> + // The generics with bounds and default values.
> + let mut decl_generics = vec![];
It'll be great if the order if the same as the declaration in struct
above.
> // Only the names of the generics, without any bounds.
> let mut ty_generics = vec![];
> // Tokens not related to the generics e.g. the `where` token and definition.
> @@ -90,10 +124,17 @@ pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
> let mut toks = input.into_iter();
> // If we are at the beginning of a generic parameter.
> let mut at_start = true;
> - for tt in &mut toks {
> + let mut skip_until_comma = false;
> + while let Some(tt) = toks.next() {
> + if nesting == 1 && matches!(&tt, TokenTree::Punct(p) if p.as_char() == '>') {
> + // Found the end of the generics.
> + break;
> + } else if nesting >= 1 {
> + decl_generics.push(tt.clone());
> + }
> match tt.clone() {
> TokenTree::Punct(p) if p.as_char() == '<' => {
> - if nesting >= 1 {
> + if nesting >= 1 && !skip_until_comma {
> // This is inside of the generics and part of some bound.
> impl_generics.push(tt);
> }
> @@ -105,49 +146,70 @@ pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
> break;
> } else {
> nesting -= 1;
> - if nesting >= 1 {
> + if nesting >= 1 && !skip_until_comma {
> // We are still inside of the generics and part of some bound.
> impl_generics.push(tt);
> }
> - if nesting == 0 {
> - break;
> - }
> }
> }
> - tt => {
> + TokenTree::Punct(p) if skip_until_comma && p.as_char() == ',' => {
> if nesting == 1 {
> - // Here depending on the token, it might be a generic variable name.
> - match &tt {
> - // Ignore const.
> - TokenTree::Ident(i) if i.to_string() == "const" => {}
> - TokenTree::Ident(_) if at_start => {
> - ty_generics.push(tt.clone());
> - // We also already push the `,` token, this makes it easier to append
> - // generics.
> - ty_generics.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
> - at_start = false;
> - }
> - TokenTree::Punct(p) if p.as_char() == ',' => at_start = true,
> - // Lifetimes begin with `'`.
> - TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
> - ty_generics.push(tt.clone());
> - }
> - _ => {}
> - }
> + impl_generics.push(TokenTree::Punct(p.clone()));
> + ty_generics.push(TokenTree::Punct(p));
This could just be
impl_generics.push(tt.clone());
ty_generics.push(tt);
?
> + skip_until_comma = false;
> }
> - if nesting >= 1 {
> - impl_generics.push(tt);
> - } else if nesting == 0 {
> + }
> + tt if !skip_until_comma => {
> + match nesting {
> // If we haven't entered the generics yet, we still want to keep these tokens.
> - rest.push(tt);
> + 0 => rest.push(tt),
> + 1 => {
> + // Here depending on the token, it might be a generic variable name.
> + match tt {
> + TokenTree::Ident(i) if at_start && i.to_string() == "const" => {
> + let Some(name) = toks.next() else {
> + // Parsing error.
> + break;
> + };
> + impl_generics.push(TokenTree::Ident(i));
Similar, this could just be push tt.
> + impl_generics.push(name.clone());
> + ty_generics.push(name.clone());
> + decl_generics.push(name);
> + at_start = false;
> + }
> + tt @ TokenTree::Ident(_) if at_start => {
> + impl_generics.push(tt.clone());
> + ty_generics.push(tt);
> + at_start = false;
> + }
> + TokenTree::Punct(p) if p.as_char() == ',' => {
> + impl_generics.push(TokenTree::Punct(p.clone()));
> + ty_generics.push(TokenTree::Punct(p));
> + at_start = true;
> + }
I am not sure why the ident above uses tt, but this spells thing all
out.
> + // Lifetimes begin with `'`.
> + TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
> + ty_generics.push(TokenTree::Punct(p.clone()));
> + impl_generics.push(TokenTree::Punct(p));
> + }
ditto
> + // Generics can have default values, we skip these.
> + TokenTree::Punct(p) if p.as_char() == '=' => {
> + skip_until_comma = true;
> + }
> + tt => impl_generics.push(tt),
> + }
> + }
> + _ => impl_generics.push(tt),
> }
> }
> + _ => {}
> }
> }
> rest.extend(toks);
> (
> Generics {
> impl_generics,
> + decl_generics,
> ty_generics,
> },
> rest,
> diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
> index 6d58cfda9872..022e68e9720d 100644
> --- a/rust/macros/pin_data.rs
> +++ b/rust/macros/pin_data.rs
> @@ -10,6 +10,7 @@ pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
> let (
> Generics {
> impl_generics,
> + decl_generics: _,
> ty_generics,
> },
> rest,
> diff --git a/rust/macros/zeroable.rs b/rust/macros/zeroable.rs
> index 0d605c46ab3b..cfee2cec18d5 100644
> --- a/rust/macros/zeroable.rs
> +++ b/rust/macros/zeroable.rs
> @@ -7,6 +7,7 @@ pub(crate) fn derive(input: TokenStream) -> TokenStream {
> let (
> Generics {
> impl_generics,
> + decl_generics: _,
> ty_generics,
> },
> mut rest,
>
> base-commit: d9857c16cfc6bce7764e1b79956c6a028f97f4d0
On Wed, 13 Dec 2023 22:08:53 +0000
Benno Lossin <[email protected]> wrote:
> Add support for generic parameters defaults in `#[pin_data]` by using
> the newly introduced `decl_generics` instead of the `impl_generics`.
>
> Before this would not compile:
>
> #[pin_data]
> struct Foo<const N: usize = 0> {
> // ...
> }
>
> because it would be expanded to this:
>
> struct Foo<const N: usize = 0> {
> // ...
> }
>
> const _: () = {
> struct __ThePinData<const N: usize = 0> {
> __phantom: ::core::marker::PhantomData<fn(Foo<N>) -> Foo<N>>,
> }
> impl<const N: usize = 0> ::core::clone::Clone for __ThePinData<N> {
> fn clone(&self) -> Self {
> *self
> }
> }
>
> // [...] rest of expansion omitted
> };
>
> The problem is with the `impl<const N: usize = 0>`, since that is
> invalid Rust syntax. It should not mention the default value at all,
> since default values only make sense on type definitions.
>
> The new `impl_generics` do not contain the default values, thus
> generating correct Rust code.
>
> This is used by the next commit that puts `#[pin_data]` on
> `kernel::workqueue::Work`.
>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> v1 -> v2:
> - clarify the change in the commit message
> - add motivation to the commit message
>
> rust/kernel/init/macros.rs | 19 ++++++++++++++++++-
> rust/macros/pin_data.rs | 3 ++-
> 2 files changed, 20 insertions(+), 2 deletions(-)
On Wed, 13 Dec 2023 22:09:04 +0000
Benno Lossin <[email protected]> wrote:
> The previous two patches made it possible to add `#[pin_data]` on
> structs with default generic parameter values.
> This patch makes `Work` use `#[pin_data]` and removes an invocation of
> `pin_init_from_closure`. This function is intended as a low level manual
> escape hatch, so it is better to rely on the safe `pin_init!` macro.
>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> v1 -> v2:
> - improve commit message wording
> - change `:` to `<-` in `pin_init!` invocation
>
> rust/kernel/workqueue.rs | 33 ++++++++++++++++++---------------
> 1 file changed, 18 insertions(+), 15 deletions(-)
On Wed, Dec 13, 2023 at 11:08 PM Benno Lossin <[email protected]> wrote:
>
> The generic parameters on a type definition can specify default values.
> Currently `parse_generics()` cannot handle this though. For example when
> parsing the following generics:
>
> <T: Clone, const N: usize = 0>
>
> The `impl_generics` will be set to `T: Clone, const N: usize = 0` and
> `ty_generics` will be set to `T, N`. Now using the `impl_generics` on an
> impl block:
>
> impl<$($impl_generics)*> Foo {}
>
> will result in invalid Rust code, because default values are only
> available on type definitions.
>
> Therefore add parsing support for generic parameter default values using
> a new kind of generics called `decl_generics` and change the old
> behavior of `impl_generics` to not contain the generic parameter default
> values.
>
> Now `Generics` has three fields:
> - `impl_generics`: the generics with bounds
> (e.g. `T: Clone, const N: usize`)
> - `decl_generics`: the generics with bounds and default values
> (e.g. `T: Clone, const N: usize = 0`)
> - `ty_generics`: contains the generics without bounds and without
> default values (e.g. `T, N`)
>
> `impl_generics` is designed to be used on `impl<$impl_generics>`,
> `decl_generics` for the type definition, so `struct Foo<$decl_generics>`
> and `ty_generics` whenever you use the type, so `Foo<$ty_generics>`.
>
> Here is an example that uses all three different types of generics:
>
> let (Generics { decl_generics, impl_generics, ty_generics }, rest) = parse_generics(input);
> quote! {
> struct Foo<$($decl_generics)*> {
> // ...
> }
>
> impl<$impl_generics> Foo<$ty_generics> {
> fn foo() {
> // ...
> }
> }
> }
>
> The next commit contains a fix to the `#[pin_data]` macro making it
> compatible with generic parameter default values by relying on this new
> behavior.
>
> Signed-off-by: Benno Lossin <[email protected]>
Tested-by: Alice Ryhl <[email protected]>
On Wed, Dec 13, 2023 at 11:09 PM Benno Lossin <[email protected]> wrote:
>
> Add support for generic parameters defaults in `#[pin_data]` by using
> the newly introduced `decl_generics` instead of the `impl_generics`.
>
> Before this would not compile:
>
> #[pin_data]
> struct Foo<const N: usize = 0> {
> // ...
> }
>
> because it would be expanded to this:
>
> struct Foo<const N: usize = 0> {
> // ...
> }
>
> const _: () = {
> struct __ThePinData<const N: usize = 0> {
> __phantom: ::core::marker::PhantomData<fn(Foo<N>) -> Foo<N>>,
> }
> impl<const N: usize = 0> ::core::clone::Clone for __ThePinData<N> {
> fn clone(&self) -> Self {
> *self
> }
> }
>
> // [...] rest of expansion omitted
> };
>
> The problem is with the `impl<const N: usize = 0>`, since that is
> invalid Rust syntax. It should not mention the default value at all,
> since default values only make sense on type definitions.
>
> The new `impl_generics` do not contain the default values, thus
> generating correct Rust code.
>
> This is used by the next commit that puts `#[pin_data]` on
> `kernel::workqueue::Work`.
>
> Signed-off-by: Benno Lossin <[email protected]>
Tested-by: Alice Ryhl <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
On Wed, Dec 13, 2023 at 11:09 PM Benno Lossin <[email protected]> wrote:
>
> The previous two patches made it possible to add `#[pin_data]` on
> structs with default generic parameter values.
> This patch makes `Work` use `#[pin_data]` and removes an invocation of
> `pin_init_from_closure`. This function is intended as a low level manual
> escape hatch, so it is better to rely on the safe `pin_init!` macro.
>
> Signed-off-by: Benno Lossin <[email protected]>
Tested-by: Alice Ryhl <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>