add a scope for if let guard temporaries and bindings

[dianne]

This fixes my concern with if let guard drop order, namely that the guard's bindings and temporaries were being dropped after their arm's pattern's bindings, instead of before (#141295 (comment)). The guard's bindings and temporaries now live in a new scope, which extends until (but not past) the end of the arm, guaranteeing they're dropped before the arm's pattern's bindings. So far, this is the only way I've thought of to achieve this without explicitly rescheduling guards' drops to move them after the arm's pattern's.

I'm not sure this should be merged as-is. It's a little hacky and it introduces a new scope for all match arms rather than just those with if let guards. However, since I'm looking for feedback on the approach, I figured this is a relatively simple way to present it. As I mention in a FIXME comment, something like this will be needed for guard patterns (#129967) too¹, so I think the final version should maybe only add these scopes as needed. That'll be better for perf too.

Tracking issue for if_let_guard: #51114

Tests are adapted from examples by @traviscross, @est31, and myself on #141295. cc, as I'd like your input on this.

I'm not entirely sure who to request for scoping changes, but let's start with r? @Nadrieril since this relates to guard patterns, we talked about it recently, and rustbot's going to ping you anyway. Feel free to reassign!

Footnotes

1. e.g., new scopes are needed to keep failed guards inside let chain patterns from dropping existing bindings/temporaries; something like this could give a way of doing that without needing to reschedule drops. Unfortunately it may not help keep failed guards in let statement patterns from dropping the let statement's initializer, so it isn't a complete solution. I'm still not sure how to do that without rescheduling drops, changing how let statements' scopes work, or restricting the functionality of guard patterns in let statements (including let-else). ↩

[rustbot]

Nadrieril is not on the review rotation at the moment.
They may take a while to respond.

[rustbot]

Some changes occurred in match lowering

cc @Nadrieril

[traviscross]

All sounds reasonable to me. Let's see what perf thinks.

@bors2 try
@rust-timer queue

[rust-bors]

⌛ Trying commit f501cdd with merge 43cc1e1…

To cancel the try build, run the command @bors2 try cancel.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 43cc1e1 (43cc1e156c2be744a9205d52fabf2fdff36c6ee2, parent: a413f77285c0ab551cf58db729e054f43150dd50)

[rust-timer]

Finished benchmarking commit (43cc1e1): 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)	-2.9%	[-2.9%, -2.9%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.9%	[-2.9%, -2.9%]	1

Max RSS (memory usage)

Results (primary 2.0%, secondary 2.8%)

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

	mean	range	count
Regressions ❌ (primary)	3.6%	[1.7%, 5.6%]	2
Regressions ❌ (secondary)	2.8%	[2.2%, 3.4%]	2
Improvements ✅ (primary)	-1.4%	[-1.4%, -1.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.0%	[-1.4%, 5.6%]	3

Cycles

Results (primary -3.1%, secondary -0.7%)

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)	2.1%	[1.0%, 3.2%]	2
Improvements ✅ (primary)	-3.1%	[-3.4%, -2.7%]	2
Improvements ✅ (secondary)	-2.5%	[-6.0%, -0.7%]	3
All ❌✅ (primary)	-3.1%	[-3.4%, -2.7%]	2

Binary size

Results (primary -1.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)	-	-	0
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.1%	[-1.1%, -1.1%]	1

Bootstrap: 461.296s -> 461.485s (0.04%)
Artifact size: 372.12 MiB -> 372.20 MiB (0.02%)

[dianne]

Looks like noise. If there was a change, it would be a regression in region_scope_tree, thir_body, or mir_built, which I'm not seeing. Likewise, this will result in a bit more memory usage when going from HIR to MIR, but it also seems to be lost in noise, at least as far as I can tell from the graphs.

[Nadrieril]

I approve of the idea! Can't speak of the impl, I'm not familiar with MIR scopes.

r? compiler

[est31]

Maybe add a comment that we don't expect any edition differences here?

[dianne]

Added: diff. I noticed a comment I wrote about the lifetime of a match scrutinee could be misleading, so I've amended that as well. It's still vague, but I think where exactly the scrutinee is dropped in that example is technically Edition-dependent (just not in a way that affects these tests).