[PATCH 1/4] rust: macros: fix usage of `#[allow]` in `quote!`

[PATCH 1/4] rust: macros: fix usage of `#[allow]` in `quote!`

[Benno Lossin]

When using `quote!` as part of an expression that was not the last one
in a function, the `#[allow(clippy::vec_init_then_push)]` attribute
would be present on an expression, which is not allowed.
This patch refactors that part of the macro to use a statement instead.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
 rust/macros/quote.rs | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
index c8e08b3c1e4c..dddbb4e6f4cb 100644
--- a/rust/macros/quote.rs
+++ b/rust/macros/quote.rs
@@ -39,12 +39,14 @@ impl ToTokens for TokenStream {
 /// [`quote_spanned!`](https://docs.rs/quote/latest/quote/macro.quote_spanned.html) macro from the
 /// `quote` crate but provides only just enough functionality needed by the current `macros` crate.
 macro_rules! quote_spanned {
-    ($span:expr => $($tt:tt)*) => {
-    #[allow(clippy::vec_init_then_push)]
-    {
-        let mut tokens = ::std::vec::Vec::new();
-        let span = $span;
-        quote_spanned!(@proc tokens span $($tt)*);
+    ($span:expr => $($tt:tt)*) => {{
+        let mut tokens;
+        #[allow(clippy::vec_init_then_push)]
+        {
+            tokens = ::std::vec::Vec::new();
+            let span = $span;
+            quote_spanned!(@proc tokens span $($tt)*);
+        }
         ::proc_macro::TokenStream::from_iter(tokens)
     }};
     (@proc $v:ident $span:ident) => {};

base-commit: ea76e08f4d901a450619831a255e9e0a4c0ed162
--
2.40.0

[PATCH 2/4] rust: macros: refactor generics parsing of `#[pin_data]` into its own function

[Benno Lossin]

Other macros might also want to parse generics. Additionally this makes
the code easier to read, as the next commit will introduce more code in
`#[pin_data]`. Also add more comments to explain how parsing generics
work.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
 rust/macros/helpers.rs  | 86 ++++++++++++++++++++++++++++++++++++++++-
 rust/macros/pin_data.rs | 70 +++++----------------------------
 2 files changed, 94 insertions(+), 62 deletions(-)

diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index b2bdd4d8c958..afb0f2e3a36a 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, TokenTree};
+use proc_macro::{token_stream, Group, Punct, Spacing, TokenStream, TokenTree};

 pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
     if let Some(TokenTree::Ident(ident)) = it.next() {
@@ -69,3 +69,87 @@ pub(crate) fn expect_end(it: &mut token_stream::IntoIter) {
         panic!("Expected end");
     }
 }
+
+pub(crate) struct Generics {
+    pub(crate) impl_generics: Vec<TokenTree>,
+    pub(crate) ty_generics: Vec<TokenTree>,
+}
+
+/// Parses the given `TokenStream` into `Generics` and the rest.
+///
+/// The generics are not present in the rest, but a where clause might remain.
+pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
+    // `impl_generics`, the declared generics with their bounds.
+    let mut impl_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.
+    let mut rest = vec![];
+    // The current level of `<`.
+    let mut nesting = 0;
+    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 {
+        match tt.clone() {
+            TokenTree::Punct(p) if p.as_char() == '<' => {
+                if nesting >= 1 {
+                    // This is inside of the generics and part of some bound.
+                    impl_generics.push(tt);
+                }
+                nesting += 1;
+            }
+            TokenTree::Punct(p) if p.as_char() == '>' => {
+                // This is a parsing error, so we just end it here.
+                if nesting == 0 {
+                    break;
+                } else {
+                    nesting -= 1;
+                    if nesting >= 1 {
+                        // We are still inside of the generics and part of some bound.
+                        impl_generics.push(tt);
+                    }
+                    if nesting == 0 {
+                        break;
+                    }
+                }
+            }
+            tt => {
+                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());
+                        }
+                        _ => {}
+                    }
+                }
+                if nesting >= 1 {
+                    impl_generics.push(tt);
+                } else if nesting == 0 {
+                    // If we haven't entered the generics yet, we still want to keep these tokens.
+                    rest.push(tt);
+                }
+            }
+        }
+    }
+    rest.extend(toks);
+    (
+        Generics {
+            impl_generics,
+            ty_generics,
+        },
+        rest,
+    )
+}
diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
index 954149d77181..c593b05d9e8c 100644
--- a/rust/macros/pin_data.rs
+++ b/rust/macros/pin_data.rs
@@ -1,71 +1,19 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT

-use proc_macro::{Punct, Spacing, TokenStream, TokenTree};
+use crate::helpers::{parse_generics, Generics};
+use proc_macro::TokenStream;

 pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
     // This proc-macro only does some pre-parsing and then delegates the actual parsing to
     // `kernel::__pin_data!`.
-    //
-    // In here we only collect the generics, since parsing them in declarative macros is very
-    // elaborate. We also do not need to analyse their structure, we only need to collect them.

-    // `impl_generics`, the declared generics with their bounds.
-    let mut impl_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 `impl` token.
-    let mut rest = vec![];
-    // The current level of `<`.
-    let mut nesting = 0;
-    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 {
-        match tt.clone() {
-            TokenTree::Punct(p) if p.as_char() == '<' => {
-                if nesting >= 1 {
-                    impl_generics.push(tt);
-                }
-                nesting += 1;
-            }
-            TokenTree::Punct(p) if p.as_char() == '>' => {
-                if nesting == 0 {
-                    break;
-                } else {
-                    nesting -= 1;
-                    if nesting >= 1 {
-                        impl_generics.push(tt);
-                    }
-                    if nesting == 0 {
-                        break;
-                    }
-                }
-            }
-            tt => {
-                if nesting == 1 {
-                    match &tt {
-                        TokenTree::Ident(i) if i.to_string() == "const" => {}
-                        TokenTree::Ident(_) if at_start => {
-                            ty_generics.push(tt.clone());
-                            ty_generics.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
-                            at_start = false;
-                        }
-                        TokenTree::Punct(p) if p.as_char() == ',' => at_start = true,
-                        TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
-                            ty_generics.push(tt.clone());
-                        }
-                        _ => {}
-                    }
-                }
-                if nesting >= 1 {
-                    impl_generics.push(tt);
-                } else if nesting == 0 {
-                    rest.push(tt);
-                }
-            }
-        }
-    }
-    rest.extend(toks);
+    let (
+        Generics {
+            impl_generics,
+            ty_generics,
+        },
+        mut rest,
+    ) = parse_generics(input);
     // This should be the body of the struct `{...}`.
     let last = rest.pop();
     quote!(::kernel::__pin_data! {
--
2.40.0

[PATCH 3/4] rust: macros: replace Self with the concrete type in #[pin_data]

[Benno Lossin]

When using `#[pin_data]` on a struct that used `Self` in the field
types, a type error would be emitted when trying to use `pin_init!`.
Since an internal type would be referenced by `Self` instead of the
defined struct.
This patch fixes this issue by replacing all occurrences of `Self` in
the `#[pin_data]` macro with the concrete type circumventing the issue.
Since rust allows type definitions inside of blocks, which are
expressions, the macro also checks for these and emits a compile error
when it finds `trait`, `enum`, `union`, `struct` or `impl`. These
keywords allow creating new `Self` contexts, which conflicts with the
current implementation of replacing every `Self` ident. If these were
allowed, some `Self` idents would be replaced incorrectly.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
Reported-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/macros/pin_data.rs | 108 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 104 insertions(+), 4 deletions(-)

diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
index c593b05d9e8c..6d58cfda9872 100644
--- a/rust/macros/pin_data.rs
+++ b/rust/macros/pin_data.rs
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT

 use crate::helpers::{parse_generics, Generics};
-use proc_macro::TokenStream;
+use proc_macro::{Group, Punct, Spacing, TokenStream, TokenTree};

 pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
     // This proc-macro only does some pre-parsing and then delegates the actual parsing to
@@ -12,16 +12,116 @@ pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
             impl_generics,
             ty_generics,
         },
-        mut rest,
+        rest,
     ) = parse_generics(input);
+    // The struct definition might contain the `Self` type. Since `__pin_data!` will define a new
+    // type with the same generics and bounds, this poses a problem, since `Self` will refer to the
+    // new type as opposed to this struct definition. Therefore we have to replace `Self` with the
+    // concrete name.
+
+    // Errors that occur when replacing `Self` with `struct_name`.
+    let mut errs = TokenStream::new();
+    // The name of the struct with ty_generics.
+    let struct_name = rest
+        .iter()
+        .skip_while(|tt| !matches!(tt, TokenTree::Ident(i) if i.to_string() == "struct"))
+        .nth(1)
+        .and_then(|tt| match tt {
+            TokenTree::Ident(_) => {
+                let tt = tt.clone();
+                let mut res = vec![tt];
+                if !ty_generics.is_empty() {
+                    // We add this, so it is maximally compatible with e.g. `Self::CONST` which
+                    // will be replaced by `StructName::<$generics>::CONST`.
+                    res.push(TokenTree::Punct(Punct::new(':', Spacing::Joint)));
+                    res.push(TokenTree::Punct(Punct::new(':', Spacing::Alone)));
+                    res.push(TokenTree::Punct(Punct::new('<', Spacing::Alone)));
+                    res.extend(ty_generics.iter().cloned());
+                    res.push(TokenTree::Punct(Punct::new('>', Spacing::Alone)));
+                }
+                Some(res)
+            }
+            _ => None,
+        })
+        .unwrap_or_else(|| {
+            // If we did not find the name of the struct then we will use `Self` as the replacement
+            // and add a compile error to ensure it does not compile.
+            errs.extend(
+                "::core::compile_error!(\"Could not locate type name.\");"
+                    .parse::<TokenStream>()
+                    .unwrap(),
+            );
+            "Self".parse::<TokenStream>().unwrap().into_iter().collect()
+        });
+    let impl_generics = impl_generics
+        .into_iter()
+        .flat_map(|tt| replace_self_and_deny_type_defs(&struct_name, tt, &mut errs))
+        .collect::<Vec<_>>();
+    let mut rest = rest
+        .into_iter()
+        .flat_map(|tt| {
+            // We ignore top level `struct` tokens, since they would emit a compile error.
+            if matches!(&tt, TokenTree::Ident(i) if i.to_string() == "struct") {
+                vec![tt]
+            } else {
+                replace_self_and_deny_type_defs(&struct_name, tt, &mut errs)
+            }
+        })
+        .collect::<Vec<_>>();
     // This should be the body of the struct `{...}`.
     let last = rest.pop();
