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 <[email protected]>
---
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
Also improve the explaining comments.
Signed-off-by: Benno Lossin <[email protected]>
---
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
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 <[email protected]>
Reported-by: Alice Ryhl <[email protected]>
---
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
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 <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
> Reported-by: Alice Ryhl <[email protected]>
> ---
> 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 <[email protected]>
On 4/24/23 05:11, Benno Lossin wrote:
> Also improve the explaining comments.
>
> Signed-off-by: Benno Lossin <[email protected]>
> ---
> 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 <[email protected]>
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 <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
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 <[email protected]>
> Reported-by: Alice Ryhl <[email protected]>
It seems like you're missing a newline in the commit message?
Reviewed-by: Alice Ryhl <[email protected]>
On 4/24/23 10:11, Benno Lossin wrote:
> Also improve the explaining comments.
>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
On Mon, 24 Apr 2023 08:11:33 +0000
Benno Lossin <[email protected]> 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 <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> 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
>
>
On Mon, 24 Apr 2023 08:11:43 +0000
Benno Lossin <[email protected]> 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 <[email protected]>
> Reported-by: Alice Ryhl <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> 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
>
>
On Mon, 24 Apr 2023 08:11:49 +0000
Benno Lossin <[email protected]> wrote:
> Also improve the explaining comments.
>
> Signed-off-by: Benno Lossin <[email protected]>
Reviewed-by: Gary Guo <[email protected]>
> ---
> 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
>
>
On Mon, Apr 24, 2023 at 10:11 AM Benno Lossin <[email protected]> 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 <[email protected]>
Series applied to `rust-next` (with the newline added) -- thanks everyone!
Cheers,
Miguel