[PATCH 3/4] rust: macros: Enable use from macro_rules!

Ethan D. Twardy posted 4 patches 1 year, 5 months ago
There is a newer version of this series
[PATCH 3/4] rust: macros: Enable use from macro_rules!
Posted by Ethan D. Twardy 1 year, 5 months ago
According to the rustdoc for the proc_macro crate[1], tokens captured
from a "macro variable" (e.g. from within macro_rules!) may be delimited
by invisible tokens and be contained within a proc_macro::Group.
Previously, this scenario was not handled by macros::paste, which caused
a proc-macro panic when the corresponding tests are enabled. Enable the
tests, and handle this case by making macros::paste::concat recursive.

[1]: https://doc.rust-lang.org/stable/proc_macro/enum.Delimiter.html

Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index d8bd34c0ba89..8afed8facb21 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -269,12 +269,26 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
 ///
 /// # Example
 ///
-/// ```ignore
-/// use kernel::macro::paste;
+/// ```
+/// # const binder_driver_return_protocol_BR_OK: u32 = 0;
+/// # const binder_driver_return_protocol_BR_ERROR: u32 = 1;
+/// # const binder_driver_return_protocol_BR_TRANSACTION: u32 = 2;
+/// # const binder_driver_return_protocol_BR_REPLY: u32 = 3;
+/// # const binder_driver_return_protocol_BR_DEAD_REPLY: u32 = 4;
+/// # const binder_driver_return_protocol_BR_TRANSACTION_COMPLETE: u32 = 5;
+/// # const binder_driver_return_protocol_BR_INCREFS: u32 = 6;
+/// # const binder_driver_return_protocol_BR_ACQUIRE: u32 = 7;
+/// # const binder_driver_return_protocol_BR_RELEASE: u32 = 8;
+/// # const binder_driver_return_protocol_BR_DECREFS: u32 = 9;
+/// # const binder_driver_return_protocol_BR_NOOP: u32 = 10;
+/// # const binder_driver_return_protocol_BR_SPAWN_LOOPER: u32 = 11;
+/// # const binder_driver_return_protocol_BR_DEAD_BINDER: u32 = 12;
+/// # const binder_driver_return_protocol_BR_CLEAR_DEATH_NOTIFICATION_DONE: u32 = 13;
+/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
 ///
 /// macro_rules! pub_no_prefix {
 ///     ($prefix:ident, $($newname:ident),+) => {
-///         paste! {
+///         kernel::macros::paste! {
 ///             $(pub(crate) const $newname: u32 = [<$prefix $newname>];)+
 ///         }
 ///     };
@@ -313,13 +327,29 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
 /// * `lower`: change the identifier to lower case.
 /// * `upper`: change the identifier to upper case.
 ///
-/// ```ignore
-/// use kernel::macro::paste;
+/// ```rust
+/// # const binder_driver_return_protocol_BR_OK: u32 = 0;
+/// # const binder_driver_return_protocol_BR_ERROR: u32 = 1;
+/// # const binder_driver_return_protocol_BR_TRANSACTION: u32 = 2;
+/// # const binder_driver_return_protocol_BR_REPLY: u32 = 3;
+/// # const binder_driver_return_protocol_BR_DEAD_REPLY: u32 = 4;
+/// # const binder_driver_return_protocol_BR_TRANSACTION_COMPLETE: u32 = 5;
+/// # const binder_driver_return_protocol_BR_INCREFS: u32 = 6;
+/// # const binder_driver_return_protocol_BR_ACQUIRE: u32 = 7;
+/// # const binder_driver_return_protocol_BR_RELEASE: u32 = 8;
+/// # const binder_driver_return_protocol_BR_DECREFS: u32 = 9;
+/// # const binder_driver_return_protocol_BR_NOOP: u32 = 10;
+/// # const binder_driver_return_protocol_BR_SPAWN_LOOPER: u32 = 11;
+/// # const binder_driver_return_protocol_BR_DEAD_BINDER: u32 = 12;
+/// # const binder_driver_return_protocol_BR_CLEAR_DEATH_NOTIFICATION_DONE: u32 = 13;
+/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
 ///
 /// macro_rules! pub_no_prefix {
 ///     ($prefix:ident, $($newname:ident),+) => {
 ///         kernel::macros::paste! {