-    quote!(::kernel::__pin_data! {
+    let mut quoted = quote!(::kernel::__pin_data! {
         parse_input:
         @args(#args),
         @sig(#(#rest)*),
         @impl_generics(#(#impl_generics)*),
         @ty_generics(#(#ty_generics)*),
         @body(#last),
-    })
+    });
+    quoted.extend(errs);
+    quoted
+}
+
+/// Replaces `Self` with `struct_name` and errors on `enum`, `trait`, `struct` `union` and `impl`
+/// keywords.
+///
+/// The error is appended to `errs` to allow normal parsing to continue.
+fn replace_self_and_deny_type_defs(
+    struct_name: &Vec<TokenTree>,
+    tt: TokenTree,
+    errs: &mut TokenStream,
+) -> Vec<TokenTree> {
+    match tt {
+        TokenTree::Ident(ref i)
+            if i.to_string() == "enum"
+                || i.to_string() == "trait"
+                || i.to_string() == "struct"
+                || i.to_string() == "union"
+                || i.to_string() == "impl" =>
+        {
+            errs.extend(
+                format!(
+                    "::core::compile_error!(\"Cannot use `{i}` inside of struct definition with \
+                        `#[pin_data]`.\");"
+                )
+                .parse::<TokenStream>()
+                .unwrap()
+                .into_iter()
+                .map(|mut tok| {
+                    tok.set_span(tt.span());
+                    tok
+                }),
+            );
+            vec![tt]
+        }
+        TokenTree::Ident(i) if i.to_string() == "Self" => struct_name.clone(),
+        TokenTree::Literal(_) | TokenTree::Punct(_) | TokenTree::Ident(_) => vec![tt],
+        TokenTree::Group(g) => vec![TokenTree::Group(Group::new(
+            g.delimiter(),
+            g.stream()
+                .into_iter()
+                .flat_map(|tt| replace_self_and_deny_type_defs(struct_name, tt, errs))
+                .collect(),
+        ))],
+    }
 }
--
2.40.0

[PATCH 4/4] rust: init: update macro expansion example in docs

[Benno Lossin]

Also improve the explaining comments.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
 rust/kernel/init/macros.rs | 85 +++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 37 deletions(-)

diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
index 541cfad1d8be..00aa4e956c0a 100644
--- a/rust/kernel/init/macros.rs
+++ b/rust/kernel/init/macros.rs
@@ -16,8 +16,9 @@
 //!
 //! We will look at the following example:
 //!
-//! ```rust
+//! ```rust,ignore
 //! # use kernel::init::*;
+//! # use core::pin::Pin;
 //! #[pin_data]
 //! #[repr(C)]
 //! struct Bar<T> {
@@ -71,11 +72,12 @@
 //!
 //! Here is the definition of `Bar` from our example:
 //!
-//! ```rust
+//! ```rust,ignore
 //! # use kernel::init::*;
 //! #[pin_data]
 //! #[repr(C)]
 //! struct Bar<T> {
+//!     #[pin]
 //!     t: T,
 //!     pub x: usize,
 //! }
@@ -83,7 +85,7 @@
 //!
 //! This expands to the following code:
 //!
-//! ```rust
+//! ```rust,ignore
 //! // Firstly the normal definition of the struct, attributes are preserved:
 //! #[repr(C)]
 //! struct Bar<T> {
@@ -116,20 +118,22 @@
 //!         unsafe fn t<E>(
 //!             self,
 //!             slot: *mut T,
-//!             init: impl ::kernel::init::Init<T, E>,
+//!             // Since `t` is `#[pin]`, this is `PinInit`.
+//!             init: impl ::kernel::init::PinInit<T, E>,
 //!         ) -> ::core::result::Result<(), E> {
-//!             unsafe { ::kernel::init::Init::__init(init, slot) }
+//!             unsafe { ::kernel::init::PinInit::__pinned_init(init, slot) }
 //!         }
 //!         pub unsafe fn x<E>(
 //!             self,
 //!             slot: *mut usize,
+//!             // Since `x` is not `#[pin]`, this is `Init`.
 //!             init: impl ::kernel::init::Init<usize, E>,
 //!         ) -> ::core::result::Result<(), E> {
 //!             unsafe { ::kernel::init::Init::__init(init, slot) }
 //!         }
 //!     }
 //!     // Implement the internal `HasPinData` trait that associates `Bar` with the pin-data struct
-//!     // that we constructed beforehand.
+//!     // that we constructed above.
 //!     unsafe impl<T> ::kernel::init::__internal::HasPinData for Bar<T> {
 //!         type PinData = __ThePinData<T>;
 //!         unsafe fn __pin_data() -> Self::PinData {
@@ -160,6 +164,8 @@
 //!     struct __Unpin<'__pin, T> {
 //!         __phantom_pin: ::core::marker::PhantomData<fn(&'__pin ()) -> &'__pin ()>,
 //!         __phantom: ::core::marker::PhantomData<fn(Bar<T>) -> Bar<T>>,
+//!         // Our only `#[pin]` field is `t`.
+//!         t: T,
 //!     }
 //!     #[doc(hidden)]
 //!     impl<'__pin, T>
@@ -193,7 +199,7 @@
 //!
 //! Here is the impl on `Bar` defining the new function:
 //!
-//! ```rust
+//! ```rust,ignore
 //! impl<T> Bar<T> {
 //!     fn new(t: T) -> impl PinInit<Self> {
 //!         pin_init!(Self { t, x: 0 })
@@ -203,7 +209,7 @@
 //!
 //! This expands to the following code:
 //!
-//! ```rust
+//! ```rust,ignore
 //! impl<T> Bar<T> {
 //!     fn new(t: T) -> impl PinInit<Self> {
 //!         {
@@ -232,25 +238,31 @@
 //!                     // that will refer to this struct instead of the one defined above.
 //!                     struct __InitOk;
 //!                     // This is the expansion of `t,`, which is syntactic sugar for `t: t,`.
-//!                     unsafe { ::core::ptr::write(&raw mut (*slot).t, t) };
+//!                     unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).t), t) };
 //!                     // Since initialization could fail later (not in this case, since the error
-//!                     // type is `Infallible`) we will need to drop this field if it fails. This
-//!                     // `DropGuard` will drop the field when it gets dropped and has not yet
-//!                     // been forgotten. We make a reference to it, so users cannot `mem::forget`
-//!                     // it from the initializer, since the name is the same as the field.
+//!                     // type is `Infallible`) we will need to drop this field if there is an
+//!                     // error later. This `DropGuard` will drop the field when it gets dropped
+//!                     // and has not yet been forgotten. We make a reference to it, so users
+//!                     // cannot `mem::forget` it from the initializer, since the name is the same
+//!                     // as the field (including hygiene).
 //!                     let t = &unsafe {
-//!                         ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).t)
+//!                         ::kernel::init::__internal::DropGuard::new(
+//!                             ::core::addr_of_mut!((*slot).t),
+//!                         )
 //!                     };
 //!                     // Expansion of `x: 0,`:
 //!                     // Since this can be an arbitrary expression we cannot place it inside of
 //!                     // the `unsafe` block, so we bind it here.
 //!                     let x = 0;
-//!                     unsafe { ::core::ptr::write(&raw mut (*slot).x, x) };
+//!                     unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).x), x) };
+//!                     // We again create a `DropGuard`.
 //!                     let x = &unsafe {
-//!                         ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).x)
+//!                         ::kernel::init::__internal::DropGuard::new(
+//!                             ::core::addr_of_mut!((*slot).x),
+//!                         )
 //!                     };
 //!
-//!                     // Here we use the type checker to ensuer that every field has been
+//!                     // Here we use the type checker to ensure that every field has been
 //!                     // initialized exactly once, since this is `if false` it will never get
 //!                     // executed, but still type-checked.
 //!                     // Additionally we abuse `slot` to automatically infer the correct type for
@@ -272,7 +284,7 @@
 //!                         };
 //!                     }
 //!                     // Since initialization has successfully completed, we can now forget the
-//!                     // guards.
+//!                     // guards. This is not `mem::forget`, since we only have `&DropGuard`.
 //!                     unsafe { ::kernel::init::__internal::DropGuard::forget(t) };
 //!                     unsafe { ::kernel::init::__internal::DropGuard::forget(x) };
 //!                 }
@@ -280,7 +292,7 @@
 //!                 // `__InitOk` that we need to return.
 //!                 Ok(__InitOk)
 //!             });
-//!             // Change the return type of the closure.
+//!             // Change the return type from `__InitOk` to `()`.
 //!             let init = move |slot| -> ::core::result::Result<(), ::core::convert::Infallible> {
 //!                 init(slot).map(|__InitOk| ())
 //!             };
@@ -299,7 +311,7 @@
 //! Since we already took a look at `#[pin_data]` on `Bar`, this section will only explain the
 //! differences/new things in the expansion of the `Foo` definition:
 //!
-//! ```rust
+//! ```rust,ignore
 //! #[pin_data(PinnedDrop)]
 //! struct Foo {
 //!     a: usize,
@@ -310,7 +322,7 @@
 //!
 //! This expands to the following code:
 //!
-//! ```rust
+//! ```rust,ignore
 //! struct Foo {
 //!     a: usize,
 //!     b: Bar<u32>,
@@ -330,8 +342,6 @@
 //!         unsafe fn b<E>(
 //!             self,
 //!             slot: *mut Bar<u32>,
-//!             // Note that this is `PinInit` instead of `Init`, this is because `b` is
-//!             // structurally pinned, as marked by the `#[pin]` attribute.
 //!             init: impl ::kernel::init::PinInit<Bar<u32>, E>,
 //!         ) -> ::core::result::Result<(), E> {
 //!             unsafe { ::kernel::init::PinInit::__pinned_init(init, slot) }
@@ -359,14 +369,13 @@
 //!     struct __Unpin<'__pin> {
 //!         __phantom_pin: ::core::marker::PhantomData<fn(&'__pin ()) -> &'__pin ()>,
 //!         __phantom: ::core::marker::PhantomData<fn(Foo) -> Foo>,
-//!         // Since this field is `#[pin]`, it is listed here.
 //!         b: Bar<u32>,
 //!     }
 //!     #[doc(hidden)]
 //!     impl<'__pin> ::core::marker::Unpin for Foo where __Unpin<'__pin>: ::core::marker::Unpin {}
 //!     // Since we specified `PinnedDrop` as the argument to `#[pin_data]`, we expect `Foo` to
 //!     // implement `PinnedDrop`. Thus we do not need to prevent `Drop` implementations like
-//!     // before, instead we implement it here and delegate to `PinnedDrop`.
+//!     // before, instead we implement `Drop` here and delegate to `PinnedDrop`.
 //!     impl ::core::ops::Drop for Foo {
 //!         fn drop(&mut self) {
 //!             // Since we are getting dropped, no one else has a reference to `self` and thus we
@@ -388,7 +397,7 @@
 //!
 //! Here is the `PinnedDrop` impl for `Foo`:
 //!
-//! ```rust
+//! ```rust,ignore
 //! #[pinned_drop]
 //! impl PinnedDrop for Foo {
 //!     fn drop(self: Pin<&mut Self>) {
@@ -399,7 +408,7 @@
 //!
 //! This expands to the following code:
 //!
-//! ```rust
+//! ```rust,ignore
 //! // `unsafe`, full path and the token parameter are added, everything else stays the same.
 //! unsafe impl ::kernel::init::PinnedDrop for Foo {
 //!     fn drop(self: Pin<&mut Self>, _: ::kernel::init::__internal::OnlyCallFromDrop) {
@@ -410,10 +419,10 @@
 //!
 //! ## `pin_init!` on `Foo`
 //!
-//! Since we already took a look at `pin_init!` on `Bar`, this section will only explain the
-//! differences/new things in the expansion of `pin_init!` on `Foo`:
+//! Since we already took a look at `pin_init!` on `Bar`, this section will only show the expansion
+//! of `pin_init!` on `Foo`:
 //!
-//! ```rust
+//! ```rust,ignore
 //! let a = 42;
 //! let initializer = pin_init!(Foo {
 //!     a,
@@ -423,7 +432,7 @@
 //!
 //! This expands to the following code:
 //!
-//! ```rust
+//! ```rust,ignore
 //! let a = 42;
 //! let initializer = {
 //!     struct __InitOk;
@@ -438,13 +447,15 @@
 //!     >(data, move |slot| {
 //!         {
 //!             struct __InitOk;
-//!             unsafe { ::core::ptr::write(&raw mut (*slot).a, a) };
-//!             let a = &unsafe { ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).a) };
+//!             unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).a), a) };
+//!             let a = &unsafe {
+//!                 ::kernel::init::__internal::DropGuard::new(::core::addr_of_mut!((*slot).a))
+//!             };
 //!             let b = Bar::new(36);
-//!             // Here we use `data` to access the correct field and require that `b` is of type
-//!             // `PinInit<Bar<u32>, Infallible>`.
-//!             unsafe { data.b(&raw mut (*slot).b, b)? };
-//!             let b = &unsafe { ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).b) };
+//!             unsafe { data.b(::core::addr_of_mut!((*slot).b), b)? };
+//!             let b = &unsafe {
+//!                 ::kernel::init::__internal::DropGuard::new(::core::addr_of_mut!((*slot).b))
+//!             };
 //!
 //!             #[allow(unreachable_code, clippy::diverging_sub_expression)]
 //!             if false {
--
2.40.0

Re: [PATCH 1/4] rust: macros: fix usage of `#[allow]` in `quote!`

[Martin Rodriguez Reboredo]

On 4/24/23 05:11, Benno Lossin wrote:
> When using `quote!` as part of an expression that was not the last one
> in a function, the `#[allow(clippy::vec_init_then_push)]` attribute
> would be present on an expression, which is not allowed.
> This patch refactors that part of the macro to use a statement instead.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
>  rust/macros/quote.rs | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
> index c8e08b3c1e4c..dddbb4e6f4cb 100644
> --- a/rust/macros/quote.rs
> +++ b/rust/macros/quote.rs
> @@ -39,12 +39,14 @@ impl ToTokens for TokenStream {
>  /// [`quote_spanned!`](https://docs.rs/quote/latest/quote/macro.quote_spanned.html) macro from the
>  /// `quote` crate but provides only just enough functionality needed by the current `macros` crate.
>  macro_rules! quote_spanned {
> -    ($span:expr => $($tt:tt)*) => {
> -    #[allow(clippy::vec_init_then_push)]
> -    {
> -        let mut tokens = ::std::vec::Vec::new();
> -        let span = $span;
> -        quote_spanned!(@proc tokens span $($tt)*);
> +    ($span:expr => $($tt:tt)*) => {{
> +        let mut tokens;
> +        #[allow(clippy::vec_init_then_push)]
> +        {
> +            tokens = ::std::vec::Vec::new();
> +            let span = $span;
> +            quote_spanned!(@proc tokens span $($tt)*);
> +        }
>          ::proc_macro::TokenStream::from_iter(tokens)
>      }};
>      (@proc $v:ident $span:ident) => {};
> 
> base-commit: ea76e08f4d901a450619831a255e9e0a4c0ed162
> --
> 2.40.0
> 
> 

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH 2/4] rust: macros: refactor generics parsing of `#[pin_data]` into its own function

[Martin Rodriguez Reboredo]

On 4/24/23 05:11, Benno Lossin wrote:
> Other macros might also want to parse generics. Additionally this makes
> the code easier to read, as the next commit will introduce more code in
> `#[pin_data]`. Also add more comments to explain how parsing generics
> work.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
>  rust/macros/helpers.rs  | 86 ++++++++++++++++++++++++++++++++++++++++-
>  rust/macros/pin_data.rs | 70 +++++----------------------------
>  2 files changed, 94 insertions(+), 62 deletions(-)
> 
> diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
> index b2bdd4d8c958..afb0f2e3a36a 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, TokenTree};
> +use proc_macro::{token_stream, Group, Punct, Spacing, TokenStream, TokenTree};
> 
>  pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
>      if let Some(TokenTree::Ident(ident)) = it.next() {
> @@ -69,3 +69,87 @@ pub(crate) fn expect_end(it: &mut token_stream::IntoIter) {
>          panic!("Expected end");
>      }
>  }
> +
> +pub(crate) struct Generics {
> +    pub(crate) impl_generics: Vec<TokenTree>,
> +    pub(crate) ty_generics: Vec<TokenTree>,
> +}
> +
> +/// Parses the given `TokenStream` into `Generics` and the rest.
> +///
> +/// The generics are not present in the rest, but a where clause might remain.
> +pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
> +    // `impl_generics`, the declared generics with their bounds.
> +    let mut impl_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.
> +    let mut rest = vec![];
> +    // The current level of `<`.
> +    let mut nesting = 0;
> +    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 {
> +        match tt.clone() {
> +            TokenTree::Punct(p) if p.as_char() == '<' => {
> +                if nesting >= 1 {
> +                    // This is inside of the generics and part of some bound.
> +                    impl_generics.push(tt);
> +                }
> +                nesting += 1;
> +            }
> +            TokenTree::Punct(p) if p.as_char() == '>' => {
> +                // This is a parsing error, so we just end it here.
> +                if nesting == 0 {
> +                    break;
> +                } else {
> +                    nesting -= 1;
> +                    if nesting >= 1 {
> +                        // We are still inside of the generics and part of some bound.
> +                        impl_generics.push(tt);
> +                    }
> +                    if nesting == 0 {
> +                        break;
> +                    }
> +                }
> +            }
> +            tt => {
> +                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());
> +                        }
> +                        _ => {}
> +                    }
> +                }
> +                if nesting >= 1 {
> +                    impl_generics.push(tt);
> +                } else if nesting == 0 {
> +                    // If we haven't entered the generics yet, we still want to keep these tokens.
> +                    rest.push(tt);
> +                }
> +            }
> +        }
> +    }
> +    rest.extend(toks);
> +    (
> +        Generics {
> +            impl_generics,
> +            ty_generics,
> +        },
> +        rest,
> +    )
> +}
> diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
> index 954149d77181..c593b05d9e8c 100644
> --- a/rust/macros/pin_data.rs
> +++ b/rust/macros/pin_data.rs
> @@ -1,71 +1,19 @@
>  // SPDX-License-Identifier: Apache-2.0 OR MIT
> 
> -use proc_macro::{Punct, Spacing, TokenStream, TokenTree};
> +use crate::helpers::{parse_generics, Generics};
> +use proc_macro::TokenStream;
> 
>  pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
>      // This proc-macro only does some pre-parsing and then delegates the actual parsing to
>      // `kernel::__pin_data!`.
> -    //
> -    // In here we only collect the generics, since parsing them in declarative macros is very
> -    // elaborate. We also do not need to analyse their structure, we only need to collect them.
> 
> -    // `impl_generics`, the declared generics with their bounds.
> -    let mut impl_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 `impl` token.
> -    let mut rest = vec![];
> -    // The current level of `<`.
> -    let mut nesting = 0;
> -    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 {
> -        match tt.clone() {
> -            TokenTree::Punct(p) if p.as_char() == '<' => {
> -                if nesting >= 1 {
> -                    impl_generics.push(tt);
> -                }
> -                nesting += 1;
> -            }
> -            TokenTree::Punct(p) if p.as_char() == '>' => {
> -                if nesting == 0 {
> -                    break;
> -                } else {
> -                    nesting -= 1;
> -                    if nesting >= 1 {
> -                        impl_generics.push(tt);
> -                    }
> -                    if nesting == 0 {
> -                        break;
> -                    }
> -                }
> -            }
> -            tt => {
> -                if nesting == 1 {
> -                    match &tt {
> -                        TokenTree::Ident(i) if i.to_string() == "const" => {}
> -                        TokenTree::Ident(_) if at_start => {
> -                            ty_generics.push(tt.clone());
> -                            ty_generics.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
> -                            at_start = false;
> -                        }
> -                        TokenTree::Punct(p) if p.as_char() == ',' => at_start = true,
> -                        TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
> -                            ty_generics.push(tt.clone());
> -                        }
> -                        _ => {}
> -                    }
> -                }
> -                if nesting >= 1 {
> -                    impl_generics.push(tt);
> -                } else if nesting == 0 {
> -                    rest.push(tt);
> -                }
> -            }
> -        }
> -    }
> -    rest.extend(toks);
> +    let (
> +        Generics {
> +            impl_generics,
> +            ty_generics,
> +        },
> +        mut rest,
> +    ) = parse_generics(input);
>      // This should be the body of the struct `{...}`.
>      let last = rest.pop();
>      quote!(::kernel::__pin_data! {
> --
> 2.40.0
> 
> 

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH 3/4] rust: macros: replace Self with the concrete type in #[pin_data]

[Martin Rodriguez Reboredo]

On 4/24/23 05:11, Benno Lossin wrote:
> When using `#[pin_data]` on a struct that used `Self` in the field
> types, a type error would be emitted when trying to use `pin_init!`.
> Since an internal type would be referenced by `Self` instead of the
> defined struct.
> This patch fixes this issue by replacing all occurrences of `Self` in
> the `#[pin_data]` macro with the concrete type circumventing the issue.
> Since rust allows type definitions inside of blocks, which are
> expressions, the macro also checks for these and emits a compile error
> when it finds `trait`, `enum`, `union`, `struct` or `impl`. These
> keywords allow creating new `Self` contexts, which conflicts with the
> current implementation of replacing every `Self` ident. If these were
> allowed, some `Self` idents would be replaced incorrectly.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> Reported-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/macros/pin_data.rs | 108 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 104 insertions(+), 4 deletions(-)
> 
> diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
> index c593b05d9e8c..6d58cfda9872 100644
> --- a/rust/macros/pin_data.rs
> +++ b/rust/macros/pin_data.rs
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: Apache-2.0 OR MIT
> 
>  use crate::helpers::{parse_generics, Generics};
> -use proc_macro::TokenStream;
> +use proc_macro::{Group, Punct, Spacing, TokenStream, TokenTree};
> 
>  pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
>      // This proc-macro only does some pre-parsing and then delegates the actual parsing to
> @@ -12,16 +12,116 @@ pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
>              impl_generics,
>              ty_generics,
>          },
> -        mut rest,
> +        rest,
>      ) = parse_generics(input);
> +    // The struct definition might contain the `Self` type. Since `__pin_data!` will define a new
> +    // type with the same generics and bounds, this poses a problem, since `Self` will refer to the
> +    // new type as opposed to this struct definition. Therefore we have to replace `Self` with the
> +    // concrete name.
> +
> +    // Errors that occur when replacing `Self` with `struct_name`.
> +    let mut errs = TokenStream::new();
> +    // The name of the struct with ty_generics.
> +    let struct_name = rest
> +        .iter()
> +        .skip_while(|tt| !matches!(tt, TokenTree::Ident(i) if i.to_string() == "struct"))
> +        .nth(1)
> +        .and_then(|tt| match tt {
> +            TokenTree::Ident(_) => {
> +                let tt = tt.clone();
> +                let mut res = vec![tt];
> +                if !ty_generics.is_empty() {
> +                    // We add this, so it is maximally compatible with e.g. `Self::CONST` which
> +                    // will be replaced by `StructName::<$generics>::CONST`.
> +                    res.push(TokenTree::Punct(Punct::new(':', Spacing::Joint)));
> +                    res.push(TokenTree::Punct(Punct::new(':', Spacing::Alone)));
> +                    res.push(TokenTree::Punct(Punct::new('<', Spacing::Alone)));
> +                    res.extend(ty_generics.iter().cloned());
> +                    res.push(TokenTree::Punct(Punct::new('>', Spacing::Alone)));
> +                }
> +                Some(res)
> +            }
> +            _ => None,
> +        })
> +        .unwrap_or_else(|| {
> +            // If we did not find the name of the struct then we will use `Self` as the replacement
> +            // and add a compile error to ensure it does not compile.
> +            errs.extend(
> +                "::core::compile_error!(\"Could not locate type name.\");"
> +                    .parse::<TokenStream>()
> +                    .unwrap(),
> +            );
> +            "Self".parse::<TokenStream>().unwrap().into_iter().collect()
> +        });
> +    let impl_generics = impl_generics
> +        .into_iter()
> +        .flat_map(|tt| replace_self_and_deny_type_defs(&struct_name, tt, &mut errs))
> +        .collect::<Vec<_>>();
> +    let mut rest = rest
> +        .into_iter()
> +        .flat_map(|tt| {
> +            // We ignore top level `struct` tokens, since they would emit a compile error.
> +            if matches!(&tt, TokenTree::Ident(i) if i.to_string() == "struct") {
> +                vec![tt]
> +            } else {
> +                replace_self_and_deny_type_defs(&struct_name, tt, &mut errs)
> +            }
> +        })
> +        .collect::<Vec<_>>();
>      // This should be the body of the struct `{...}`.
>      let last = rest.pop();
> -    quote!(::kernel::__pin_data! {
> +    let mut quoted = quote!(::kernel::__pin_data! {
>          parse_input:
>          @args(#args),
>          @sig(#(#rest)*),
>          @impl_generics(#(#impl_generics)*),
>          @ty_generics(#(#ty_generics)*),
>          @body(#last),
> -    })
> +    });
> +    quoted.extend(errs);
> +    quoted
> +}
> +
> +/// Replaces `Self` with `struct_name` and errors on `enum`, `trait`, `struct` `union` and `impl`
> +/// keywords.
> +///
> +/// The error is appended to `errs` to allow normal parsing to continue.
> +fn replace_self_and_deny_type_defs(
> +    struct_name: &Vec<TokenTree>,
> +    tt: TokenTree,
> +    errs: &mut TokenStream,
> +) -> Vec<TokenTree> {
> +    match tt {
> +        TokenTree::Ident(ref i)
> +            if i.to_string() == "enum"
> +                || i.to_string() == "trait"
> +                || i.to_string() == "struct"
> +                || i.to_string() == "union"
> +                || i.to_string() == "impl" =>
> +        {
> +            errs.extend(
> +                format!(
> +                    "::core::compile_error!(\"Cannot use `{i}` inside of struct definition with \
> +                        `#[pin_data]`.\");"
> +                )
> +                .parse::<TokenStream>()
> +                .unwrap()
> +                .into_iter()
> +                .map(|mut tok| {
> +                    tok.set_span(tt.span());
> +                    tok
> +                }),
> +            );
> +            vec![tt]
> +        }
> +        TokenTree::Ident(i) if i.to_string() == "Self" => struct_name.clone(),
> +        TokenTree::Literal(_) | TokenTree::Punct(_) | TokenTree::Ident(_) => vec![tt],
> +        TokenTree::Group(g) => vec![TokenTree::Group(Group::new(
> +            g.delimiter(),
> +            g.stream()
> +                .into_iter()
> +                .flat_map(|tt| replace_self_and_deny_type_defs(struct_name, tt, errs))
> +                .collect(),
> +        ))],
> +    }
>  }
> --
> 2.40.0
> 
> 

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH 4/4] rust: init: update macro expansion example in docs

[Martin Rodriguez Reboredo]

On 4/24/23 05:11, Benno Lossin wrote:
> Also improve the explaining comments.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
>  rust/kernel/init/macros.rs | 85 +++++++++++++++++++++-----------------
>  1 file changed, 48 insertions(+), 37 deletions(-)
> 
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 541cfad1d8be..00aa4e956c0a 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -16,8 +16,9 @@
>  //!
>  //! We will look at the following example:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! # use kernel::init::*;
> +//! # use core::pin::Pin;
>  //! #[pin_data]
>  //! #[repr(C)]
>  //! struct Bar<T> {
> @@ -71,11 +72,12 @@
>  //!
>  //! Here is the definition of `Bar` from our example:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! # use kernel::init::*;
>  //! #[pin_data]
>  //! #[repr(C)]
>  //! struct Bar<T> {
> +//!     #[pin]
>  //!     t: T,
>  //!     pub x: usize,
>  //! }
> @@ -83,7 +85,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! // Firstly the normal definition of the struct, attributes are preserved:
>  //! #[repr(C)]
>  //! struct Bar<T> {
> @@ -116,20 +118,22 @@
>  //!         unsafe fn t<E>(
>  //!             self,
>  //!             slot: *mut T,
> -//!             init: impl ::kernel::init::Init<T, E>,
> +//!             // Since `t` is `#[pin]`, this is `PinInit`.
> +//!             init: impl ::kernel::init::PinInit<T, E>,
>  //!         ) -> ::core::result::Result<(), E> {
> -//!             unsafe { ::kernel::init::Init::__init(init, slot) }
> +//!             unsafe { ::kernel::init::PinInit::__pinned_init(init, slot) }
>  //!         }
>  //!         pub unsafe fn x<E>(
>  //!             self,
>  //!             slot: *mut usize,
> +//!             // Since `x` is not `#[pin]`, this is `Init`.
>  //!             init: impl ::kernel::init::Init<usize, E>,
>  //!         ) -> ::core::result::Result<(), E> {
>  //!             unsafe { ::kernel::init::Init::__init(init, slot) }
>  //!         }
>  //!     }
>  //!     // Implement the internal `HasPinData` trait that associates `Bar` with the pin-data struct
> -//!     // that we constructed beforehand.
> +//!     // that we constructed above.
>  //!     unsafe impl<T> ::kernel::init::__internal::HasPinData for Bar<T> {
>  //!         type PinData = __ThePinData<T>;
>  //!         unsafe fn __pin_data() -> Self::PinData {
> @@ -160,6 +164,8 @@
>  //!     struct __Unpin<'__pin, T> {
>  //!         __phantom_pin: ::core::marker::PhantomData<fn(&'__pin ()) -> &'__pin ()>,
>  //!         __phantom: ::core::marker::PhantomData<fn(Bar<T>) -> Bar<T>>,
> +//!         // Our only `#[pin]` field is `t`.
> +//!         t: T,
>  //!     }
>  //!     #[doc(hidden)]
>  //!     impl<'__pin, T>
> @@ -193,7 +199,7 @@
>  //!
>  //! Here is the impl on `Bar` defining the new function:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! impl<T> Bar<T> {
>  //!     fn new(t: T) -> impl PinInit<Self> {
>  //!         pin_init!(Self { t, x: 0 })
> @@ -203,7 +209,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! impl<T> Bar<T> {
>  //!     fn new(t: T) -> impl PinInit<Self> {
>  //!         {
> @@ -232,25 +238,31 @@
>  //!                     // that will refer to this struct instead of the one defined above.
>  //!                     struct __InitOk;
>  //!                     // This is the expansion of `t,`, which is syntactic sugar for `t: t,`.
> -//!                     unsafe { ::core::ptr::write(&raw mut (*slot).t, t) };
> +//!                     unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).t), t) };
>  //!                     // Since initialization could fail later (not in this case, since the error
> -//!                     // type is `Infallible`) we will need to drop this field if it fails. This
> -//!                     // `DropGuard` will drop the field when it gets dropped and has not yet
> -//!                     // been forgotten. We make a reference to it, so users cannot `mem::forget`
> -//!                     // it from the initializer, since the name is the same as the field.
> +//!                     // type is `Infallible`) we will need to drop this field if there is an
> +//!                     // error later. This `DropGuard` will drop the field when it gets dropped
> +//!                     // and has not yet been forgotten. We make a reference to it, so users
> +//!                     // cannot `mem::forget` it from the initializer, since the name is the same
> +//!                     // as the field (including hygiene).
>  //!                     let t = &unsafe {
> -//!                         ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).t)
> +//!                         ::kernel::init::__internal::DropGuard::new(
> +//!                             ::core::addr_of_mut!((*slot).t),
> +//!                         )
>  //!                     };
>  //!                     // Expansion of `x: 0,`:
>  //!                     // Since this can be an arbitrary expression we cannot place it inside of
>  //!                     // the `unsafe` block, so we bind it here.
>  //!                     let x = 0;
> -//!                     unsafe { ::core::ptr::write(&raw mut (*slot).x, x) };
> +//!                     unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).x), x) };
> +//!                     // We again create a `DropGuard`.
>  //!                     let x = &unsafe {
> -//!                         ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).x)
> +//!                         ::kernel::init::__internal::DropGuard::new(
> +//!                             ::core::addr_of_mut!((*slot).x),
> +//!                         )
>  //!                     };
>  //!
> -//!                     // Here we use the type checker to ensuer that every field has been
> +//!                     // Here we use the type checker to ensure that every field has been
>  //!                     // initialized exactly once, since this is `if false` it will never get
>  //!                     // executed, but still type-checked.
>  //!                     // Additionally we abuse `slot` to automatically infer the correct type for
> @@ -272,7 +284,7 @@
>  //!                         };
>  //!                     }
>  //!                     // Since initialization has successfully completed, we can now forget the
> -//!                     // guards.
> +//!                     // guards. This is not `mem::forget`, since we only have `&DropGuard`.
>  //!                     unsafe { ::kernel::init::__internal::DropGuard::forget(t) };
>  //!                     unsafe { ::kernel::init::__internal::DropGuard::forget(x) };
>  //!                 }
> @@ -280,7 +292,7 @@
>  //!                 // `__InitOk` that we need to return.
>  //!                 Ok(__InitOk)
>  //!             });
> -//!             // Change the return type of the closure.
> +//!             // Change the return type from `__InitOk` to `()`.
>  //!             let init = move |slot| -> ::core::result::Result<(), ::core::convert::Infallible> {
>  //!                 init(slot).map(|__InitOk| ())
>  //!             };
> @@ -299,7 +311,7 @@
>  //! Since we already took a look at `#[pin_data]` on `Bar`, this section will only explain the
>  //! differences/new things in the expansion of the `Foo` definition:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! #[pin_data(PinnedDrop)]
>  //! struct Foo {
>  //!     a: usize,
> @@ -310,7 +322,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! struct Foo {
>  //!     a: usize,
>  //!     b: Bar<u32>,
> @@ -330,8 +342,6 @@
>  //!         unsafe fn b<E>(
>  //!             self,
>  //!             slot: *mut Bar<u32>,
> -//!             // Note that this is `PinInit` instead of `Init`, this is because `b` is
> -//!             // structurally pinned, as marked by the `#[pin]` attribute.
>  //!             init: impl ::kernel::init::PinInit<Bar<u32>, E>,
>  //!         ) -> ::core::result::Result<(), E> {
>  //!             unsafe { ::kernel::init::PinInit::__pinned_init(init, slot) }
> @@ -359,14 +369,13 @@
>  //!     struct __Unpin<'__pin> {
>  //!         __phantom_pin: ::core::marker::PhantomData<fn(&'__pin ()) -> &'__pin ()>,
>  //!         __phantom: ::core::marker::PhantomData<fn(Foo) -> Foo>,
> -//!         // Since this field is `#[pin]`, it is listed here.
>  //!         b: Bar<u32>,
>  //!     }
>  //!     #[doc(hidden)]
>  //!     impl<'__pin> ::core::marker::Unpin for Foo where __Unpin<'__pin>: ::core::marker::Unpin {}
>  //!     // Since we specified `PinnedDrop` as the argument to `#[pin_data]`, we expect `Foo` to
>  //!     // implement `PinnedDrop`. Thus we do not need to prevent `Drop` implementations like
> -//!     // before, instead we implement it here and delegate to `PinnedDrop`.
> +//!     // before, instead we implement `Drop` here and delegate to `PinnedDrop`.
>  //!     impl ::core::ops::Drop for Foo {
>  //!         fn drop(&mut self) {
>  //!             // Since we are getting dropped, no one else has a reference to `self` and thus we
> @@ -388,7 +397,7 @@
>  //!
>  //! Here is the `PinnedDrop` impl for `Foo`:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! #[pinned_drop]
>  //! impl PinnedDrop for Foo {
>  //!     fn drop(self: Pin<&mut Self>) {
> @@ -399,7 +408,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! // `unsafe`, full path and the token parameter are added, everything else stays the same.
>  //! unsafe impl ::kernel::init::PinnedDrop for Foo {
>  //!     fn drop(self: Pin<&mut Self>, _: ::kernel::init::__internal::OnlyCallFromDrop) {
> @@ -410,10 +419,10 @@
>  //!
>  //! ## `pin_init!` on `Foo`
>  //!
> -//! Since we already took a look at `pin_init!` on `Bar`, this section will only explain the
> -//! differences/new things in the expansion of `pin_init!` on `Foo`:
> +//! Since we already took a look at `pin_init!` on `Bar`, this section will only show the expansion
> +//! of `pin_init!` on `Foo`:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! let a = 42;
>  //! let initializer = pin_init!(Foo {
>  //!     a,
> @@ -423,7 +432,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! let a = 42;
>  //! let initializer = {
>  //!     struct __InitOk;
> @@ -438,13 +447,15 @@
>  //!     >(data, move |slot| {
>  //!         {
>  //!             struct __InitOk;
> -//!             unsafe { ::core::ptr::write(&raw mut (*slot).a, a) };
> -//!             let a = &unsafe { ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).a) };
> +//!             unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).a), a) };
> +//!             let a = &unsafe {
> +//!                 ::kernel::init::__internal::DropGuard::new(::core::addr_of_mut!((*slot).a))
> +//!             };
>  //!             let b = Bar::new(36);
> -//!             // Here we use `data` to access the correct field and require that `b` is of type
> -//!             // `PinInit<Bar<u32>, Infallible>`.
> -//!             unsafe { data.b(&raw mut (*slot).b, b)? };
> -//!             let b = &unsafe { ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).b) };
> +//!             unsafe { data.b(::core::addr_of_mut!((*slot).b), b)? };
> +//!             let b = &unsafe {
> +//!                 ::kernel::init::__internal::DropGuard::new(::core::addr_of_mut!((*slot).b))
> +//!             };
>  //!
>  //!             #[allow(unreachable_code, clippy::diverging_sub_expression)]
>  //!             if false {
> --
> 2.40.0
> 
> 

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

Re: [PATCH 1/4] rust: macros: fix usage of `#[allow]` in `quote!`

[Alice Ryhl]

On 4/24/23 10:11, Benno Lossin wrote:
> When using `quote!` as part of an expression that was not the last one
> in a function, the `#[allow(clippy::vec_init_then_push)]` attribute
> would be present on an expression, which is not allowed.
> This patch refactors that part of the macro to use a statement instead.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 2/4] rust: macros: refactor generics parsing of `#[pin_data]` into its own function

[Alice Ryhl]

On 4/24/23 10:11, Benno Lossin wrote:
> Other macros might also want to parse generics. Additionally this makes
> the code easier to read, as the next commit will introduce more code in
> `#[pin_data]`. Also add more comments to explain how parsing generics
> work.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

> +/// Parses the given `TokenStream` into `Generics` and the rest.
> +///
> +/// The generics are not present in the rest, but a where clause might remain.
> +pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
> +    // `impl_generics`, the declared generics with their bounds.
> +    let mut impl_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.
> +    let mut rest = vec![];
> +    // The current level of `<`.
> +    let mut nesting = 0;
> +    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 {
> +        match tt.clone() {

Do you need the call to `clone` here? Not a big deal either way.

Re: [PATCH 3/4] rust: macros: replace Self with the concrete type in #[pin_data]

[Alice Ryhl]

On 4/24/23 10:11, Benno Lossin wrote:
> When using `#[pin_data]` on a struct that used `Self` in the field
> types, a type error would be emitted when trying to use `pin_init!`.
> Since an internal type would be referenced by `Self` instead of the
> defined struct.
> This patch fixes this issue by replacing all occurrences of `Self` in
> the `#[pin_data]` macro with the concrete type circumventing the issue.
> Since rust allows type definitions inside of blocks, which are
> expressions, the macro also checks for these and emits a compile error
> when it finds `trait`, `enum`, `union`, `struct` or `impl`. These
> keywords allow creating new `Self` contexts, which conflicts with the
> current implementation of replacing every `Self` ident. If these were
> allowed, some `Self` idents would be replaced incorrectly.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> Reported-by: Alice Ryhl <aliceryhl@google.com>

It seems like you're missing a newline in the commit message?

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 4/4] rust: init: update macro expansion example in docs

[Alice Ryhl]

On 4/24/23 10:11, Benno Lossin wrote:
> Also improve the explaining comments.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 1/4] rust: macros: fix usage of `#[allow]` in `quote!`

[Gary Guo]

On Mon, 24 Apr 2023 08:11:33 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

> When using `quote!` as part of an expression that was not the last one
> in a function, the `#[allow(clippy::vec_init_then_push)]` attribute
> would be present on an expression, which is not allowed.
> This patch refactors that part of the macro to use a statement instead.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/macros/quote.rs | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/macros/quote.rs b/rust/macros/quote.rs
> index c8e08b3c1e4c..dddbb4e6f4cb 100644
> --- a/rust/macros/quote.rs
> +++ b/rust/macros/quote.rs
> @@ -39,12 +39,14 @@ impl ToTokens for TokenStream {
>  /// [`quote_spanned!`](https://docs.rs/quote/latest/quote/macro.quote_spanned.html) macro from the
>  /// `quote` crate but provides only just enough functionality needed by the current `macros` crate.
>  macro_rules! quote_spanned {
> -    ($span:expr => $($tt:tt)*) => {
> -    #[allow(clippy::vec_init_then_push)]
> -    {
> -        let mut tokens = ::std::vec::Vec::new();
> -        let span = $span;
> -        quote_spanned!(@proc tokens span $($tt)*);
> +    ($span:expr => $($tt:tt)*) => {{
> +        let mut tokens;
> +        #[allow(clippy::vec_init_then_push)]
> +        {
> +            tokens = ::std::vec::Vec::new();
> +            let span = $span;
> +            quote_spanned!(@proc tokens span $($tt)*);
> +        }
>          ::proc_macro::TokenStream::from_iter(tokens)
>      }};
>      (@proc $v:ident $span:ident) => {};
> 
> base-commit: ea76e08f4d901a450619831a255e9e0a4c0ed162
> --
> 2.40.0
> 
> 

Re: [PATCH 2/4] rust: macros: refactor generics parsing of `#[pin_data]` into its own function

[Gary Guo]

On Mon, 24 Apr 2023 08:11:38 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

> Other macros might also want to parse generics. Additionally this makes
> the code easier to read, as the next commit will introduce more code in
> `#[pin_data]`. Also add more comments to explain how parsing generics
> work.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/macros/helpers.rs  | 86 ++++++++++++++++++++++++++++++++++++++++-
>  rust/macros/pin_data.rs | 70 +++++----------------------------
>  2 files changed, 94 insertions(+), 62 deletions(-)
> 
> diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
> index b2bdd4d8c958..afb0f2e3a36a 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, TokenTree};
> +use proc_macro::{token_stream, Group, Punct, Spacing, TokenStream, TokenTree};
> 
>  pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
>      if let Some(TokenTree::Ident(ident)) = it.next() {
> @@ -69,3 +69,87 @@ pub(crate) fn expect_end(it: &mut token_stream::IntoIter) {
>          panic!("Expected end");
>      }
>  }
> +
> +pub(crate) struct Generics {
> +    pub(crate) impl_generics: Vec<TokenTree>,
> +    pub(crate) ty_generics: Vec<TokenTree>,
> +}
> +
> +/// Parses the given `TokenStream` into `Generics` and the rest.
> +///
> +/// The generics are not present in the rest, but a where clause might remain.
> +pub(crate) fn parse_generics(input: TokenStream) -> (Generics, Vec<TokenTree>) {
> +    // `impl_generics`, the declared generics with their bounds.
> +    let mut impl_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.
> +    let mut rest = vec![];
> +    // The current level of `<`.
> +    let mut nesting = 0;
> +    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 {
> +        match tt.clone() {
> +            TokenTree::Punct(p) if p.as_char() == '<' => {
> +                if nesting >= 1 {
> +                    // This is inside of the generics and part of some bound.
> +                    impl_generics.push(tt);
> +                }
> +                nesting += 1;
> +            }
> +            TokenTree::Punct(p) if p.as_char() == '>' => {
> +                // This is a parsing error, so we just end it here.
> +                if nesting == 0 {
> +                    break;
> +                } else {
> +                    nesting -= 1;
> +                    if nesting >= 1 {
> +                        // We are still inside of the generics and part of some bound.
> +                        impl_generics.push(tt);
> +                    }
> +                    if nesting == 0 {
> +                        break;
> +                    }
> +                }
> +            }
> +            tt => {
> +                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());
> +                        }
> +                        _ => {}
> +                    }
> +                }
> +                if nesting >= 1 {
> +                    impl_generics.push(tt);
> +                } else if nesting == 0 {
> +                    // If we haven't entered the generics yet, we still want to keep these tokens.
> +                    rest.push(tt);
> +                }
> +            }
> +        }
> +    }
> +    rest.extend(toks);
> +    (
> +        Generics {
> +            impl_generics,
> +            ty_generics,
> +        },
> +        rest,
> +    )
> +}
> diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
> index 954149d77181..c593b05d9e8c 100644
> --- a/rust/macros/pin_data.rs
> +++ b/rust/macros/pin_data.rs
> @@ -1,71 +1,19 @@
>  // SPDX-License-Identifier: Apache-2.0 OR MIT
> 
> -use proc_macro::{Punct, Spacing, TokenStream, TokenTree};
> +use crate::helpers::{parse_generics, Generics};
> +use proc_macro::TokenStream;
> 
>  pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
>      // This proc-macro only does some pre-parsing and then delegates the actual parsing to
>      // `kernel::__pin_data!`.
> -    //
> -    // In here we only collect the generics, since parsing them in declarative macros is very
> -    // elaborate. We also do not need to analyse their structure, we only need to collect them.
> 
> -    // `impl_generics`, the declared generics with their bounds.
> -    let mut impl_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 `impl` token.
> -    let mut rest = vec![];
> -    // The current level of `<`.
> -    let mut nesting = 0;
> -    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 {
> -        match tt.clone() {
> -            TokenTree::Punct(p) if p.as_char() == '<' => {
> -                if nesting >= 1 {
> -                    impl_generics.push(tt);
> -                }
> -                nesting += 1;
> -            }
> -            TokenTree::Punct(p) if p.as_char() == '>' => {
> -                if nesting == 0 {
> -                    break;
> -                } else {
> -                    nesting -= 1;
> -                    if nesting >= 1 {
> -                        impl_generics.push(tt);
> -                    }
> -                    if nesting == 0 {
> -                        break;
> -                    }
> -                }
> -            }
> -            tt => {
> -                if nesting == 1 {
> -                    match &tt {
> -                        TokenTree::Ident(i) if i.to_string() == "const" => {}
> -                        TokenTree::Ident(_) if at_start => {
> -                            ty_generics.push(tt.clone());
> -                            ty_generics.push(TokenTree::Punct(Punct::new(',', Spacing::Alone)));
> -                            at_start = false;
> -                        }
> -                        TokenTree::Punct(p) if p.as_char() == ',' => at_start = true,
> -                        TokenTree::Punct(p) if p.as_char() == '\'' && at_start => {
> -                            ty_generics.push(tt.clone());
> -                        }
> -                        _ => {}
> -                    }
> -                }
> -                if nesting >= 1 {
> -                    impl_generics.push(tt);
> -                } else if nesting == 0 {
> -                    rest.push(tt);
> -                }
> -            }
> -        }
> -    }
> -    rest.extend(toks);
> +    let (
> +        Generics {
> +            impl_generics,
> +            ty_generics,
> +        },
> +        mut rest,
> +    ) = parse_generics(input);
>      // This should be the body of the struct `{...}`.
>      let last = rest.pop();
>      quote!(::kernel::__pin_data! {
> --
> 2.40.0
> 
> 

Re: [PATCH 3/4] rust: macros: replace Self with the concrete type in #[pin_data]

[Gary Guo]

On Mon, 24 Apr 2023 08:11:43 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

> When using `#[pin_data]` on a struct that used `Self` in the field
> types, a type error would be emitted when trying to use `pin_init!`.
> Since an internal type would be referenced by `Self` instead of the
> defined struct.
> This patch fixes this issue by replacing all occurrences of `Self` in
> the `#[pin_data]` macro with the concrete type circumventing the issue.
> Since rust allows type definitions inside of blocks, which are
> expressions, the macro also checks for these and emits a compile error
> when it finds `trait`, `enum`, `union`, `struct` or `impl`. These
> keywords allow creating new `Self` contexts, which conflicts with the
> current implementation of replacing every `Self` ident. If these were
> allowed, some `Self` idents would be replaced incorrectly.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> Reported-by: Alice Ryhl <aliceryhl@google.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/macros/pin_data.rs | 108 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 104 insertions(+), 4 deletions(-)
> 
> diff --git a/rust/macros/pin_data.rs b/rust/macros/pin_data.rs
> index c593b05d9e8c..6d58cfda9872 100644
> --- a/rust/macros/pin_data.rs
> +++ b/rust/macros/pin_data.rs
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: Apache-2.0 OR MIT
> 
>  use crate::helpers::{parse_generics, Generics};
> -use proc_macro::TokenStream;
> +use proc_macro::{Group, Punct, Spacing, TokenStream, TokenTree};
> 
>  pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
>      // This proc-macro only does some pre-parsing and then delegates the actual parsing to
> @@ -12,16 +12,116 @@ pub(crate) fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
>              impl_generics,
>              ty_generics,
>          },
> -        mut rest,
> +        rest,
>      ) = parse_generics(input);
> +    // The struct definition might contain the `Self` type. Since `__pin_data!` will define a new
> +    // type with the same generics and bounds, this poses a problem, since `Self` will refer to the
> +    // new type as opposed to this struct definition. Therefore we have to replace `Self` with the
> +    // concrete name.
> +
> +    // Errors that occur when replacing `Self` with `struct_name`.
> +    let mut errs = TokenStream::new();
> +    // The name of the struct with ty_generics.
> +    let struct_name = rest
> +        .iter()
> +        .skip_while(|tt| !matches!(tt, TokenTree::Ident(i) if i.to_string() == "struct"))
> +        .nth(1)
> +        .and_then(|tt| match tt {
> +            TokenTree::Ident(_) => {
> +                let tt = tt.clone();
> +                let mut res = vec![tt];
> +                if !ty_generics.is_empty() {
> +                    // We add this, so it is maximally compatible with e.g. `Self::CONST` which
> +                    // will be replaced by `StructName::<$generics>::CONST`.
> +                    res.push(TokenTree::Punct(Punct::new(':', Spacing::Joint)));
> +                    res.push(TokenTree::Punct(Punct::new(':', Spacing::Alone)));
> +                    res.push(TokenTree::Punct(Punct::new('<', Spacing::Alone)));
> +                    res.extend(ty_generics.iter().cloned());
> +                    res.push(TokenTree::Punct(Punct::new('>', Spacing::Alone)));
> +                }
> +                Some(res)
> +            }
> +            _ => None,
> +        })
> +        .unwrap_or_else(|| {
> +            // If we did not find the name of the struct then we will use `Self` as the replacement
> +            // and add a compile error to ensure it does not compile.
> +            errs.extend(
> +                "::core::compile_error!(\"Could not locate type name.\");"
> +                    .parse::<TokenStream>()
> +                    .unwrap(),
> +            );
> +            "Self".parse::<TokenStream>().unwrap().into_iter().collect()
> +        });
> +    let impl_generics = impl_generics
> +        .into_iter()
> +        .flat_map(|tt| replace_self_and_deny_type_defs(&struct_name, tt, &mut errs))
> +        .collect::<Vec<_>>();
> +    let mut rest = rest
> +        .into_iter()
> +        .flat_map(|tt| {
> +            // We ignore top level `struct` tokens, since they would emit a compile error.
> +            if matches!(&tt, TokenTree::Ident(i) if i.to_string() == "struct") {
> +                vec![tt]
> +            } else {
> +                replace_self_and_deny_type_defs(&struct_name, tt, &mut errs)
> +            }
> +        })
> +        .collect::<Vec<_>>();
>      // This should be the body of the struct `{...}`.
>      let last = rest.pop();
> -    quote!(::kernel::__pin_data! {
> +    let mut quoted = quote!(::kernel::__pin_data! {
>          parse_input:
>          @args(#args),
>          @sig(#(#rest)*),
>          @impl_generics(#(#impl_generics)*),
>          @ty_generics(#(#ty_generics)*),
>          @body(#last),
> -    })
> +    });
> +    quoted.extend(errs);
> +    quoted
> +}
> +
> +/// Replaces `Self` with `struct_name` and errors on `enum`, `trait`, `struct` `union` and `impl`
> +/// keywords.
> +///
> +/// The error is appended to `errs` to allow normal parsing to continue.
> +fn replace_self_and_deny_type_defs(
> +    struct_name: &Vec<TokenTree>,
> +    tt: TokenTree,
> +    errs: &mut TokenStream,
> +) -> Vec<TokenTree> {
> +    match tt {
> +        TokenTree::Ident(ref i)
> +            if i.to_string() == "enum"
> +                || i.to_string() == "trait"
> +                || i.to_string() == "struct"
> +                || i.to_string() == "union"
> +                || i.to_string() == "impl" =>
> +        {
> +            errs.extend(
> +                format!(
> +                    "::core::compile_error!(\"Cannot use `{i}` inside of struct definition with \
> +                        `#[pin_data]`.\");"
> +                )
> +                .parse::<TokenStream>()
> +                .unwrap()
> +                .into_iter()
> +                .map(|mut tok| {
> +                    tok.set_span(tt.span());
> +                    tok
> +                }),
> +            );
> +            vec![tt]
> +        }
> +        TokenTree::Ident(i) if i.to_string() == "Self" => struct_name.clone(),
> +        TokenTree::Literal(_) | TokenTree::Punct(_) | TokenTree::Ident(_) => vec![tt],
> +        TokenTree::Group(g) => vec![TokenTree::Group(Group::new(
> +            g.delimiter(),
> +            g.stream()
> +                .into_iter()
> +                .flat_map(|tt| replace_self_and_deny_type_defs(struct_name, tt, errs))
> +                .collect(),
> +        ))],
> +    }
>  }
> --
> 2.40.0
> 
> 

Re: [PATCH 4/4] rust: init: update macro expansion example in docs

[Gary Guo]

On Mon, 24 Apr 2023 08:11:49 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

> Also improve the explaining comments.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/kernel/init/macros.rs | 85 +++++++++++++++++++++-----------------
>  1 file changed, 48 insertions(+), 37 deletions(-)
> 
> diff --git a/rust/kernel/init/macros.rs b/rust/kernel/init/macros.rs
> index 541cfad1d8be..00aa4e956c0a 100644
> --- a/rust/kernel/init/macros.rs
> +++ b/rust/kernel/init/macros.rs
> @@ -16,8 +16,9 @@
>  //!
>  //! We will look at the following example:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! # use kernel::init::*;
> +//! # use core::pin::Pin;
>  //! #[pin_data]
>  //! #[repr(C)]
>  //! struct Bar<T> {
> @@ -71,11 +72,12 @@
>  //!
>  //! Here is the definition of `Bar` from our example:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! # use kernel::init::*;
>  //! #[pin_data]
>  //! #[repr(C)]
>  //! struct Bar<T> {
> +//!     #[pin]
>  //!     t: T,
>  //!     pub x: usize,
>  //! }
> @@ -83,7 +85,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! // Firstly the normal definition of the struct, attributes are preserved:
>  //! #[repr(C)]
>  //! struct Bar<T> {
> @@ -116,20 +118,22 @@
>  //!         unsafe fn t<E>(
>  //!             self,
>  //!             slot: *mut T,
> -//!             init: impl ::kernel::init::Init<T, E>,
> +//!             // Since `t` is `#[pin]`, this is `PinInit`.
> +//!             init: impl ::kernel::init::PinInit<T, E>,
>  //!         ) -> ::core::result::Result<(), E> {
> -//!             unsafe { ::kernel::init::Init::__init(init, slot) }
> +//!             unsafe { ::kernel::init::PinInit::__pinned_init(init, slot) }
>  //!         }
>  //!         pub unsafe fn x<E>(
>  //!             self,
>  //!             slot: *mut usize,
> +//!             // Since `x` is not `#[pin]`, this is `Init`.
>  //!             init: impl ::kernel::init::Init<usize, E>,
>  //!         ) -> ::core::result::Result<(), E> {
>  //!             unsafe { ::kernel::init::Init::__init(init, slot) }
>  //!         }
>  //!     }
>  //!     // Implement the internal `HasPinData` trait that associates `Bar` with the pin-data struct
> -//!     // that we constructed beforehand.
> +//!     // that we constructed above.
>  //!     unsafe impl<T> ::kernel::init::__internal::HasPinData for Bar<T> {
>  //!         type PinData = __ThePinData<T>;
>  //!         unsafe fn __pin_data() -> Self::PinData {
> @@ -160,6 +164,8 @@
>  //!     struct __Unpin<'__pin, T> {
>  //!         __phantom_pin: ::core::marker::PhantomData<fn(&'__pin ()) -> &'__pin ()>,
>  //!         __phantom: ::core::marker::PhantomData<fn(Bar<T>) -> Bar<T>>,
> +//!         // Our only `#[pin]` field is `t`.
> +//!         t: T,
>  //!     }
>  //!     #[doc(hidden)]
>  //!     impl<'__pin, T>
> @@ -193,7 +199,7 @@
>  //!
>  //! Here is the impl on `Bar` defining the new function:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! impl<T> Bar<T> {
>  //!     fn new(t: T) -> impl PinInit<Self> {
>  //!         pin_init!(Self { t, x: 0 })
> @@ -203,7 +209,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! impl<T> Bar<T> {
>  //!     fn new(t: T) -> impl PinInit<Self> {
>  //!         {
> @@ -232,25 +238,31 @@
>  //!                     // that will refer to this struct instead of the one defined above.
>  //!                     struct __InitOk;
>  //!                     // This is the expansion of `t,`, which is syntactic sugar for `t: t,`.
> -//!                     unsafe { ::core::ptr::write(&raw mut (*slot).t, t) };
> +//!                     unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).t), t) };
>  //!                     // Since initialization could fail later (not in this case, since the error
> -//!                     // type is `Infallible`) we will need to drop this field if it fails. This
> -//!                     // `DropGuard` will drop the field when it gets dropped and has not yet
> -//!                     // been forgotten. We make a reference to it, so users cannot `mem::forget`
> -//!                     // it from the initializer, since the name is the same as the field.
> +//!                     // type is `Infallible`) we will need to drop this field if there is an
> +//!                     // error later. This `DropGuard` will drop the field when it gets dropped
> +//!                     // and has not yet been forgotten. We make a reference to it, so users
> +//!                     // cannot `mem::forget` it from the initializer, since the name is the same
> +//!                     // as the field (including hygiene).
>  //!                     let t = &unsafe {
> -//!                         ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).t)
> +//!                         ::kernel::init::__internal::DropGuard::new(
> +//!                             ::core::addr_of_mut!((*slot).t),
> +//!                         )
>  //!                     };
>  //!                     // Expansion of `x: 0,`:
>  //!                     // Since this can be an arbitrary expression we cannot place it inside of
>  //!                     // the `unsafe` block, so we bind it here.
>  //!                     let x = 0;
> -//!                     unsafe { ::core::ptr::write(&raw mut (*slot).x, x) };
> +//!                     unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).x), x) };
> +//!                     // We again create a `DropGuard`.
>  //!                     let x = &unsafe {
> -//!                         ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).x)
> +//!                         ::kernel::init::__internal::DropGuard::new(
> +//!                             ::core::addr_of_mut!((*slot).x),
> +//!                         )
>  //!                     };
>  //!
> -//!                     // Here we use the type checker to ensuer that every field has been
> +//!                     // Here we use the type checker to ensure that every field has been
>  //!                     // initialized exactly once, since this is `if false` it will never get
>  //!                     // executed, but still type-checked.
>  //!                     // Additionally we abuse `slot` to automatically infer the correct type for
> @@ -272,7 +284,7 @@
>  //!                         };
>  //!                     }
>  //!                     // Since initialization has successfully completed, we can now forget the
> -//!                     // guards.
> +//!                     // guards. This is not `mem::forget`, since we only have `&DropGuard`.
>  //!                     unsafe { ::kernel::init::__internal::DropGuard::forget(t) };
>  //!                     unsafe { ::kernel::init::__internal::DropGuard::forget(x) };
>  //!                 }
> @@ -280,7 +292,7 @@
>  //!                 // `__InitOk` that we need to return.
>  //!                 Ok(__InitOk)
>  //!             });
> -//!             // Change the return type of the closure.
> +//!             // Change the return type from `__InitOk` to `()`.
>  //!             let init = move |slot| -> ::core::result::Result<(), ::core::convert::Infallible> {
>  //!                 init(slot).map(|__InitOk| ())
>  //!             };
> @@ -299,7 +311,7 @@
>  //! Since we already took a look at `#[pin_data]` on `Bar`, this section will only explain the
>  //! differences/new things in the expansion of the `Foo` definition:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! #[pin_data(PinnedDrop)]
>  //! struct Foo {
>  //!     a: usize,
> @@ -310,7 +322,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! struct Foo {
>  //!     a: usize,
>  //!     b: Bar<u32>,
> @@ -330,8 +342,6 @@
>  //!         unsafe fn b<E>(
>  //!             self,
>  //!             slot: *mut Bar<u32>,
> -//!             // Note that this is `PinInit` instead of `Init`, this is because `b` is
> -//!             // structurally pinned, as marked by the `#[pin]` attribute.
>  //!             init: impl ::kernel::init::PinInit<Bar<u32>, E>,
>  //!         ) -> ::core::result::Result<(), E> {
>  //!             unsafe { ::kernel::init::PinInit::__pinned_init(init, slot) }
> @@ -359,14 +369,13 @@
>  //!     struct __Unpin<'__pin> {
>  //!         __phantom_pin: ::core::marker::PhantomData<fn(&'__pin ()) -> &'__pin ()>,
>  //!         __phantom: ::core::marker::PhantomData<fn(Foo) -> Foo>,
> -//!         // Since this field is `#[pin]`, it is listed here.
>  //!         b: Bar<u32>,
>  //!     }
>  //!     #[doc(hidden)]
>  //!     impl<'__pin> ::core::marker::Unpin for Foo where __Unpin<'__pin>: ::core::marker::Unpin {}
>  //!     // Since we specified `PinnedDrop` as the argument to `#[pin_data]`, we expect `Foo` to
>  //!     // implement `PinnedDrop`. Thus we do not need to prevent `Drop` implementations like
> -//!     // before, instead we implement it here and delegate to `PinnedDrop`.
> +//!     // before, instead we implement `Drop` here and delegate to `PinnedDrop`.
>  //!     impl ::core::ops::Drop for Foo {
>  //!         fn drop(&mut self) {
>  //!             // Since we are getting dropped, no one else has a reference to `self` and thus we
> @@ -388,7 +397,7 @@
>  //!
>  //! Here is the `PinnedDrop` impl for `Foo`:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! #[pinned_drop]
>  //! impl PinnedDrop for Foo {
>  //!     fn drop(self: Pin<&mut Self>) {
> @@ -399,7 +408,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! // `unsafe`, full path and the token parameter are added, everything else stays the same.
>  //! unsafe impl ::kernel::init::PinnedDrop for Foo {
>  //!     fn drop(self: Pin<&mut Self>, _: ::kernel::init::__internal::OnlyCallFromDrop) {
> @@ -410,10 +419,10 @@
>  //!
>  //! ## `pin_init!` on `Foo`
>  //!
> -//! Since we already took a look at `pin_init!` on `Bar`, this section will only explain the
> -//! differences/new things in the expansion of `pin_init!` on `Foo`:
> +//! Since we already took a look at `pin_init!` on `Bar`, this section will only show the expansion
> +//! of `pin_init!` on `Foo`:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! let a = 42;
>  //! let initializer = pin_init!(Foo {
>  //!     a,
> @@ -423,7 +432,7 @@
>  //!
>  //! This expands to the following code:
>  //!
> -//! ```rust
> +//! ```rust,ignore
>  //! let a = 42;
>  //! let initializer = {
>  //!     struct __InitOk;
> @@ -438,13 +447,15 @@
>  //!     >(data, move |slot| {
>  //!         {
>  //!             struct __InitOk;
> -//!             unsafe { ::core::ptr::write(&raw mut (*slot).a, a) };
> -//!             let a = &unsafe { ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).a) };
> +//!             unsafe { ::core::ptr::write(::core::addr_of_mut!((*slot).a), a) };
> +//!             let a = &unsafe {
> +//!                 ::kernel::init::__internal::DropGuard::new(::core::addr_of_mut!((*slot).a))
> +//!             };
>  //!             let b = Bar::new(36);
> -//!             // Here we use `data` to access the correct field and require that `b` is of type
> -//!             // `PinInit<Bar<u32>, Infallible>`.
> -//!             unsafe { data.b(&raw mut (*slot).b, b)? };
> -//!             let b = &unsafe { ::kernel::init::__internal::DropGuard::new(&raw mut (*slot).b) };
> +//!             unsafe { data.b(::core::addr_of_mut!((*slot).b), b)? };
> +//!             let b = &unsafe {
> +//!                 ::kernel::init::__internal::DropGuard::new(::core::addr_of_mut!((*slot).b))
> +//!             };
>  //!
>  //!             #[allow(unreachable_code, clippy::diverging_sub_expression)]
>  //!             if false {
> --
> 2.40.0
> 
> 

Re: [PATCH 1/4] rust: macros: fix usage of `#[allow]` in `quote!`

[Miguel Ojeda]

On Mon, Apr 24, 2023 at 10:11 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> When using `quote!` as part of an expression that was not the last one
> in a function, the `#[allow(clippy::vec_init_then_push)]` attribute
> would be present on an expression, which is not allowed.
> This patch refactors that part of the macro to use a statement instead.
>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>

Series applied to `rust-next` (with the newline added) -- thanks everyone!

Cheers,
Miguel