Skip to content

Commit 21d2e81

Browse files
diannedianne
committed
add a scope for if let guard temporaries and bindings
This ensures `if let` guard temporaries and bindings are dropped before the match arm's pattern's bindings.
1 parent a1adcce commit 21d2e81

File tree

6 files changed

+79
-61
lines changed

6 files changed

+79
-61
lines changed

‎compiler/rustc_hir_analysis/src/check/region.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,14 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir:
191191
visitor.cx.var_parent = visitor.cx.parent;
192192

193193
resolve_pat(visitor, arm.pat);
194+
// In order to ensure bindings and temporaries from `if let` guards are dropped before the arm's
195+
// pattern's bindings, we introduce a new scope nested within the arm's. This contains the
196+
// guard's bindings as well as any temporaries and scopes introduced within the arm's body.
197+
// `ScopeData::IfThenRescope` is used as it restricts temporaries' lifetimes.
198+
// FIXME(guard_patterns): This will likewise be needed for guard patterns. Nesting will be more
199+
// complex in that case, and multiple may be needed for `let` guards inside `let` chains.
200+
visitor.enter_scope(Scope { local_id: arm.hir_id.local_id, data: ScopeData::IfThenRescope });
201+
visitor.cx.var_parent = visitor.cx.parent;
194202
if let Some(guard) = arm.guard {
195203
resolve_expr(visitor, guard, !has_let_expr(guard));
196204
}

‎compiler/rustc_mir_build/src/builder/matches/mod.rs‎

