Guard patterns: MIR lowering

[Human9000-bit]

This pr (hopefully) implements THIR -> MIR lowering of guard patterns:

• When PatKind::Guard is encountered, we lower the subpattern and push ExprId of a condition to extra_data.guard_patterns
• Then we pass that field to MatchTreeSubBranch
• In bind_ang_guard_matched_candidate we push actual guard to the sub_branch.guard_patterns vec
• Then we iterate over Vec<ExprId> of guards and (hopefully) merge them in MIR chain

r? @dianne
cc @Nadrieril, @max-niederman

[rustbot]

Some changes occurred in match lowering

cc @Nadrieril

[Zalathar]

@bors try @rust-timer queue

[Zalathar]

It's very awkward that this duplicates the else block below.

There should be a way to rephrase the if condition so that it only fires if there is at least one guard in the arm and/or guard patterns.

[Human9000-bit]

Agree. It's just a workaround untill I figure out smth better

[dianne]

It'd be nice for guards from match arms and guards from guard patterns to be handled the same here. Could the arm's guard not be part of the PatternExtraData along with the guards from patterns, so they can be handled uniformly?

Also, anything with a guard, whether it's from a match arm or a guard pattern, will need some equivalent of the match_scope there; returning to that scope after guard execution is what drops the guard's temporaries.

[Human9000-bit]

@Zalathar could you schedule perf run once again?

[Zalathar]

It’ll be faster to just let the old try job keep running. The new changes shouldn’t affect perf, so I think benchmarking the current job will be fine.

[dianne]

This will need some //@ run-pass tests to make sure the runtime semantics are correct, possibly also with //@ compile-flags: -Zvalidate-mir -Zlint-mir to help catch drop scheduling bugs. Getting scoping right and scheduling drops properly for guard patterns in all cases is a little subtle and will end up being the trickiest part of this; I know my first stab at that produced broken MIR ^^

You'll also want to look into how fake borrows work; patterns with guards on them will need fake borrows to make sure the guards can't modify any places being tested. For match and irrefutable let, this is needed for soundness (and in other cases, we should probably be consistent with that). At a glance, it doesn't look like this is setting has_guard for candidates, so certain things like fake borrows won't work. Likewise, this will need tests. I think some other things might use has_guard too, like or-pattern simplification.

As a meta note, I do have some opinions about how guard patterns should be implemented from my own attempt at lowering them to MIR last year. I'll try not just to compare this to what I'd do, since I'd effectively be reviewing my own code, but it might help to have more eyes on it just in case.

View changes since this review

[dianne]

It'd be nice for guards from match arms and guards from guard patterns to be handled the same here. Could the arm's guard not be part of the PatternExtraData along with the guards from patterns, so they can be handled uniformly?

Also, anything with a guard, whether it's from a match arm or a guard pattern, will need some equivalent of the match_scope there; returning to that scope after guard execution is what drops the guard's temporaries.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 94df5ce (94df5ce4eb9a637adba2da13b3f79cf010e61aeb, parent: 584d32e3ee7a2051c9ec1338d259ed8ef16380ca)

[rust-timer]

Finished benchmarking commit (94df5ce): comparison URL.

Overall result: ❌ regressions - 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.1%	[0.1%, 0.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (secondary 2.0%)

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.0%	[0.8%, 3.5%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

Results (primary 2.6%)

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)	-	-	0
All ❌✅ (primary)	2.6%	[2.6%, 2.6%]	1

Binary size

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

Bootstrap: 484.385s -> 484.355s (-0.01%)
Artifact size: 394.81 MiB -> 394.86 MiB (0.01%)