lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 28 Jun 2023 19:03:24 +0000
From:   Björn Roy Baron <bjorn3_gh@...tonmail.com>
To:     Gary Guo <gary@...yguo.net>
Cc:     Miguel Ojeda <ojeda@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Wedson Almeida Filho <wedsonaf@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Benno Lossin <benno.lossin@...ton.me>,
        Andreas Hindborg <a.hindborg@...sung.com>,
        Alice Ryhl <aliceryhl@...gle.com>,
        linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] rust: macros: add `paste!` proc macro

On Wednesday, June 28th, 2023 at 19:11, Gary Guo <gary@...yguo.net> wrote:

> This macro provides a flexible way to concatenated identifiers together
> and it allows the resulting identifier to be used to declare new items,
> which `concat_idents!` does not allow. It also allows identifiers to be
> transformed before concatenated.
> 
> The `concat_idents!` example
> 
>     let x_1 = 42;
>     let x_2 = concat_idents!(x, _1);
>     assert!(x_1 == x_2);
> 
> can be written with `paste!` macro like this:
> 
>     let x_1 = 42;
>     let x_2 = paste!([<x _1>]);
>     assert!(x_1 == x_2);
> 
> However `paste!` macro is more flexible because it can be used to create
> a new variable:
> 
>     let x_1 = 42;
>     paste!(let [<x _2>] = [<x _1>];);
>     assert!(x_1 == x_2);
> 
> While this is not possible with `concat_idents!`.
> 
> This macro is similar to the `paste!` crate [1], but this is a fresh
> implementation to avoid vendoring large amount of code directly. Also, I
> have augmented it to provide a way to specify span of the resulting
> token, allowing precise control.
> 
> For example, this code is broken because the variable is declared inside
> the macro, so Rust macro hygiene rules prevents access from the outside:
> 
>     macro_rules! m {
>         ($id: ident) => {
>             // The resulting token has hygiene of the macro.
>             paste!(let [<$id>] = 1;)
>         }
>     }
> 
>     m!(a);
>     let _ = a;
> 
> In this versionn of `paste!` macro I added a `span` modifier to allow

*version

> this:
> 
>     macro_rules! m {
>         ($id: ident) => {
>             // The resulting token has hygiene of `$id`.
>             paste!(let [<$id:span>] = 1;)
>         }
>     }
> 
>     m!(a);
>     let _ = a;
> 
> Link: http://docs.rs/paste/ [1]
> Signed-off-by: Gary Guo <gary@...yguo.net>

With the typo above fixed:

Reviewed-by: Björn Roy Baron <bjorn3_gh@...tonmail.com>

I have also got a minor suggestion below, but I'm ok with keeping it as is.

> ---
>  rust/macros/lib.rs   | 97 ++++++++++++++++++++++++++++++++++++++++++++
>  rust/macros/paste.rs | 94 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 191 insertions(+)
>  create mode 100644 rust/macros/paste.rs
> 
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 3fc74cb4ea19..b4bc44c27bd4 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -7,6 +7,7 @@
>  mod concat_idents;
>  mod helpers;
>  mod module;
> +mod paste;
>  mod pin_data;
>  mod pinned_drop;
>  mod vtable;
> @@ -246,3 +247,99 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
>  pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
>      pinned_drop::pinned_drop(args, input)
>  }
> +
> +/// Paste identifiers together.
> +///
> +/// Within the `paste!` macro, identifiers inside `[<` and `>]` are concatenated together to form a
> +/// single identifier.
> +///
> +/// This is similar to the [`paste`] crate, but with pasting feature limited to identifiers
> +/// (literals, lifetimes and documentation strings are not supported). There is a difference in
> +/// supported modifiers as well.
> +///
> +/// # Example
> +///
> +/// ```ignore
> +/// use kernel::macro::paste;
> +///
> +/// macro_rules! pub_no_prefix {
> +///     ($prefix:ident, $($newname:ident),+) => {
> +///         paste! {
> +///             $(pub(crate) const $newname: u32 = [<$prefix $newname>];)+
> +///         }
> +///     };
> +/// }
> +///
> +/// pub_no_prefix!(
> +///     binder_driver_return_protocol_,
> +///     BR_OK,
> +///     BR_ERROR,
> +///     BR_TRANSACTION,
> +///     BR_REPLY,
> +///     BR_DEAD_REPLY,
> +///     BR_TRANSACTION_COMPLETE,
> +///     BR_INCREFS,
> +///     BR_ACQUIRE,
> +///     BR_RELEASE,
> +///     BR_DECREFS,
> +///     BR_NOOP,
> +///     BR_SPAWN_LOOPER,
> +///     BR_DEAD_BINDER,
> +///     BR_CLEAR_DEATH_NOTIFICATION_DONE,
> +///     BR_FAILED_REPLY
> +/// );
> +///
> +/// assert_eq!(BR_OK, binder_driver_return_protocol_BR_OK);
> +/// ```
> +///
> +/// # Modifiers
> +///
> +/// For each identifier, it is possible to attach one or multiple modifiers to
> +/// it.
> +///
> +/// Currently supported modifiers are:
> +/// * `span`: change the span of concatenated identifier to the span of the specified token. By
> +/// default the span of the `[< >]` group is used.
> +/// * `lower`: change the identifier to lower case.
> +/// * `upper`: change the identifier to upper case.
> +///
> +/// ```ignore
> +/// use kernel::macro::paste;
> +///
> +/// macro_rules! pub_no_prefix {
> +///     ($prefix:ident, $($newname:ident),+) => {
> +///         kernel::macros::paste! {
> +///             $(pub(crate) const fn [<$newname:lower:span>]: u32 = [<$prefix $newname:span>];)+

