[`macro_metavar_expr_concat`] Allow `concat` in repetitions #127720

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

    dcx: DiagCtxtHandle<'a>,
    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

@@ -752,39 +763,37 @@ fn transcribe_metavar_expr<'a>(
 }

 /// 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

@@ -752,39 +763,37 @@ fn transcribe_metavar_expr<'a>(
 }

 /// 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>(
    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
6 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`macro_metavar_expr_concat`] Allow `concat` in repetitions #127720

[`macro_metavar_expr_concat`] Allow `concat` in repetitions #127720

c410-f3r commented Jul 14, 2024 •

edited

Loading

rustbot commented Jul 14, 2024

Noratrieb commented Jul 14, 2024

c410-f3r left a comment

bors commented Jul 18, 2024

c410-f3r commented Jul 18, 2024

cjgillot Jul 19, 2024

c410-f3r Jul 20, 2024 •

edited

Loading

cjgillot Jul 19, 2024

c410-f3r Jul 19, 2024

cjgillot Jul 19, 2024

c410-f3r Jul 19, 2024

cjgillot Jul 19, 2024

c410-f3r Jul 19, 2024

c410-f3r commented Jul 20, 2024

cjgillot commented Jul 20, 2024

bors commented Jul 20, 2024

c410-f3r commented Jul 20, 2024

	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)) {

[macro_metavar_expr_concat] Allow concat in repetitions #127720

[macro_metavar_expr_concat] Allow concat in repetitions #127720

Conversation

c410-f3r commented Jul 14, 2024 • edited Loading

rustbot commented Jul 14, 2024

Noratrieb commented Jul 14, 2024

c410-f3r left a comment

Choose a reason for hiding this comment

bors commented Jul 18, 2024

c410-f3r commented Jul 18, 2024

cjgillot Jul 19, 2024

Choose a reason for hiding this comment

c410-f3r Jul 20, 2024 • edited Loading

Choose a reason for hiding this comment

cjgillot Jul 19, 2024

Choose a reason for hiding this comment

c410-f3r Jul 19, 2024

Choose a reason for hiding this comment

cjgillot Jul 19, 2024

Choose a reason for hiding this comment

c410-f3r Jul 19, 2024

Choose a reason for hiding this comment

cjgillot Jul 19, 2024

Choose a reason for hiding this comment

c410-f3r Jul 19, 2024

Choose a reason for hiding this comment

c410-f3r commented Jul 20, 2024

cjgillot commented Jul 20, 2024

bors commented Jul 20, 2024

c410-f3r commented Jul 20, 2024

[`macro_metavar_expr_concat`] Allow `concat` in repetitions #127720

[`macro_metavar_expr_concat`] Allow `concat` in repetitions #127720

c410-f3r commented Jul 14, 2024 •

edited

Loading

c410-f3r Jul 20, 2024 •

edited

Loading