mir_build: Avoid some useless work when visiting "primary" bindings

[Zalathar]

While looking over visit_primary_bindings, I noticed that it does a bunch of extra work to build up a collection of “user-type projections”, even though 2/3 of its call sites don't even use them. Those callers can get the same result via thir::Pat::walk_always.

(And it turns out that doing so also avoids creating some redundant user-type entries in MIR for some binding constructs.)

I also noticed that even when the user-type projections are used, the process of building them ends up eagerly cloning some nested vectors at every recursion step, even in cases where they won't be used because the current subpattern has no bindings. To avoid this, the visit method now assembles a linked list on the stack containing the information that would be needed to create projections, and only creates the concrete projections as needed when a primary binding is encountered.

Some relevant prior PRs:

• Handle bindings in substructure of patterns with type ascriptions #55274
• 0bfe184 in NLL: User type annotations refactor, associated constant patterns and ref bindings. #55937

[rustbot]

r? @cjgillot

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

[Zalathar]

My earlier drafts didn't have any measurable perf effect, but let's try the real thing just in case:

@bors try @rust-timer queue

[bors]

⌛ Trying commit 1d8a6e7 with merge cde7127...

[bors]

☀️ Try build successful - checks-actions
Build commit: cde7127 (cde7127057d6247682880d66211dc381f9921f64)

[rust-timer]

Finished benchmarking commit (cde7127): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.4%	[-0.4%, -0.3%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

Cycles

Results (secondary -4.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Binary size

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

Bootstrap: 770.304s -> 770.384s (0.01%)
Artifact size: 359.67 MiB -> 359.64 MiB (-0.01%)

[Zalathar]

@bors rollup=maybe

[Zalathar]

(Rebased; no changes.)

[Zalathar]

I couldn't come up with a good name for this method, so I punted and went with something that signals “please read the docs for this weird method instead of just calling it”.

One of the problems with the old name (visit_primary_bindings) was that it sounded like a very normal visitor method, when in fact it was also doing some non-trivial extra stuff, and I think that's how we ended up in a situation where it was being called in other places that didn't need the extra stuff.