Having multiple : when using multiple flags feels a bit weird to me. Maybe
use $newname:lower+span, $newname:lower,span or something like that? If you
prefer the current syntax that is fine with me too though.

> +///         }
> +///     };
> +/// }
> +///
> +/// pub_no_prefix!(
> +///     binder_driver_return_protocol_,
> +///     BR_OK,
> +///     BR_ERROR,
> +///     BR_TRANSACTION,
> +///     BR_REPLY,
> +///     BR_DEAD_REPLY,
> +///     BR_TRANSACTION_COMPLETE,
> +///     BR_INCREFS,
> +///     BR_ACQUIRE,
> +///     BR_RELEASE,
> +///     BR_DECREFS,
> +///     BR_NOOP,
> +///     BR_SPAWN_LOOPER,
> +///     BR_DEAD_BINDER,
> +///     BR_CLEAR_DEATH_NOTIFICATION_DONE,
> +///     BR_FAILED_REPLY
> +/// );
> +///
> +/// assert_eq!(br_ok(), binder_driver_return_protocol_BR_OK);
> +/// ```
> +///
> +/// [`paste`]: https://docs.rs/paste/
> +#[proc_macro]
> +pub fn paste(input: TokenStream) -> TokenStream {
> +    let mut tokens = input.into_iter().collect();
> +    paste::expand(&mut tokens);
> +    tokens.into_iter().collect()
> +}
> diff --git a/rust/macros/paste.rs b/rust/macros/paste.rs
> new file mode 100644
> index 000000000000..42fde0930b05
> --- /dev/null
> +++ b/rust/macros/paste.rs
> @@ -0,0 +1,94 @@
> +use proc_macro::{Delimiter, Group, Ident, Spacing, Span, TokenTree};
> +
> +fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
> +    let mut tokens = tokens.iter();
> +    let mut segments = Vec::new();
> +    let mut span = None;
> +    loop {
> +        match tokens.next() {
> +            None => break,
> +            Some(TokenTree::Literal(lit)) => segments.push((lit.to_string(), lit.span())),
> +            Some(TokenTree::Ident(ident)) => {
> +                let mut value = ident.to_string();
> +                if value.starts_with("r#") {
> +                    value.replace_range(0..2, "");
> +                }
> +                segments.push((value, ident.span()));
> +            }
> +            Some(TokenTree::Punct(p)) if p.as_char() == ':' => {
> +                let Some(TokenTree::Ident(ident)) = tokens.next() else {
> +                    panic!("expected identifier as modifier");
> +                };
> +
> +                let (mut value, sp) = segments.pop().expect("expected identifier before modifier");
> +                match ident.to_string().as_str() {
> +                    // Set the overall span of concatenated token as current span
> +                    "span" => {
> +                        assert!(
> +                            span.is_none(),
> +                            "span modifier should only appear at most once"
> +                        );
> +                        span = Some(sp);
> +                    }
> +                    "lower" => value = value.to_lowercase(),
> +                    "upper" => value = value.to_uppercase(),
> +                    v => panic!("unknown modifier `{v}`"),
> +                };
> +                segments.push((value, sp));
> +            }
> +            _ => panic!("unexpected token in paste segments"),
> +        };
> +    }
> +
> +    let pasted: String = segments.into_iter().map(|x| x.0).collect();
> +    TokenTree::Ident(Ident::new(&pasted, span.unwrap_or(group_span)))
> +}
> +
> +pub(crate) fn expand(tokens: &mut Vec<TokenTree>) {
> +    for token in tokens.iter_mut() {
> +        if let TokenTree::Group(group) = token {
> +            let delimiter = group.delimiter();
> +            let span = group.span();
> +            let mut stream: Vec<_> = group.stream().into_iter().collect();
> +            // Find groups that looks like `[< A B C D >]`
> +            if delimiter == Delimiter::Bracket
> +                && stream.len() >= 3
> +                && matches!(&stream[0], TokenTree::Punct(p) if p.as_char() == '<')
> +                && matches!(&stream[stream.len() - 1], TokenTree::Punct(p) if p.as_char() == '>')
> +            {
> +                // Replace the group with concatenated token
> +                *token = concat(&stream[1..stream.len() - 1], span);
> +            } else {
> +                // Recursively expand tokens inside the group
> +                expand(&mut stream);
> +                let mut group = Group::new(delimiter, stream.into_iter().collect());
> +                group.set_span(span);
> +                *token = TokenTree::Group(group);
> +            }
> +        }
> +    }
> +
> +    // Path segments cannot contain invisible delimiter group, so remove them if any.
> +    for i in (0..tokens.len().saturating_sub(3)).rev() {
> +        // Looking for a double colon
> +        if matches!(
> +            (&tokens[i + 1], &tokens[i + 2]),
> +            (TokenTree::Punct(a), TokenTree::Punct(b))
> +                if a.as_char() == ':' && a.spacing() == Spacing::Joint && b.as_char() == ':'
> +        ) {
> +            match &tokens[i + 3] {
> +                TokenTree::Group(group) if group.delimiter() == Delimiter::None => {
> +                    tokens.splice(i + 3..i + 4, group.stream());
> +                }
> +                _ => (),
> +            }
> +
> +            match &tokens[i] {
> +                TokenTree::Group(group) if group.delimiter() == Delimiter::None => {
> +                    tokens.splice(i..i + 1, group.stream());
> +                }
> +                _ => (),
> +            }
> +        }
> +    }
> +}
> -- 
> 2.34.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