Lines changed: 42 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -448,47 +448,51 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
448448
let arm_scope = (arm.scope, arm_source_info);
449449
let match_scope = self.local_scope();
450450
self.in_scope(arm_scope, arm.lint_level, |this| {
451-
let old_dedup_scope =
452-
mem::replace(&mut this.fixed_temps_scope, Some(arm.scope));
453-
454-
// `try_to_place` may fail if it is unable to resolve the given
455-
// `PlaceBuilder` inside a closure. In this case, we don't want to include
456-
// a scrutinee place. `scrutinee_place_builder` will fail to be resolved
457-
// if the only match arm is a wildcard (`_`).
458-
// Example:
459-
// ```
460-
// let foo = (0, 1);
461-
// let c = || {
462-
// match foo { _ => () };
463-
// };
464-
// ```
465-
let scrutinee_place = scrutinee_place_builder.try_to_place(this);
466-
let opt_scrutinee_place =
467-
scrutinee_place.as_ref().map(|place| (Some(place), scrutinee_span));
468-
let scope = this.declare_bindings(
469-
None,
470-
arm.span,
471-
&arm.pattern,
472-
arm.guard,
473-
opt_scrutinee_place,
474-
);
451+
let guard_scope =
452+
region::Scope { data: region::ScopeData::IfThenRescope, ..arm.scope };
453+
this.in_scope((guard_scope, arm_source_info), LintLevel::Inherited, |this| {
454+
let old_dedup_scope =
455+
mem::replace(&mut this.fixed_temps_scope, Some(guard_scope));
456+
457+
// `try_to_place` may fail if it is unable to resolve the given
458+
// `PlaceBuilder` inside a closure. In this case, we don't want to include
459+
// a scrutinee place. `scrutinee_place_builder` will fail to be resolved
460+
// if the only match arm is a wildcard (`_`).
461+
// Example:
462+
// ```
463+
// let foo = (0, 1);
464+
// let c = || {
465+
// match foo { _ => () };
466+
// };
467+
// ```
468+
let scrutinee_place = scrutinee_place_builder.try_to_place(this);
469+
let opt_scrutinee_place =
470+
scrutinee_place.as_ref().map(|place| (Some(place), scrutinee_span));
471+
let scope = this.declare_bindings(
472+
None,
473+
arm.span,
474+
&arm.pattern,
475+
arm.guard,
476+
opt_scrutinee_place,
477+
);
475478

476-
let arm_block = this.bind_pattern(
477-
outer_source_info,
478-
branch,
479-
&built_match_tree.fake_borrow_temps,
480-
scrutinee_span,
481-
Some((arm, match_scope)),
482-
EmitStorageLive::Yes,
483-
);
479+
let arm_block = this.bind_pattern(
480+
outer_source_info,
481+
branch,
482+
&built_match_tree.fake_borrow_temps,
483+
scrutinee_span,
484+
Some((arm, match_scope)),
485+
EmitStorageLive::Yes,
486+
);
484487

485-
this.fixed_temps_scope = old_dedup_scope;
488+
this.fixed_temps_scope = old_dedup_scope;
486489

487-
if let Some(source_scope) = scope {
488-
this.source_scope = source_scope;
489-
}
490+
if let Some(source_scope) = scope {
491+
this.source_scope = source_scope;
492+
}
490493

491-
this.expr_into_dest(destination, arm_block, arm.body)
494+
this.expr_into_dest(destination, arm_block, arm.body)
495+
})
492496
})
493497
.into_block()
494498
})
@@ -569,7 +573,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
569573
// be bound for each candidate.
570574
for sub_branch in branch.sub_branches {
571575
if let Some(arm) = arm {
572-
self.clear_top_scope(arm.scope);
576+
self.clear_match_arm_scope(arm.scope);
573577
}
574578
let binding_end = self.bind_and_guard_matched_candidate(
575579
sub_branch,

‎compiler/rustc_mir_build/src/builder/scope.rs‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,17 +1724,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
17241724
success_block
17251725
}
17261726

1727-
/// Unschedules any drops in the top scope.
1727+
/// Unschedules any drops in the top two scopes.
17281728
///
17291729
/// This is only needed for `match` arm scopes, because they have one
17301730
/// entrance per pattern, but only one exit.
1731-
pub(crate) fn clear_top_scope(&mut self, region_scope: region::Scope) {
1732-
let top_scope = self.scopes.scopes.last_mut().unwrap();
1731+
// FIXME(@dianne): This can hopefully be removed if we only lower guards once.
1732+
pub(crate) fn clear_match_arm_scope(&mut self, region_scope: region::Scope) {
1733+
let [.., arm_scope, guard_scope] = &mut *self.scopes.scopes else {
1734+
bug!("match arms should introduce two scopes");
1735+
};
17331736

1734-
assert_eq!(top_scope.region_scope, region_scope);
1737+
assert_eq!(arm_scope.region_scope, region_scope);
1738+
assert_eq!(guard_scope.region_scope.data, region::ScopeData::IfThenRescope);
1739+
assert_eq!(guard_scope.region_scope.local_id, region_scope.local_id);
17351740

1736-
top_scope.drops.clear();
1737-
top_scope.invalidate_cache();
1741+
arm_scope.drops.clear();
1742+
arm_scope.invalidate_cache();
1743+
guard_scope.drops.clear();
1744+
guard_scope.invalidate_cache();
17381745
}
17391746
}
17401747

‎tests/ui/drop/if-let-guards.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,12 @@ use core::{cell::RefCell, ops::Drop};
1414

1515
fn main() {
1616
// Test that `let` guard bindings and temps are dropped before the arm's pattern's bindings.
17-
// TODO: this is currently the old behavior (`let` bindings dropped after arm bindings).
1817
assert_drop_order(1..=6, |o| {
1918
// We move out of the scrutinee, so the drop order of the array's elements are based on
2019
// binding declaration order, and they're dropped in the arm's scope.
21-
match [o.log(3), o.log(2)] {
20+
match [o.log(6), o.log(5)] {
2221
// Partially move from the guard temporary to test drops both for temps and the binding.
23-
[_x, _y] if let [_, _z, _] = [o.log(6), o.log(4), o.log(5)]
22+
[_x, _y] if let [_, _z, _] = [o.log(4), o.log(2), o.log(3)]
2423
&& true => { let _a = o.log(1); }
2524
_ => unreachable!(),
2625
}

‎tests/ui/thir-print/thir-tree-loop-match.stdout‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ body:
106106
body:
107107
Expr {
108108
ty: bool
109-
temp_lifetime: TempLifetime { temp_lifetime: Some(Node(16)), backwards_incompatible: None }
109+
temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](16)), backwards_incompatible: None }
110110
span: $DIR/thir-tree-loop-match.rs:12:25: 15:18 (#0)
111111
kind:
112112
Scope {
@@ -115,14 +115,14 @@ body:
115115
value:
116116
Expr {
117117
ty: bool
118-
temp_lifetime: TempLifetime { temp_lifetime: Some(Node(16)), backwards_incompatible: None }
118+
temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](16)), backwards_incompatible: None }
119119
span: $DIR/thir-tree-loop-match.rs:12:25: 15:18 (#0)
120120
kind:
121121
NeverToAny {
122122
source:
123123
Expr {
124124
ty: !
125-
temp_lifetime: TempLifetime { temp_lifetime: Some(Node(16)), backwards_incompatible: None }
125+
temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](16)), backwards_incompatible: None }
126126
span: $DIR/thir-tree-loop-match.rs:12:25: 15:18 (#0)
127127
kind:
128128
Block {
@@ -204,7 +204,7 @@ body:
204204
body:
205205
Expr {
206206
ty: bool
207-
temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None }
207+
temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None }
208208
span: $DIR/thir-tree-loop-match.rs:16:26: 16:38 (#0)
209209
kind:
210210
Scope {
@@ -213,21 +213,21 @@ body:
213213
value:
214214
Expr {
215215
ty: bool
216-
temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None }
216+
temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None }
217217
span: $DIR/thir-tree-loop-match.rs:16:26: 16:38 (#0)
218218
kind:
219219
NeverToAny {
220220
source:
221221
Expr {
222222
ty: !
223-
temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None }
223+
temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None }
224224
span: $DIR/thir-tree-loop-match.rs:16:26: 16:38 (#0)
225225
kind:
226226
Return {
227227
value:
228228
Expr {
229229
ty: bool
230-
temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None }
230+
temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None }
231231
span: $DIR/thir-tree-loop-match.rs:16:33: 16:38 (#0)
232232
kind:
233233
Scope {
@@ -236,7 +236,7 @@ body:
236236
value:
237237
Expr {
238238
ty: bool
239-
temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None }
239+
temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None }
240240
span: $DIR/thir-tree-loop-match.rs:16:33: 16:38 (#0)
241241
kind:
242242
VarRef {

0 commit comments

Comments
 (0)