Skip to content

Replace paste by with_builtin_macros. #1394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 9, 2025
Merged

Conversation

maximebuyse
Copy link
Contributor

Fixes #1379.

I looked for a solution without introducing a new dependency. concat_idents seems like a good candidate but it doesn't work for function names as discussed in rust-lang/rust#29599. So I replaced paste by with_builtin_macros which builts on top of concat_idents to provide the functionality we need.

@maximebuyse maximebuyse requested a review from a team as a code owner April 8, 2025 14:52
@maximebuyse maximebuyse requested a review from W95Psp April 8, 2025 14:54
Copy link
Collaborator

@W95Psp W95Psp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good to me, let's see how maintained with_builtin_macros is, but for now that seems a great solution! :)

And that works without nightly, so that's nice.

@Nadrieril
Copy link
Collaborator

Are you aware of ${concat(x, y)}?

Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with_builtin_macros doesn't look great 😬
Did you try the macro_metavar_expr_concat? If that works, I think it would be the better option.

@maximebuyse
Copy link
Contributor Author

Are you aware of ${concat(x, y)}?

Looks like it does what we need, thanks! If we are ok to use something experimental, I'll try it.

@karthikbhargavan
Copy link
Contributor

Not that I am expert, but in the same thread that talks about macro_metavar_expr_concat and ${concat}, the last suggestion, especially for stable rust, appears to be with_builtin_macros?

rust-lang/rust#124225 (comment)

@maximebuyse
Copy link
Contributor Author

Not that I am expert, but in the same thread that talks about macro_metavar_expr_concat and ${concat}, the last suggestion, especially for stable rust, appears to be with_builtin_macros?

rust-lang/rust#124225 (comment)

The suggestion comes from the author of with_builtin_macros though.

Copy link
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are ok to use something experimental, I'll try it.

If it works, I think something experimental here within the language feels better than a 3rd party crate. We're on nightly anyway.

@maximebuyse
Copy link
Contributor Author

If we are ok to use something experimental, I'll try it.

If it works, I think something experimental here within the language feels better than a 3rd party crate. We're on nightly anyway.

Yes it works, let's merge then!

@maximebuyse maximebuyse added this pull request to the merge queue Apr 9, 2025
Merged via the queue into main with commit ae537b5 Apr 9, 2025
16 checks passed
@maximebuyse maximebuyse deleted the remove-paste-dependency branch April 9, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crate paste is unmaintained
5 participants