-///             $(pub(crate) const fn [<$newname:lower:span>]: u32 = [<$prefix $newname:span>];)+
+///             $(pub(crate) const fn [<$newname:lower:span>]() -> u32 {
+///             [<$prefix $newname:span>]
+///             })+
 ///         }
 ///     };
 /// }
@@ -350,7 +380,7 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
 ///
 /// Literals can also be concatenated with other identifiers:
 ///
-/// ```ignore
+/// ```rust
 /// macro_rules! create_numbered_fn {
 ///     ($name:literal, $val:literal) => {
 ///         kernel::macros::paste! {
diff --git a/rust/macros/paste.rs b/rust/macros/paste.rs
index f40d42b35b58..6529a387673f 100644
--- a/rust/macros/paste.rs
+++ b/rust/macros/paste.rs
@@ -2,7 +2,7 @@
 
 use proc_macro::{Delimiter, Group, Ident, Spacing, Span, TokenTree};
 
-fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
+fn concat_helper(tokens: &[TokenTree]) -> Vec<(String, Span)> {
     let mut tokens = tokens.iter();
     let mut segments = Vec::new();
     let mut span = None;
@@ -46,12 +46,21 @@ fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
                 };
                 segments.push((value, sp));
             }
-            _ => panic!("unexpected token in paste segments"),
+            Some(TokenTree::Group(group)) if group.delimiter() == Delimiter::None => {
+                let tokens = group.stream().into_iter().collect::<Vec<TokenTree>>();
+                segments.append(&mut concat_helper(tokens.as_slice()));
+            }
+            token => panic!("unexpected token in paste segments: {:?}", token),
         };
     }
 
+    segments
+}
+
+fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
+    let segments = concat_helper(tokens);
     let pasted: String = segments.into_iter().map(|x| x.0).collect();
