2023-10-08 09:56:21

by Trevor Gross

[permalink] [raw]
Subject: [PATCH] rust: macros: update 'paste!' macro to accept string literals

Enable combining identifiers with string literals in the 'paste!' macro.
This allows combining user-specified strings with affixes to create
namespaced identifiers.

This sample code:

macro_rules! m {
($name:lit) => {
paste!(struct [<_some_ $name _struct_>];)
}
}

m!("foo_bar");

Would previously cause a compilation error. It will now generate:

struct _some_foo_bar_struct_;

Reported-by: FUJITA Tomonori <[email protected]>
Signed-off-by: Trevor Gross <[email protected]>
---

Original mention of this problem in [1]

[1]: https://lore.kernel.org/rust-for-linux/[email protected]/

rust/macros/paste.rs | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/rust/macros/paste.rs b/rust/macros/paste.rs
index 385a78434224..f40d42b35b58 100644
--- a/rust/macros/paste.rs
+++ b/rust/macros/paste.rs
@@ -9,7 +9,15 @@ fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
loop {
match tokens.next() {
None => break,
- Some(TokenTree::Literal(lit)) => segments.push((lit.to_string(), lit.span())),
+ Some(TokenTree::Literal(lit)) => {
+ // Allow us to concat string literals by stripping quotes
+ let mut value = lit.to_string();
+ if value.starts_with('"') && value.ends_with('"') {
+ value.remove(0);
+ value.pop();
+ }
+ segments.push((value, lit.span()));
+ }
Some(TokenTree::Ident(ident)) => {
let mut value = ident.to_string();
if value.starts_with("r#") {
--
2.34.1


Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On 10/8/23 06:48, Trevor Gross wrote:
> Enable combining identifiers with string literals in the 'paste!' macro.
> This allows combining user-specified strings with affixes to create
> namespaced identifiers.
>
> This sample code:
>
> macro_rules! m {
> ($name:lit) => {
> paste!(struct [<_some_ $name _struct_>];)
> }
> }
>
> m!("foo_bar");
>
> Would previously cause a compilation error. It will now generate:
>
> struct _some_foo_bar_struct_;
>
> Reported-by: FUJITA Tomonori <[email protected]>
> Signed-off-by: Trevor Gross <[email protected]>
> ---
>
> Original mention of this problem in [1]
>
> [1]: https://lore.kernel.org/rust-for-linux/[email protected]/

Next time I think you should put this in `Fixes:`.

> [...]
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>

2023-10-09 03:04:15

by Trevor Gross

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On Sun, Oct 8, 2023 at 8:27 AM Martin Rodriguez Reboredo
<[email protected]> wrote:
>
> Next time I think you should put this in `Fixes:`.
>
> > [...]
> Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
>

Good point, thanks! I'll add that if there is a v2 (or Miguel can
probably add it if not)

2023-10-09 08:45:01

by Vincenzo Palazzo

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

> This allows combining user-specified strings with affixes to create
> namespaced identifiers.
>
> This sample code:
>
> macro_rules! m {
> ($name:lit) => {
> paste!(struct [<_some_ $name _struct_>];)
> }
> }
>
> m!("foo_bar");
>
> Would previously cause a compilation error. It will now generate:
>
> struct _some_foo_bar_struct_;
>
> Reported-by: FUJITA Tomonori <[email protected]>
> Signed-off-by: Trevor Gross <[email protected]>
> ---

Reviewed-by: Vincenzo Palazzo <[email protected]>

2023-10-09 10:02:54

by Alice Ryhl

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On Sun, Oct 8, 2023 at 11:51 AM Trevor Gross <[email protected]> wrote:
> Enable combining identifiers with string literals in the 'paste!' macro.
> This allows combining user-specified strings with affixes to create
> namespaced identifiers.
>
> This sample code:
>
> macro_rules! m {
> ($name:lit) => {
> paste!(struct [<_some_ $name _struct_>];)
> }
> }
>
> m!("foo_bar");
>
> Would previously cause a compilation error. It will now generate:
>
> struct _some_foo_bar_struct_;
>
> Reported-by: FUJITA Tomonori <[email protected]>
> Signed-off-by: Trevor Gross <[email protected]>

Reviewed-by: Alice Ryhl <[email protected]>

2023-10-09 10:49:59

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On Mon, Oct 9, 2023 at 5:04 AM Trevor Gross <[email protected]> wrote:
>
> Good point, thanks! I'll add that if there is a v2 (or Miguel can
> probably add it if not)

Yes, I add them myself when I notice they are missing (e.g. most
recently 2 of the ones in `rust-fixes`), but patches should definitely
come with the `Fixes: ` tag themselves, i.e. it should be the
exceptional case.

However, is this actually a fix? The title and commit message make it
sound like it is a new feature rather than a fix. And the docs of the
macro says literals are not supported, right?

So this probably needs to update those docs too (and ideally add an
example with the newly supported construct too). Or am I
misunderstanding?

Cheers,
Miguel

2023-10-09 14:52:02

by Benno Lossin

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On 08.10.23 11:48, Trevor Gross wrote:
> Enable combining identifiers with string literals in the 'paste!' macro.
> This allows combining user-specified strings with affixes to create
> namespaced identifiers.
>
> This sample code:
>
> macro_rules! m {
> ($name:lit) => {
> paste!(struct [<_some_ $name _struct_>];)
> }
> }
>
> m!("foo_bar");
>
> Would previously cause a compilation error. It will now generate:
>
> struct _some_foo_bar_struct_;
>
> Reported-by: FUJITA Tomonori <[email protected]>
> Signed-off-by: Trevor Gross <[email protected]>

Reviewed-by: Benno Lossin <[email protected]>

2023-10-09 19:15:05

by Trevor Gross

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On Mon, Oct 9, 2023 at 6:49 AM Miguel Ojeda
<[email protected]> wrote:
>
> On Mon, Oct 9, 2023 at 5:04 AM Trevor Gross <[email protected]> wrote:
> >
> > Good point, thanks! I'll add that if there is a v2 (or Miguel can
> > probably add it if not)
>
> Yes, I add them myself when I notice they are missing (e.g. most
> recently 2 of the ones in `rust-fixes`), but patches should definitely
> come with the `Fixes: ` tag themselves, i.e. it should be the
> exceptional case.
>
> However, is this actually a fix? The title and commit message make it
> sound like it is a new feature rather than a fix. And the docs of the
> macro says literals are not supported, right?

I suppose it is something that augments current behavior and "fixes"
the linked use case by making it possible. I am not sure what
qualifies as a fix.

> So this probably needs to update those docs too (and ideally add an
> example with the newly supported construct too). Or am I
> misunderstanding?
>
> Cheers,
> Miguel

I will update the documentation, thanks for the catch.

2023-10-12 20:46:28

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On Mon, Oct 9, 2023 at 9:14 PM Trevor Gross <[email protected]> wrote:
>
> I suppose it is something that augments current behavior and "fixes"
> the linked use case by making it possible. I am not sure what
> qualifies as a fix.

`Fixes` is meant for issues/bugs. So if the macro was broken, i.e. it
does not do what it says it would, it would be a fix.

But if I understand correctly, the docs say this was not supported, so
it is not a fix, it is just expanding the feature set.

Similarly, `Reported-by` is not meant for feature requests.

> I will update the documentation, thanks for the catch.

Thanks!

Cheers,
Miguel

2023-10-24 12:55:14

by Gary Guo

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On Sun, 8 Oct 2023 05:48:18 -0400
Trevor Gross <[email protected]> wrote:

> Enable combining identifiers with string literals in the 'paste!' macro.
> This allows combining user-specified strings with affixes to create
> namespaced identifiers.
>
> This sample code:
>
> macro_rules! m {
> ($name:lit) => {
> paste!(struct [<_some_ $name _struct_>];)
> }
> }
>
> m!("foo_bar");
>
> Would previously cause a compilation error. It will now generate:
>
> struct _some_foo_bar_struct_;
>
> Reported-by: FUJITA Tomonori <[email protected]>
> Signed-off-by: Trevor Gross <[email protected]>

Reviewed-by: Gary Guo <[email protected]>

> ---
>
> Original mention of this problem in [1]
>
> [1]: https://lore.kernel.org/rust-for-linux/[email protected]/
>
> rust/macros/paste.rs | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/rust/macros/paste.rs b/rust/macros/paste.rs
> index 385a78434224..f40d42b35b58 100644
> --- a/rust/macros/paste.rs
> +++ b/rust/macros/paste.rs
> @@ -9,7 +9,15 @@ fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
> loop {
> match tokens.next() {
> None => break,
> - Some(TokenTree::Literal(lit)) => segments.push((lit.to_string(), lit.span())),
> + Some(TokenTree::Literal(lit)) => {
> + // Allow us to concat string literals by stripping quotes
> + let mut value = lit.to_string();
> + if value.starts_with('"') && value.ends_with('"') {
> + value.remove(0);
> + value.pop();
> + }
> + segments.push((value, lit.span()));
> + }
> Some(TokenTree::Ident(ident)) => {
> let mut value = ident.to_string();
> if value.starts_with("r#") {

2023-10-24 16:52:18

by Boqun Feng

[permalink] [raw]
Subject: Re: [PATCH] rust: macros: update 'paste!' macro to accept string literals

On Sun, Oct 08, 2023 at 05:48:18AM -0400, Trevor Gross wrote:
> Enable combining identifiers with string literals in the 'paste!' macro.
> This allows combining user-specified strings with affixes to create
> namespaced identifiers.
>
> This sample code:
>
> macro_rules! m {
> ($name:lit) => {
> paste!(struct [<_some_ $name _struct_>];)
> }
> }
>
> m!("foo_bar");
>
> Would previously cause a compilation error. It will now generate:
>
> struct _some_foo_bar_struct_;
>
> Reported-by: FUJITA Tomonori <[email protected]>
> Signed-off-by: Trevor Gross <[email protected]>

This looks good to me, but could you (in a follow-up patch mabye) add an
example demonstrating the usage, which could also serve as a test if we
can run doctest for macro doc. Thanks!

Regards,
Boqun

> ---
>
> Original mention of this problem in [1]
>
> [1]: https://lore.kernel.org/rust-for-linux/[email protected]/
>
> rust/macros/paste.rs | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/rust/macros/paste.rs b/rust/macros/paste.rs
> index 385a78434224..f40d42b35b58 100644
> --- a/rust/macros/paste.rs
> +++ b/rust/macros/paste.rs
> @@ -9,7 +9,15 @@ fn concat(tokens: &[TokenTree], group_span: Span) -> TokenTree {
> loop {
> match tokens.next() {
> None => break,
> - Some(TokenTree::Literal(lit)) => segments.push((lit.to_string(), lit.span())),
> + Some(TokenTree::Literal(lit)) => {
> + // Allow us to concat string literals by stripping quotes
> + let mut value = lit.to_string();
> + if value.starts_with('"') && value.ends_with('"') {
> + value.remove(0);
> + value.pop();
> + }
> + segments.push((value, lit.span()));
> + }
> Some(TokenTree::Ident(ident)) => {
> let mut value = ident.to_string();
> if value.starts_with("r#") {
> --
> 2.34.1
>
>