Use .map.collect to aggregate in .to_ty of tuples

[lapla-cogito]

Since #48994 might have been resolved, we can address the FIXME.

[rustbot]

r? @mati865

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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler, parser
• compiler, parser expanded to 22 candidates
• Random selection from 12 candidates

[Zalathar]

According to #53019 the current code was motivated by performance, so let's double-check; things are probably fine nowadays.

@bors try @rust-timer queue

[Zalathar]

(Also I was very confused by the issue number in this PR's description, but it was just a typo and I updated it to the actual issue number from the FIXME.)

EDIT: Actually the “fix” cited in the description is also bogus, because it applies to the typoed PR number and appears to have nothing to do with this FIXME. I'll remove that too.

[Zalathar]

• Actually, from looking at Collecting into a Result<Vec<_>> doesn't reserve the capacity in advance #48994 it might still be a problem. I guess we'll see from the benchmark results.

[Zalathar]

r? me

[lapla-cogito]

This PR doesn't address the issue mentioned in the updated description. The Span-related issue from my original description was fixed in 50924 and this FIXME was added in b68b396. This makes me question whether performance measurements are actually necessary.

[Zalathar]

The code change in this PR tries to address a // FIXME(#48994) comment:

• Collecting into a Result<Vec<_>> doesn't reserve the capacity in advance #48994

However, the original description of this PR cited this seemingly-unrelated issue instead:

• Span issues with FromStr in unstable proc_macro #48944

Notice the similarity between 48994 and 48944.

[lapla-cogito]

Ah my bad! Sorry for the mistake. Anyway I also think this won't hurt performance.

[rust-bors]

☀️ Try build successful (CI)
Build commit: e85694f (e85694f3b539b8d4c85cb59b7e92c5880d159bcc, parent: c58d9f9f82ead734d73d99a64e910718f5f464d3)

[rust-timer]

Finished benchmarking commit (e85694f): comparison URL.

Overall result: ✅ improvements - no action needed

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.

@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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	3
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary -4.9%)

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

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

Cycles

Results (secondary 4.1%)

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

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

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 475.659s -> 475.091s (-0.12%)
Artifact size: 398.00 MiB -> 397.96 MiB (-0.01%)

[Zalathar]

Oh I just noticed, is .collect.map in the commit message a mistake? I think it should be .map.collect instead.

[lapla-cogito]

Thanks, fixed. It seems I can't change the PR title myself - is there any way to do this?

[Zalathar]

Since there was no measurable perf difference, not even in stress tests, it should be safe to merge this nice little improvement.

Thanks for the fix!

@bors r+ rollup

[rust-bors]

📌 Commit b5cd820 has been approved by Zalathar

It is now in the queue for this repository.