Skip to content

Commit cbbf21c

Browse files
y86-devy86-dev
committed
rust: macros: rewrite #[pinned_drop] using syn
Now that `syn` and `quote` are available in the Kernel use them for the `#[pinned_drop]` proc-macro attribute instead of the current hybrid proc-macro/declarative approach. Proc-macros written using `syn` are a lot more readable and thus easier to maintain than convoluted declarative macros. An additional advantage of `syn` is the improved error reporting, here is an example: #[pin_data(PinnedDrop)] struct Foo {} #[pinned_drop] impl PinnedDrop for Foo {} Prior to this patch, the error reads: error: no rules expected the token `)` --> samples/rust/rust_minimal.rs:43:1 | 43 | #[pinned_drop] | ^^^^^^^^^^^^^^ no rules expected this token in macro call | note: while trying to match `fn` --> rust/kernel/init/macros.rs:511:13 | 511 | fn drop($($sig:tt)*) { | ^^ = note: this error originates in the attribute macro `pinned_drop` (in Nightly builds, run with -Z macro-backtrace for more info) error[E0277]: the trait bound `Foo: PinnedDrop` is not satisfied --> samples/rust/rust_minimal.rs:40:1 | 40 | #[pin_data(PinnedDrop)] | ^^^^^^^^^^^^^^^^^^^^^^^ | | | the trait `PinnedDrop` is not implemented for `Foo` | required by a bound introduced by this call | = note: this error originates in the macro `$crate::__pin_data` which comes from the expansion of the attribute macro `pin_data` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 2 previous errors After this patch, the error reads: error: Expected exactly one function in the `PinnedDrop` impl. --> samples/rust/rust_minimal.rs:44:25 | 44 | impl PinnedDrop for Foo {} | ^^ error: aborting due to 1 previous error Signed-off-by: Benno Lossin <[email protected]>
1 parent c6b76d9 commit cbbf21c

File tree

3 files changed

+118
-66
lines changed

3 files changed

+118
-66
lines changed

‎rust/kernel/init/macros.rs

