proc_macro: Stop flattening groups with dummy spans by petrochenkov · Pull Request #73102 · rust-lang/rust

petrochenkov · 2020-06-07T20:27:19Z

Reduce the scope of the hack described in #72545 (comment).

We still pass AST fragments to attribute and derive macros as single nonterminal tokens rather than as tokens streams, but now use a precise flag instead of the span-based heuristic that could do lead to incorrect behavior in unrelated cases.

#73345 attempts to fully resolve this issue, but there are some compatibility issues to be addressed.

petrochenkov · 2020-06-07T20:27:47Z

@bors try
cc @Aaron1011

bors · 2020-06-07T20:28:02Z

⌛ Trying commit d754b6fccd23147e425772d925152c7ef19850be with merge 71b81291baafb8a58836272757b28817fe1d8190...

bors · 2020-06-07T23:19:42Z

☀️ Try build successful - checks-azure
Build commit: 71b81291baafb8a58836272757b28817fe1d8190 (71b81291baafb8a58836272757b28817fe1d8190)

petrochenkov · 2020-06-08T06:24:19Z

@craterbot check

craterbot · 2020-06-08T06:24:22Z

👌 Experiment pr-73102 created and queued.
🤖 Automatically detected try build 71b81291baafb8a58836272757b28817fe1d8190
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-06-11T15:54:33Z

🚧 Experiment pr-73102 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2020-06-14T08:10:12Z

🎉 Experiment pr-73102 is completed!
📊 3 regressed and 0 fixed (107905 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

petrochenkov · 2020-06-14T08:29:21Z

Interesting and surprising.
Assuming the implementation is correct, this means syn and other utilities are fixed and all other regressions in #73084 are caused by hygiene issues rather than grouping?
cc @Aaron1011

petrochenkov · 2020-06-14T16:19:14Z

Closing in favor of #73345 which is simpler and more principled.

petrochenkov · 2020-06-25T21:14:50Z

Rebased and added some comments.
r? @Aaron1011

src/librustc_expand/proc_macro_server.rs

Aaron1011 · 2020-06-25T21:28:09Z

r=me with the nits fixed

petrochenkov · 2020-06-25T21:48:06Z

@bors r=Aaron1011

bors · 2020-06-25T21:48:07Z

📌 Commit 77b0ed7 has been approved by Aaron1011

proc_macro: Stop flattening groups with dummy spans Reduce the scope of the hack described in rust-lang#72545 (comment). We still pass AST fragments to attribute and derive macros as single nonterminal tokens rather than as tokens streams, but now use a precise flag instead of the span-based heuristic that could do lead to incorrect behavior in unrelated cases. rust-lang#73345 attempts to fully resolve this issue, but there are some compatibility issues to be addressed.

@ghost

…arth Rollup of 13 pull requests Successful merges: - rust-lang#72620 (Omit DW_AT_linkage_name when it is the same as DW_AT_name) - rust-lang#72967 (Don't move cursor in search box when using arrows to navigate results) - rust-lang#73102 (proc_macro: Stop flattening groups with dummy spans) - rust-lang#73297 (Support configurable deny-warnings for all in-tree crates.) - rust-lang#73507 (Cleanup MinGW LLVM linkage workaround) - rust-lang#73588 (Fix handling of reserved registers for ARM inline asm) - rust-lang#73597 (Record span of `const` kw in GenericParamKind) - rust-lang#73629 (Make AssocOp Copy) - rust-lang#73681 (Update Chalk to 0.14) - rust-lang#73707 (Fix links in `SliceIndex` documentation) - rust-lang#73719 (emitter: column width defaults to 140) - rust-lang#73729 (disable collectionbenches for android) - rust-lang#73748 (Add code block to code in documentation of `List::rebase_onto`) Failed merges: r? @ghost

nnethercote · 2020-06-30T00:08:58Z

I suspect this may have caused a perf regression. I'm going to try to do a perf CI run after the landing. Not sure if it will work...

@bors try @rust-timer queue

rust-timer · 2020-06-30T00:08:59Z

Awaiting bors try build completion

mati865 · 2020-06-30T00:17:20Z

Bors doesn't listen on closed issues/PRs.

@Aaron1011

expand: Stop using nonterminals for passing tokens to attribute and derive macros Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment). Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later. The cost is making pretty-printing of token streams less pretty. Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway. Unfortunately, this is not exactly the same thing as rust-lang#73102. One more observable effect is how `$crate` is printed in the attribute input. Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs). This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed. So, I think we should do this regardless, but we need to run crater first. r? @Aaron1011

@Aaron1011

expand: Stop using nonterminals for passing tokens to attribute and derive macros Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment). Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later. The cost is making pretty-printing of token streams less pretty. Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway. Unfortunately, this is not exactly the same thing as rust-lang#73102. One more observable effect is how `$crate` is printed in the attribute input. Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs). This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed. So, I think we should do this regardless, but we need to run crater first. r? @Aaron1011

@Aaron1011

