Skip to content

Conversation

@moatom
Copy link
Contributor

@moatom moatom commented Oct 28, 2025

Fix #140238

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 28, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@moatom moatom force-pushed the stabilize-proc-macro branch from 114e740 to 02f04b0 Compare October 30, 2025 14:12
@rust-log-analyzer

This comment has been minimized.

@moatom moatom force-pushed the stabilize-proc-macro branch from 02f04b0 to 0e341e2 Compare October 30, 2025 15:13
@moatom moatom changed the title WIP: Fix parseing in proc_macro::quote Fix parsing in proc_macro::quote Oct 30, 2025
@moatom moatom marked this pull request as ready for review October 30, 2025 15:14
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 30, 2025
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 30, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@moatom
Copy link
Contributor Author

moatom commented Oct 30, 2025

r? @tgross35

@rustbot rustbot assigned tgross35 and unassigned petrochenkov Oct 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 30, 2025

tgross35 is currently at their maximum review capacity.
They may take a while to respond.

@moatom moatom changed the title Fix parsing in proc_macro::quote Fix parsing logic in proc_macro::quote Oct 30, 2025
@moatom moatom force-pushed the stabilize-proc-macro branch 2 times, most recently from 8dc03cd to 36c13e9 Compare October 30, 2025 15:27
@rust-log-analyzer

This comment has been minimized.

@tgross35
Copy link
Contributor

David knows this much better than I do

r? @dtolnay

@rustbot rustbot assigned dtolnay and unassigned tgross35 Oct 30, 2025
@moatom
Copy link
Contributor Author

moatom commented Oct 31, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 31, 2025

@moatom: 🔑 Insufficient privileges: Not in reviewers

@tgross35
Copy link
Contributor

@bors r+

@moatom what are you trying to do here? This is the command to approve PRs.

@moatom
Copy link
Contributor Author

moatom commented Oct 31, 2025

@tgross35 thx. I wanted to restart CI just in case because this PR compiles locally.

@tgross35
Copy link
Contributor

I restarted it. For future reference, you can either force push something or close+reopen to restart CI (bors doesn’t interact with PR CI)

@moatom moatom force-pushed the stabilize-proc-macro branch from 36c13e9 to 900f0e2 Compare November 1, 2025 08:06
@rustbot
Copy link
Collaborator

rustbot commented Nov 1, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@moatom
Copy link
Contributor Author

moatom commented Nov 1, 2025

@dtolnay I've resolved all of the problems. Could you please confirm when you have a moment?

match tree {
TokenTree::Group(tt) => {
TokenTree::Group(ref tt) => {
// Handles repetition by expanding `$( CONTENTS ) SEP_OPT *` to `{ REP_EXPANDED }`.
Copy link
Member

Choose a reason for hiding this comment

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

The repetition syntax is supposed to be $(CONTENTS) SEP *, like inside macro_rules. Not $[CONTENTS] SEP * and not ${CONTENTS} SEP *, and especially not $ CONTENTS SEP * with Delimiter::None. Please add tests for these.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please add tests for these.

I think it is impossible to prepare it for Delimiter::None.

Copy link
Member

Choose a reason for hiding this comment

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

It can be tested by calling quote! from a macro_rules macro.

macro_rules! do_quote {
    ($dollar:tt $content:expr) => {
        proc_macro::quote!($dollar $content *)
    };
}

let arr = [0; 2];
eprintln!("{}", do_quote!($ f!($arr)));

On current nightly this nonsensically prints f! (0i32) f! (0i32).

]);

minimal_quote!((@ TokenTree::Group(Group::new(Delimiter::Brace, rep_expanded)))).to_tokens(&mut tokens);
consume_dollar_group_sep_star(contents.clone(), &mut iter)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
consume_dollar_group_sep_star(contents.clone(), &mut iter)
consume_dollar_group_sep_star(contents, &mut iter)

Copy link
Member

Choose a reason for hiding this comment

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

This has not been addressed. You made a change on the same line but it is not the suggested one. There is no reason to clone the value of tt.stream(), which is already an owned TokenStream.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2025
@moatom
Copy link
Contributor Author

moatom commented Nov 6, 2025

@dtolnay
Thank you for the review!
I've addressed your comments and left the fix commit separate for traceability.
Please let me know if it's okay to squash.

@moatom moatom requested a review from dtolnay November 6, 2025 15:31
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 6, 2025

fn main() {
let arr = [1, 2, 3];
let _ = quote! { ${$arr}* }; //~ ERROR the trait bound `[{integer}; 3]: ToTokens` is not satisfied [E0277]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let _ = quote! { ${$arr}* }; //~ ERROR the trait bound `[{integer}; 3]: ToTokens` is not satisfied [E0277]
let _ = quote! { $[$arr]* }; //~ ERROR the trait bound `[{integer}; 3]: ToTokens` is not satisfied [E0277]

match &tree {
TokenTree::Punct(tt) if tt.as_char() == '$' => {
if let Some(TokenTree::Ident(id)) = iter.peek() {
while let Some(ref tree) = iter.next() {
Copy link
Member

Choose a reason for hiding this comment

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

This is going to miss adjacent metavariables. For example this works:

let a = ['a'];
let b = ['b'];
eprintln!("{}", proc_macro::quote!($($a . $b)*));

but this does not:

eprintln!("{}", proc_macro::quote!($($a $b)*));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

}

/// Consume a `$( CONTENTS ) SEP *` accordingly. It handles repetition by expanding `$( CONTENTS ) SEP *` to `{ REP_EXPANDED }`.
fn consume_dollar_group_sep_star(
Copy link
Member

Choose a reason for hiding this comment

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

This is accepting too many tokens in the "separator" in some cases.

let arr = ['a', 'b'];
eprintln!("{}", proc_macro::quote!($($arr) $$ x .. false [] *));
'a' $x .. false [] 'b'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I introduced is_valid_sep_prefix to resolve this, but there might be a more elegant solution.

@moatom moatom requested a review from dtolnay November 18, 2025 17:36
@moatom moatom force-pushed the stabilize-proc-macro branch from abc4763 to ae0617b Compare November 18, 2025 17:57
@moatom
Copy link
Contributor Author

moatom commented Nov 18, 2025

r? @dtolnay

@rustbot
Copy link
Collaborator

rustbot commented Nov 18, 2025

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

@rust-log-analyzer

This comment has been minimized.

@moatom moatom force-pushed the stabilize-proc-macro branch from ae0617b to 93fd70a Compare November 19, 2025 00:18
@rust-log-analyzer

This comment has been minimized.

@moatom moatom force-pushed the stabilize-proc-macro branch 2 times, most recently from 6c63c7b to 88f4fbe Compare November 19, 2025 00:48
@moatom moatom force-pushed the stabilize-proc-macro branch from 88f4fbe to 3613f22 Compare November 19, 2025 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for repetition to proc_macro::quote

8 participants