-25
Original file line numberDiff line numberDiff line change
@@ -498,31 +498,6 @@
498498
//! };
499499
//! ```
500500
501-
/// Creates a `unsafe impl<...> PinnedDrop for $type` block.
502-
///
503-
/// See [`PinnedDrop`] for more information.
504-
#[doc(hidden)]
505-
#[macro_export]
506-
macro_rules! __pinned_drop {
507-
(
508-
@impl_sig($($impl_sig:tt)*),
509-
@impl_body(
510-
$(#[$($attr:tt)*])*
511-
fn drop($($sig:tt)*) {
512-
$($inner:tt)*
513-
}
514-
),
515-
) => {
516-
unsafe $($impl_sig)* {
517-
// Inherit all attributes and the type/ident tokens for the signature.
518-
$(#[$($attr)*])*
519-
fn drop($($sig)*, _: $crate::init::__internal::OnlyCallFromDrop) {
520-
$($inner)*
521-
}
522-
}
523-
}
524-
}
525-
526501
/// The internal init macro. Do not call manually!
527502
///
528503
/// This is called by the `{try_}{pin_}init!` macros with various inputs.

‎rust/macros/lib.rs

+16-1
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,22 @@ pub fn pin_data(args: TokenStream, input: TokenStream) -> TokenStream {
293293
/// ```
294294
#[proc_macro_attribute]
295295
pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
296-
pinned_drop::pinned_drop(args.into(), input.into()).into()
296+
parse_macro_input!(args as syn::parse::Nothing);
297+
match syn::parse(input.clone()) {
298+
Ok(input) => match pinned_drop::pinned_drop(input) {
299+
Ok(output) => output,
300+
Err((err, impl_)) => {
301+
let err = err.into_compile_error();
302+
quote! {
303+
#impl_
304+
#err
305+
}
306+
}
307+
}
308+
.into(),
309+
// Let the compiler handle the error.
310+
Err(_) => input,
311+
}
297312
}
298313

299314
/// Paste identifiers together.

‎rust/macros/pinned_drop.rs

+102-40
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,112 @@
11
// SPDX-License-Identifier: Apache-2.0 OR MIT
22

3-
use proc_macro2::{TokenStream, TokenTree};
4-
use quote::quote;
3+
use proc_macro2::TokenStream;
4+
use quote::{quote, ToTokens};
5+
use syn::{parse_quote, Error, ImplItem, ImplItemFn, ItemImpl, Token};
56

6-
pub(crate) fn pinned_drop(_args: TokenStream, input: TokenStream) -> TokenStream {
7-
let mut toks = input.into_iter().collect::<Vec<_>>();
8-
assert!(!toks.is_empty());
9-
// Ensure that we have an `impl` item.
10-
assert!(matches!(&toks[0], TokenTree::Ident(i) if i.to_string() == "impl"));
11-
// Ensure that we are implementing `PinnedDrop`.
12-
let mut nesting: usize = 0;
13-
let mut pinned_drop_idx = None;
14-
for (i, tt) in toks.iter().enumerate() {
15-
match tt {
16-
TokenTree::Punct(p) if p.as_char() == '<' => {
17-
nesting += 1;
7+
struct PinnedDropImpl(ItemImpl, ImplItemFn);
8+
9+
impl TryFrom<ItemImpl> for PinnedDropImpl {
10+
type Error = (Error, TokenStream);
11+
fn try_from(mut impl_: ItemImpl) -> Result<Self, Self::Error> {
12+
let aux_impl = match &impl_ {
13+
ItemImpl {
14+
attrs,
15+
impl_token,
16+
generics,
17+
trait_: Some((polarity, path, for_)),
18+
self_ty,
19+
..
20+
} if path.is_ident("PinnedDrop") => {
21+
let (impl_generics, _, whr) = generics.split_for_impl();
22+
quote! {
23+
#(#attrs)*
24+
unsafe #impl_token #impl_generics #polarity ::kernel::init::PinnedDrop #for_ #self_ty
25+
#whr
26+
{
27+
fn drop(
28+
self: ::core::pin::Pin<&mut Self>,
29+
_: ::kernel::init::__internal::OnlyCallFromDrop,
30+
) {}
31+
}
32+
}
33+
}
34+
_ => quote!(#impl_),
35+
};
36+
if impl_.unsafety.is_some() {
37+
return Err((
38+
Error::new_spanned(
39+
impl_.unsafety,
40+
"The `PinnedDrop` impl must not be `unsafe`.",
41+
),
42+
aux_impl,
43+
));
44+
}
45+
let trait_tokens = match &impl_.trait_ {
46+
None => quote!(),
47+
Some((None, path, _)) => quote!(#path),
48+
Some((Some(not), path, _)) => quote!(#not #path),
49+
};
50+
match &impl_.trait_ {
51+
Some((None, path, _)) if path.is_ident("PinnedDrop") => {}
52+
None | Some((None, ..)) => {
53+
return Err((
54+
Error::new_spanned(
55+
trait_tokens,
56+
"`#[pinned_drop]` can only be used on `PinnedDrop` impls.",
57+
),
58+
aux_impl,
59+
));
1860
}
19-
TokenTree::Punct(p) if p.as_char() == '>' => {
20-
nesting = nesting.checked_sub(1).unwrap();
21-
continue;
61+
Some((Some(_), ..)) => {
62+
return Err((
63+
Error::new_spanned(
64+
trait_tokens,
65+
"`#[pinned_drop]` can only be used on positive `PinnedDrop` impls.",
66+
),
67+
aux_impl,
68+
));
2269
}
23-
_ => {}
2470
}
25-
if i >= 1 && nesting == 0 {
26-
// Found the end of the generics, this should be `PinnedDrop`.
27-
assert!(
28-
matches!(tt, TokenTree::Ident(i) if i.to_string() == "PinnedDrop"),
29-
"expected 'PinnedDrop', found: '{:?}'",
30-
tt
31-
);
32-
pinned_drop_idx = Some(i);
33-
break;
71+
if impl_.items.len() != 1 {
72+
return Err((
73+
Error::new(
74+
impl_.brace_token.span.join(),
75+
"Expected exactly one function in the `PinnedDrop` impl.",
76+
),
77+
aux_impl,
78+
));
3479
}
80+
let ImplItem::Fn(drop) = impl_.items.pop().unwrap() else {
81+
return Err((
82+
Error::new(impl_.brace_token.span.join(), "Expected a function."),
83+
aux_impl,
84+
));
85+
};
86+
87+
Ok(Self(impl_, drop))
3588
}
36-
let idx = pinned_drop_idx
37-
.unwrap_or_else(|| panic!("Expected an `impl` block implementing `PinnedDrop`."));
38-
// Fully qualify the `PinnedDrop`, as to avoid any tampering.
39-
toks.splice(idx..idx, quote!(::kernel::init::));
40-
// Take the `{}` body and call the declarative macro.
41-
if let Some(TokenTree::Group(last)) = toks.pop() {
42-
let last = last.stream();
43-
quote!(::kernel::__pinned_drop! {
44-
@impl_sig(#(#toks)*),
45-
@impl_body(#last),
46-
})
47-
} else {
48-
TokenStream::from_iter(toks)
89+
}
90+
91+
impl ToTokens for PinnedDropImpl {
92+
fn to_tokens(&self, tokens: &mut TokenStream) {
93+
self.0.to_tokens(tokens)
4994
}
5095
}
96+
97+
pub(crate) fn pinned_drop(drop_impl: ItemImpl) -> Result<TokenStream, (Error, TokenStream)> {
98+
let PinnedDropImpl(mut drop_impl, mut drop) = drop_impl.try_into()?;
99+
drop.sig
100+
.inputs
101+
.push(parse_quote!(_: ::kernel::init::__internal::OnlyCallFromDrop));
102+
let span = drop_impl.impl_token.span;
103+
104+
drop_impl.items.push(ImplItem::Fn(drop));
105+
drop_impl.unsafety = Some(Token![unsafe](span));
106+
drop_impl.trait_ = Some((
107+
None,
108+
parse_quote!(::kernel::init::PinnedDrop),
109+
Token![for](span),
110+
));
111+
Ok(quote! { #drop_impl })
112+
}

0 commit comments

Comments
 (0)