expand: Stop using nonterminals for passing tokens to attribute and derive macros Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment). Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later. The cost is making pretty-printing of token streams less pretty. Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway. Unfortunately, this is not exactly the same thing as rust-lang#73102. One more observable effect is how `$crate` is printed in the attribute input. Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs). This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed. So, I think we should do this regardless, but we need to run crater first. r? @Aaron1011

@Aaron1011

expand: Stop using nonterminals for passing tokens to attribute and derive macros Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment). Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later. The cost is making pretty-printing of token streams less pretty. Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway. Unfortunately, this is not exactly the same thing as rust-lang#73102. One more observable effect is how `$crate` is printed in the attribute input. Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs). This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed. So, I think we should do this regardless, but we need to run crater first. r? @Aaron1011

@Aaron1011

expand: Stop using nonterminals for passing tokens to attribute and derive macros Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment). Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later. The cost is making pretty-printing of token streams less pretty. Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway. Unfortunately, this is not exactly the same thing as rust-lang#73102. One more observable effect is how `$crate` is printed in the attribute input. Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs). This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed. So, I think we should do this regardless, but we need to run crater first. r? @Aaron1011

@ghost

…arth Rollup of 13 pull requests Successful merges: - rust-lang#72620 (Omit DW_AT_linkage_name when it is the same as DW_AT_name) - rust-lang#72967 (Don't move cursor in search box when using arrows to navigate results) - rust-lang#73102 (proc_macro: Stop flattening groups with dummy spans) - rust-lang#73297 (Support configurable deny-warnings for all in-tree crates.) - rust-lang#73507 (Cleanup MinGW LLVM linkage workaround) - rust-lang#73588 (Fix handling of reserved registers for ARM inline asm) - rust-lang#73597 (Record span of `const` kw in GenericParamKind) - rust-lang#73629 (Make AssocOp Copy) - rust-lang#73681 (Update Chalk to 0.14) - rust-lang#73707 (Fix links in `SliceIndex` documentation) - rust-lang#73719 (emitter: column width defaults to 140) - rust-lang#73729 (disable collectionbenches for android) - rust-lang#73748 (Add code block to code in documentation of `List::rebase_onto`) Failed merges: r? @ghost

petrochenkov mentioned this pull request Jun 7, 2020

error: expected identifier when building time-macros-impl v0.1.1 #72545

Closed

craterbot added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Jun 8, 2020

petrochenkov mentioned this pull request Jun 12, 2020

Re-land PR #72388: Recursively expand TokenKind::Interpolated in probably_equal_for_proc_macro #73084

Merged

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Jun 14, 2020

petrochenkov 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 Jun 14, 2020

petrochenkov mentioned this pull request Jun 14, 2020

expand: Stop using nonterminals for passing tokens to attribute and derive macros #73345

Merged

petrochenkov closed this Jun 14, 2020

petrochenkov reopened this Jun 25, 2020

petrochenkov changed the title ~~[experiment] proc_macro: Stop flattening groups with dummy spans~~ proc_macro: Stop flattening groups with dummy spans Jun 25, 2020

petrochenkov force-pushed the flatgroup branch from d754b6f to 6017f1d Compare June 25, 2020 21:14

rust-highfive assigned Aaron1011 Jun 25, 2020

Aaron1011 reviewed Jun 25, 2020

View reviewed changes

src/librustc_expand/proc_macro_server.rs Outdated Show resolved Hide resolved

Aaron1011 reviewed Jun 25, 2020

View reviewed changes

src/librustc_expand/proc_macro_server.rs Outdated Show resolved Hide resolved

proc_macro: Stop flattening groups with dummy spans

77b0ed7

petrochenkov force-pushed the flatgroup branch from 6017f1d to 77b0ed7 Compare June 25, 2020 21:47

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 25, 2020

This was referenced Jun 26, 2020

Rollup of 14 pull requests #73744

Closed

Rollup of 13 pull requests #73756

Merged

bors merged commit 2aee608 into rust-lang:master Jun 26, 2020

cuviper added this to the 1.46 milestone May 2, 2024

petrochenkov deleted the flatgroup branch February 22, 2025 18:32

Uh oh!

Conversation

petrochenkov commented Jun 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrochenkov commented Jun 7, 2020

Uh oh!

bors commented Jun 7, 2020

Uh oh!

bors commented Jun 7, 2020

Uh oh!

petrochenkov commented Jun 8, 2020

Uh oh!

craterbot commented Jun 8, 2020

Uh oh!

craterbot commented Jun 11, 2020

Uh oh!

craterbot commented Jun 14, 2020

Uh oh!

petrochenkov commented Jun 14, 2020

Uh oh!

petrochenkov commented Jun 14, 2020

Uh oh!

petrochenkov commented Jun 25, 2020

Uh oh!

Uh oh!

Uh oh!

Aaron1011 commented Jun 25, 2020

Uh oh!

petrochenkov commented Jun 25, 2020

Uh oh!

bors commented Jun 25, 2020

Uh oh!

nnethercote commented Jun 30, 2020

Uh oh!

rust-timer commented Jun 30, 2020

Uh oh!

mati865 commented Jun 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

petrochenkov commented Jun 7, 2020 •

edited

Loading

mati865 commented Jun 30, 2020 •

edited

Loading