[`macro_metavar_expr_concat`] Allow `concat` in repetitions by c410-f3r · Pull Request #127720 · rust-lang/rust

c410-f3r · 2024-07-14T14:51:04Z

rustbot · 2024-07-14T14:51:12Z

rustbot has assigned @cjgillot.
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

Noratrieb · 2024-07-14T16:03:08Z

I created a new issue for the problem and referenced that in your description, your previous "fixes" would have closed the tracking issue.

c410-f3r

@rustbot review

compiler/rustc_expand/src/mbe/metavar_expr.rs

compiler/rustc_expand/src/mbe/transcribe.rs

bors · 2024-07-18T06:50:24Z

☔ The latest upstream changes (presumably #127892) made this pull request unmergeable. Please resolve the merge conflicts.

c410-f3r · 2024-07-18T11:10:59Z

@cjgillot If preferable, the dice of rustbot can be re-rolled

cjgillot · 2024-07-19T22:36:55Z

compiler/rustc_expand/src/mbe/metavar_expr.rs

-        match *self {
-            MetaVarExpr::Count(ident, _) | MetaVarExpr::Ignore(ident) => Some(ident),
-            MetaVarExpr::Concat { .. } | MetaVarExpr::Index(..) | MetaVarExpr::Len(..) => None,
+    pub(crate) fn metavars<A>(&self, mut aux: A, mut cb: impl FnMut(A, &Ident) -> A) -> A {


Could we get this signature?
This would make the logic a bit easier to read in consumer code.

Suggested change

pub(crate) fn metavars<A>(&self, mut aux: A, mut cb: impl FnMut(A, &Ident) -> A) -> A {

pub(crate) fn for_each_metavar(&self, mut cb: impl FnMut(&Ident)) {

That was my first attempt. Unfortunately, LockstepIterSize::with takes ownership.

The name has been changed.

compiler/rustc_expand/src/mbe/transcribe.rs

cjgillot · 2024-07-19T22:42:56Z

compiler/rustc_expand/src/mbe/transcribe.rs

    ident: Ident,
-    interp: &FxHashMap<MacroRulesNormalizedIdent, NamedMatch>,
+    pnr: &ParseNtResult,
 ) -> PResult<'a, Symbol> {


Could you make all this a match?

Refactored to use match but it is important to note that the only diff in this function is the removal of the outer most block.

cjgillot · 2024-07-19T22:45:20Z

compiler/rustc_expand/src/mbe/transcribe.rs


 /// Extracts an metavariable symbol that can be an identifier, a token tree or a literal.
-fn extract_var_symbol<'a>(
+fn extract_symbol_from_pnr<'a>(


The jargon is getting a bit obscure. It took me a while to figure what a pnr is. "Parc Naturel Régional"?

Non, ce n'est pas "Parc Naturel Régional" :)

pnr is in the function signature and extract_symbol_from_parse_nt_result would be too long IMO.

cjgillot · 2024-07-19T22:47:33Z

compiler/rustc_expand/src/mbe/transcribe.rs

-fn extract_var_symbol<'a>(
+fn extract_symbol_from_pnr<'a>(
    dcx: DiagCtxtHandle<'a>,
    ident: Ident,


Should we only pass the span? It may be interesting to document that it's meant for error reporting only.

c410-f3r · 2024-07-20T00:01:26Z

@rustbot review

cjgillot · 2024-07-20T11:24:34Z

@bors r+

bors · 2024-07-20T11:24:36Z

📌 Commit b8d4e4d has been approved by cjgillot

It is now in the queue for this repository.

c410-f3r · 2024-07-20T13:56:45Z

Thanks

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#127720 ([`macro_metavar_expr_concat`] Allow `concat` in repetitions) - rust-lang#127734 (Windows: move BSD socket shims to netc) - rust-lang#127752 (Ignore allocation bytes in one more mir-opt test) - rust-lang#127839 (Fix git safe-directory path for docker images) - rust-lang#127867 (Add `wasm32-wasip2` to `build-manifest` tool) - rust-lang#127958 (Cleanup rmake.rs setup in compiletest) - rust-lang#127975 (Fix trait bounds display) - rust-lang#128005 (Remove _tls_used hack) r? `@ghost` `@rustbot` modify labels: rollup

[`macro_metavar_expr_concat`] Allow `concat` in repetitions cc rust-lang#127723

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#127720 ([`macro_metavar_expr_concat`] Allow `concat` in repetitions) - rust-lang#127734 (Windows: move BSD socket shims to netc) - rust-lang#127752 (Ignore allocation bytes in one more mir-opt test) - rust-lang#127839 (Fix git safe-directory path for docker images) - rust-lang#127867 (Add `wasm32-wasip2` to `build-manifest` tool) - rust-lang#127958 (Cleanup rmake.rs setup in compiletest) - rust-lang#127975 (Fix trait bounds display) - rust-lang#128005 (Remove _tls_used hack) r? `@ghost` `@rustbot` modify labels: rollup

…mpiler-errors Rollup of 7 pull requests Successful merges: - rust-lang#126450 (Promote Mac Catalyst targets to Tier 2, and ship with rustup) - rust-lang#127177 (Distribute rustc_codegen_cranelift for arm64 macOS) - rust-lang#127510 (Rewrite `test-float-parse` in Rust) - rust-lang#127720 ([`macro_metavar_expr_concat`] Allow `concat` in repetitions) - rust-lang#127734 (Windows: move BSD socket shims to netc) - rust-lang#127839 (Fix git safe-directory path for docker images) - rust-lang#128005 (Remove _tls_used hack) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#127720 - c410-f3r:concat-rep, r=cjgillot [`macro_metavar_expr_concat`] Allow `concat` in repetitions cc rust-lang#127723

rustbot assigned cjgillot Jul 14, 2024

rustbot added 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. labels Jul 14, 2024

cjgillot 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 Jul 14, 2024

c410-f3r force-pushed the concat-rep branch from cc888dc to 9be3bdd Compare July 15, 2024 14:02

c410-f3r commented Jul 15, 2024

View reviewed changes

compiler/rustc_expand/src/mbe/metavar_expr.rs Outdated Show resolved Hide resolved

compiler/rustc_expand/src/mbe/transcribe.rs Show resolved Hide resolved

compiler/rustc_expand/src/mbe/transcribe.rs Show resolved Hide resolved

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 Jul 15, 2024

c410-f3r force-pushed the concat-rep branch from 9be3bdd to 1bc2747 Compare July 18, 2024 10:56

cjgillot reviewed Jul 19, 2024

View reviewed changes

cjgillot 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 Jul 19, 2024

Allow concat in repetitions

b8d4e4d

c410-f3r force-pushed the concat-rep branch from 1bc2747 to b8d4e4d Compare July 20, 2024 00:00

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 Jul 20, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 20, 2024

matthiaskrgr mentioned this pull request Jul 20, 2024

Rollup of 8 pull requests #128011

Merged

compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jul 20, 2024

Rollup merge of rust-lang#127720 - c410-f3r:concat-rep, r=cjgillot

141ea5c

[`macro_metavar_expr_concat`] Allow `concat` in repetitions cc rust-lang#127723

compiler-errors mentioned this pull request Jul 20, 2024

Rollup of 7 pull requests #128021

Closed

bors merged commit fba6e1e into rust-lang:master Jul 21, 2024

rustbot added this to the 1.81.0 milestone Jul 21, 2024

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 21, 2024

Unrolled build for rust-lang#127720

9d8a698

Rollup merge of rust-lang#127720 - c410-f3r:concat-rep, r=cjgillot [`macro_metavar_expr_concat`] Allow `concat` in repetitions cc rust-lang#127723

cuviper modified the milestones: 1.81.0, 1.82.0 Aug 15, 2024

tgross35 added the F-macro_metavar_expr_concat `#![feature(macro_metavar_expr_concat)]` label Aug 24, 2024

moxian mentioned this pull request Apr 29, 2025

ICE: expand: unimplemented #140479

Closed

tgross35 mentioned this pull request Jun 19, 2025

Macro metavar expr concat doesn't work with nested repetitions #127723

Open

	pub(crate) fn metavars<A>(&self, mut aux: A, mut cb: impl FnMut(A, &Ident) -> A) -> A {
	pub(crate) fn for_each_metavar(&self, mut cb: impl FnMut(&Ident)) {

Uh oh!

Conversation

c410-f3r commented Jul 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jul 14, 2024

Uh oh!

Noratrieb commented Jul 14, 2024

Uh oh!

c410-f3r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bors commented Jul 18, 2024

Uh oh!

c410-f3r commented Jul 18, 2024

Uh oh!

cjgillot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

c410-f3r Jul 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cjgillot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

c410-f3r Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

cjgillot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

c410-f3r Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

cjgillot Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

c410-f3r Jul 19, 2024

Choose a reason for hiding this comment

Uh oh!

c410-f3r commented Jul 20, 2024

Uh oh!

cjgillot commented Jul 20, 2024

Uh oh!

bors commented Jul 20, 2024

Uh oh!

c410-f3r commented Jul 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

c410-f3r commented Jul 14, 2024 •

edited

Loading

c410-f3r Jul 20, 2024 •

edited

Loading