-    TokenTree::Ident(Ident::new(&pasted, span.unwrap_or(group_span)))
+    TokenTree::Ident(Ident::new(&pasted, group_span))
 }
 
 pub(crate) fn expand(tokens: &mut Vec<TokenTree>) {
-- 
2.44.2
Re: [PATCH 3/4] rust: macros: Enable use from macro_rules!
Posted by Alice Ryhl 1 year, 5 months ago
Ethan D. Twardy <ethan.twardy@gmail.com> writes:
> According to the rustdoc for the proc_macro crate[1], tokens captured
> from a "macro variable" (e.g. from within macro_rules!) may be delimited
> by invisible tokens and be contained within a proc_macro::Group.
> Previously, this scenario was not handled by macros::paste, which caused
> a proc-macro panic when the corresponding tests are enabled. Enable the
> tests, and handle this case by making macros::paste::concat recursive.

The actual change looks good to me.

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

However, I have a bunch of formatting nits:

> [1]: https://doc.rust-lang.org/stable/proc_macro/enum.Delimiter.html
> 
> Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

Normally this would be formatted as:

Link: https://doc.rust-lang.org/stable/proc_macro/enum.Delimiter.html [1]
Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index d8bd34c0ba89..8afed8facb21 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -269,12 +269,26 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
>  ///
>  /// # Example
>  ///
> -/// ```ignore
> -/// use kernel::macro::paste;
> +/// ```
> +/// # const binder_driver_return_protocol_BR_OK: u32 = 0;
> +/// # const binder_driver_return_protocol_BR_ERROR: u32 = 1;
> +/// # const binder_driver_return_protocol_BR_TRANSACTION: u32 = 2;
> +/// # const binder_driver_return_protocol_BR_REPLY: u32 = 3;
> +/// # const binder_driver_return_protocol_BR_DEAD_REPLY: u32 = 4;
> +/// # const binder_driver_return_protocol_BR_TRANSACTION_COMPLETE: u32 = 5;
> +/// # const binder_driver_return_protocol_BR_INCREFS: u32 = 6;
> +/// # const binder_driver_return_protocol_BR_ACQUIRE: u32 = 7;
> +/// # const binder_driver_return_protocol_BR_RELEASE: u32 = 8;
> +/// # const binder_driver_return_protocol_BR_DECREFS: u32 = 9;
> +/// # const binder_driver_return_protocol_BR_NOOP: u32 = 10;
> +/// # const binder_driver_return_protocol_BR_SPAWN_LOOPER: u32 = 11;
> +/// # const binder_driver_return_protocol_BR_DEAD_BINDER: u32 = 12;
> +/// # const binder_driver_return_protocol_BR_CLEAR_DEATH_NOTIFICATION_DONE: u32 = 13;
> +/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
>  ///
>  /// macro_rules! pub_no_prefix {
>  ///     ($prefix:ident, $($newname:ident),+) => {

There's a non-hidden empty line between the last constant and
`macro_rules! pub_no_prefix`. You should either hide the empty line or
get rid of it, because it will look weird when the example is rendered.

> -///         paste! {
> +///         kernel::macros::paste! {

Another option would be to keep the import so that the empty line
separates the import from the macro declaration.

>  ///             $(pub(crate) const $newname: u32 = [<$prefix $newname>];)+
>  ///         }
>  ///     };
> @@ -313,13 +327,29 @@ pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
>  /// * `lower`: change the identifier to lower case.
>  /// * `upper`: change the identifier to upper case.
>  ///
> -/// ```ignore
> -/// use kernel::macro::paste;
> +/// ```rust
> +/// # const binder_driver_return_protocol_BR_OK: u32 = 0;
> +/// # const binder_driver_return_protocol_BR_ERROR: u32 = 1;
> +/// # const binder_driver_return_protocol_BR_TRANSACTION: u32 = 2;
> +/// # const binder_driver_return_protocol_BR_REPLY: u32 = 3;
> +/// # const binder_driver_return_protocol_BR_DEAD_REPLY: u32 = 4;
> +/// # const binder_driver_return_protocol_BR_TRANSACTION_COMPLETE: u32 = 5;
> +/// # const binder_driver_return_protocol_BR_INCREFS: u32 = 6;
> +/// # const binder_driver_return_protocol_BR_ACQUIRE: u32 = 7;
> +/// # const binder_driver_return_protocol_BR_RELEASE: u32 = 8;
> +/// # const binder_driver_return_protocol_BR_DECREFS: u32 = 9;
> +/// # const binder_driver_return_protocol_BR_NOOP: u32 = 10;
> +/// # const binder_driver_return_protocol_BR_SPAWN_LOOPER: u32 = 11;
> +/// # const binder_driver_return_protocol_BR_DEAD_BINDER: u32 = 12;
> +/// # const binder_driver_return_protocol_BR_CLEAR_DEATH_NOTIFICATION_DONE: u32 = 13;
> +/// # const binder_driver_return_protocol_BR_FAILED_REPLY: u32 = 14;
>  ///
>  /// macro_rules! pub_no_prefix {
>  ///     ($prefix:ident, $($newname:ident),+) => {

Same here.

>  ///         kernel::macros::paste! {
> -///             $(pub(crate) const fn [<$newname:lower:span>]: u32 = [<$prefix $newname:span>];)+
> +///             $(pub(crate) const fn [<$newname:lower:span>]() -> u32 {
> +///             [<$prefix $newname:span>]
> +///             })+

I would probably indent [<$prefix $newname:span>] one more time.

Alice
Re: [PATCH 3/4] rust: macros: Enable use from macro_rules!
Posted by Ethan D. Twardy 1 year, 5 months ago
On Mon Jun 24, 2024 at 3:43 AM CDT, Alice Ryhl wrote:
> The actual change looks good to me.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>

Thank you!

> Normally this would be formatted as:
>
> Link: https://doc.rust-lang.org/stable/proc_macro/enum.Delimiter.html [1]
> Signed-off-by: Ethan D. Twardy <ethan.twardy@gmail.com>

Ah, thank you for informing me on this :).

> There's a non-hidden empty line between the last constant and
> `macro_rules! pub_no_prefix`. You should either hide the empty line or
> get rid of it, because it will look weird when the example is rendered.
>
>
> Another option would be to keep the import so that the empty line
> separates the import from the macro declaration.

Done! (And the one other identical one, as well).

> I would probably indent [<$prefix $newname:span>] one more time.
>
> Alice

Done, as well. As with PATCH 2/4, these will ship with v2.