remove TokenStream from the bridge

[cyrgani]

View all comments

This PR replaces the handle-based management of TokenStreams in proc_macro with a new type that is just a Rc<Vec<TokenTree>>. This type is very similar to the current TokenStream types in rustc and rust-analyzer, which are roughly Arc<Vec<TokenTree>>. This comes with some gains and drawbacks:

Improvements

• A significant part of proc_macro::bridge can be removed.
• Simple TokenStream operations are faster now since they do not need to use RPC anymore. This should be especially helpful for r-a.
• The token-stream-stress test is much faster now, which is hopefully representative for large proc macros.
• This is a significant improvement towards making proc_macro usable in regular crates (Tracking issue for proc_macro in non-proc-macro crates #130856).

Regressions

• TokenStream now has a worse Display implementation now that adds whitespace between most tokens. This is is theory allowed by the documentation of this implementation:

Note: the exact form of the output is subject to change, e.g. there might
be changes in the whitespace used between tokens. Therefore, you should
not do any kind of simple substring matching on the output string (as
produced by to_string) to implement a proc macro, because that matching
might stop working if such changes happen. Instead, you should work at the
TokenTree level, e.g. matching against TokenTree::Ident,
TokenTree::Punct, or TokenTree::Literal.

but might still cause too much breakage to be acceptable. This needs a crater run.

• Instruction counts for clap_derive and serde_derive are higher now. This is somewhat expected since LLVM can now understand what a TokenStream really is and perform some optimizations based on that.

related: #101419

[jieyouxu]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: a1f3eec (a1f3eecc1c3e174e689386582cf8307d3b170a81, parent: 370143facfb348ad3b29749c0393402d76b280c3)

[rust-timer]

Finished benchmarking commit (a1f3eec): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	1.6%	[0.1%, 6.9%]	64
Regressions ❌ (secondary)	0.9%	[0.1%, 2.3%]	9
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-22.3%	[-40.7%, -2.0%]	10
All ❌✅ (primary)	1.6%	[0.1%, 6.9%]	64

Max RSS (memory usage)

Results (primary 2.6%, secondary -4.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.6%	[2.6%, 2.6%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.2%	[-4.2%, -4.2%]	1
All ❌✅ (primary)	2.6%	[2.6%, 2.6%]	1

Cycles

Results (primary 0.7%, secondary -10.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	4.4%	[2.0%, 8.6%]	14
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-9.7%	[-10.8%, -8.6%]	5
Improvements ✅ (secondary)	-10.5%	[-20.0%, -3.8%]	9
All ❌✅ (primary)	0.7%	[-10.8%, 8.6%]	19

Binary size

Results (primary 1.0%, secondary -0.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.0%	[0.1%, 1.6%]	21
Regressions ❌ (secondary)	0.2%	[0.1%, 0.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.0%	[-1.0%, -1.0%]	3
All ❌✅ (primary)	1.0%	[0.1%, 1.6%]	21

Bootstrap: 474.117s -> 476.858s (0.58%)
Artifact size: 397.79 MiB -> 397.77 MiB (-0.01%)

[cyrgani]

@jieyouxu can you start another perf run?

[Kobzol]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 31b33e0 (31b33e0c3dcaf50135ca0eb8d0708ae383a35d85, parent: e823167aa6f7f03aea8e91208ce9bace8ad9ebf2)

[rust-timer]

Finished benchmarking commit (31b33e0): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	13.4%	[0.2%, 87.1%]	67
Regressions ❌ (secondary)	24735.6%	[0.0%, 97823.1%]	21
Improvements ✅ (primary)	-0.5%	[-0.5%, -0.5%]	1
Improvements ✅ (secondary)	-0.1%	[-0.2%, -0.0%]	2
All ❌✅ (primary)	13.2%	[-0.5%, 87.1%]	68

Max RSS (memory usage)

Results (primary 0.0%, secondary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.9%	[1.3%, 2.2%]	3
Regressions ❌ (secondary)	3.0%	[2.9%, 3.0%]	2
Improvements ✅ (primary)	-1.8%	[-2.8%, -0.6%]	3
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	0.0%	[-2.8%, 2.2%]	6

Cycles

Results (primary 21.5%, secondary 31513.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	21.5%	[2.0%, 87.2%]	41
Regressions ❌ (secondary)	37816.9%	[4.0%, 112549.0%]	15
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-6.3%	[-6.4%, -6.1%]	3
All ❌✅ (primary)	21.5%	[2.0%, 87.2%]	41

Binary size

Results (primary 1.5%, secondary 1.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.5%	[0.6%, 2.2%]	21
Regressions ❌ (secondary)	1.6%	[0.6%, 2.6%]	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.5%	[0.6%, 2.2%]	21

Bootstrap: 474.712s -> 475.37s (0.14%)
Artifact size: 397.79 MiB -> 397.76 MiB (-0.01%)

[Kobzol]

@bors try @rust-timer queue

[jieyouxu]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: 595142e (595142ec72045c7f2fab2923de8e59995a52e6e7, parent: d9563937fa3b030c5845811113505070109414d2)

[jieyouxu]

@craterbot check

[craterbot]

👌 Experiment pr-151830 created and queued.
🤖 Automatically detected try build 595142e
🔍 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]

🚧 Experiment pr-151830 is now running

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

[craterbot]

🎉 Experiment pr-151830 is completed!
📊 95 regressed and 11 fixed (837236 total)
📊 2453 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

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

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-151830/retry-regressed-list.txt ↩

[cyrgani]

one minimization:

use proc_macro::TokenStream;

#[proc_macro]
pub fn identity(input: TokenStream) -> TokenStream {
    input
}

and

macro_rules! operator_impl {
    ($target_expr:expr) => {
        x::identity! {
            $target_expr as fn()
        };
    };
}
fn main() {
    operator_impl!(|| ());
}

caused by the fact that with this change, identity is actually doing something like input.into_iter().collect(), which already does not compile without the change.

[rust-bors]

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