diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index c458878da15b5..a09a65b3d20f1 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -191,6 +191,14 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir: visitor.cx.var_parent = visitor.cx.parent; resolve_pat(visitor, arm.pat); + // In order to ensure bindings and temporaries from `if let` guards are dropped before the arm's + // pattern's bindings, we introduce a new scope nested within the arm's. This contains the + // guard's bindings as well as any temporaries and scopes introduced within the arm's body. + // `ScopeData::IfThenRescope` is used as it restricts temporaries' lifetimes. + // FIXME(guard_patterns): This will likewise be needed for guard patterns. Nesting will be more + // complex in that case, and multiple may be needed for `let` guards inside `let` chains. + visitor.enter_scope(Scope { local_id: arm.hir_id.local_id, data: ScopeData::IfThenRescope }); + visitor.cx.var_parent = visitor.cx.parent; if let Some(guard) = arm.guard { resolve_expr(visitor, guard, !has_let_expr(guard)); } diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index 2c29b8628417f..fd776b9b26a64 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -448,47 +448,51 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let arm_scope = (arm.scope, arm_source_info); let match_scope = self.local_scope(); self.in_scope(arm_scope, arm.lint_level, |this| { - let old_dedup_scope = - mem::replace(&mut this.fixed_temps_scope, Some(arm.scope)); - - // `try_to_place` may fail if it is unable to resolve the given - // `PlaceBuilder` inside a closure. In this case, we don't want to include - // a scrutinee place. `scrutinee_place_builder` will fail to be resolved - // if the only match arm is a wildcard (`_`). - // Example: - // ``` - // let foo = (0, 1); - // let c = || { - // match foo { _ => () }; - // }; - // ``` - let scrutinee_place = scrutinee_place_builder.try_to_place(this); - let opt_scrutinee_place = - scrutinee_place.as_ref().map(|place| (Some(place), scrutinee_span)); - let scope = this.declare_bindings( - None, - arm.span, - &arm.pattern, - arm.guard, - opt_scrutinee_place, - ); + let guard_scope = + region::Scope { data: region::ScopeData::IfThenRescope, ..arm.scope }; + this.in_scope((guard_scope, arm_source_info), LintLevel::Inherited, |this| { + let old_dedup_scope = + mem::replace(&mut this.fixed_temps_scope, Some(guard_scope)); + + // `try_to_place` may fail if it is unable to resolve the given + // `PlaceBuilder` inside a closure. In this case, we don't want to include + // a scrutinee place. `scrutinee_place_builder` will fail to be resolved + // if the only match arm is a wildcard (`_`). + // Example: + // ``` + // let foo = (0, 1); + // let c = || { + // match foo { _ => () }; + // }; + // ``` + let scrutinee_place = scrutinee_place_builder.try_to_place(this); + let opt_scrutinee_place = + scrutinee_place.as_ref().map(|place| (Some(place), scrutinee_span)); + let scope = this.declare_bindings( + None, + arm.span, + &arm.pattern, + arm.guard, + opt_scrutinee_place, + ); - let arm_block = this.bind_pattern( - outer_source_info, - branch, - &built_match_tree.fake_borrow_temps, - scrutinee_span, - Some((arm, match_scope)), - EmitStorageLive::Yes, - ); + let arm_block = this.bind_pattern( + outer_source_info, + branch, + &built_match_tree.fake_borrow_temps, + scrutinee_span, + Some((arm, match_scope)), + EmitStorageLive::Yes, + ); - this.fixed_temps_scope = old_dedup_scope; + this.fixed_temps_scope = old_dedup_scope; - if let Some(source_scope) = scope { - this.source_scope = source_scope; - } + if let Some(source_scope) = scope { + this.source_scope = source_scope; + } - this.expr_into_dest(destination, arm_block, arm.body) + this.expr_into_dest(destination, arm_block, arm.body) + }) }) .into_block() }) @@ -569,7 +573,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // be bound for each candidate. for sub_branch in branch.sub_branches { if let Some(arm) = arm { - self.clear_top_scope(arm.scope); + self.clear_match_arm_scope(arm.scope); } let binding_end = self.bind_and_guard_matched_candidate( sub_branch, diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs index 405d47c7c79cb..87f0e3ef9b67d 100644 --- a/compiler/rustc_mir_build/src/builder/scope.rs +++ b/compiler/rustc_mir_build/src/builder/scope.rs @@ -1724,17 +1724,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { success_block } - /// Unschedules any drops in the top scope. + /// Unschedules any drops in the top two scopes. /// /// This is only needed for `match` arm scopes, because they have one /// entrance per pattern, but only one exit. - pub(crate) fn clear_top_scope(&mut self, region_scope: region::Scope) { - let top_scope = self.scopes.scopes.last_mut().unwrap(); + // FIXME(@dianne): This can hopefully be removed if we only lower guards once. + pub(crate) fn clear_match_arm_scope(&mut self, region_scope: region::Scope) { + let [.., arm_scope, guard_scope] = &mut *self.scopes.scopes else { + bug!("match arms should introduce two scopes"); + }; - assert_eq!(top_scope.region_scope, region_scope); + assert_eq!(arm_scope.region_scope, region_scope); + assert_eq!(guard_scope.region_scope.data, region::ScopeData::IfThenRescope); + assert_eq!(guard_scope.region_scope.local_id, region_scope.local_id); - top_scope.drops.clear(); - top_scope.invalidate_cache(); + arm_scope.drops.clear(); + arm_scope.invalidate_cache(); + guard_scope.drops.clear(); + guard_scope.invalidate_cache(); } } diff --git a/tests/ui/drop/if-let-guards.rs b/tests/ui/drop/if-let-guards.rs new file mode 100644 index 0000000000000..bd353112c09bc --- /dev/null +++ b/tests/ui/drop/if-let-guards.rs @@ -0,0 +1,88 @@ +//! Tests drop order for `if let` guard bindings and temporaries. This is for behavior specific to +//! `match` expressions, whereas `tests/ui/drop/drop-order-comparisons.rs` compares `let` chains in +//! guards to `let` chains in `if` expressions. Drop order for `let` chains in guards shouldn't +//! differ between Editions, so we test on both 2021 and 2024, expecting the same results. +//@ revisions: e2021 e2024 +//@ [e2021] edition: 2021 +//@ [e2024] edition: 2024 +//@ run-pass + +#![feature(if_let_guard)] +#![deny(rust_2024_compatibility)] + +use core::{cell::RefCell, ops::Drop}; + +fn main() { + // Test that `let` guard bindings and temps are dropped before the arm's pattern's bindings. + assert_drop_order(1..=6, |o| { + // We move out of the scrutinee, so the drop order of the array's elements are based on + // binding declaration order, and they're dropped in the arm's scope. + match [o.log(6), o.log(5)] { + // Partially move from the guard temporary to test drops both for temps and the binding. + [_x, _y] if let [_, _z, _] = [o.log(4), o.log(2), o.log(3)] + && true => { let _a = o.log(1); } + _ => unreachable!(), + } + }); + + // Sanity check: we don't move out of the match scrutinee when the guard fails. + assert_drop_order(1..=4, |o| { + // The scrutinee uses the drop order for arrays since its elements aren't moved. + match [o.log(3), o.log(4)] { + [_x, _y] if let _z = o.log(1) + && false => unreachable!(), + _ => { let _a = o.log(2); } + } + }); + + // Test `let` guards' temporaries are dropped immediately when a guard fails, even if the guard + // is lowered and run multiple times on the same arm due to or-patterns. + assert_drop_order(1..=8, |o| { + let mut _x = 1; + // The match's scrutinee isn't bound by-move, so it outlives the match. + match o.log(8) { + // Failing a guard breaks out of the arm's scope, dropping the `let` guard's scrutinee. + _ | _ | _ if let _ = o.log(_x) + && { _x += 1; false } => unreachable!(), + // The temporaries from a failed guard are dropped before testing the next guard. + _ if let _ = o.log(5) + && { o.push(4); false } => unreachable!(), + // If the guard succeeds, we stay in the arm's scope to execute its body. + _ if let _ = o.log(7) + && true => { o.log(6); } + _ => unreachable!(), + } + }); +} + +// # Test scaffolding... + +struct DropOrder(RefCell>); +struct LogDrop<'o>(&'o DropOrder, u64); + +impl DropOrder { + fn log(&self, n: u64) -> LogDrop<'_> { + LogDrop(self, n) + } + fn push(&self, n: u64) { + self.0.borrow_mut().push(n); + } +} + +impl<'o> Drop for LogDrop<'o> { + fn drop(&mut self) { + self.0.push(self.1); + } +} + +#[track_caller] +fn assert_drop_order( + ex: impl IntoIterator, + f: impl Fn(&DropOrder), +) { + let order = DropOrder(RefCell::new(Vec::new())); + f(&order); + let order = order.0.into_inner(); + let expected: Vec = ex.into_iter().collect(); + assert_eq!(order, expected); +} diff --git a/tests/ui/thir-print/thir-tree-loop-match.stdout b/tests/ui/thir-print/thir-tree-loop-match.stdout index 5c4c50cb15623..bed5f9dda26d0 100644 --- a/tests/ui/thir-print/thir-tree-loop-match.stdout +++ b/tests/ui/thir-print/thir-tree-loop-match.stdout @@ -129,7 +129,7 @@ body: body: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(16)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](16)), backwards_incompatible: None } span: $DIR/thir-tree-loop-match.rs:12:25: 15:18 (#0) kind: Scope { @@ -138,14 +138,14 @@ body: value: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(16)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](16)), backwards_incompatible: None } span: $DIR/thir-tree-loop-match.rs:12:25: 15:18 (#0) kind: NeverToAny { source: Expr { ty: ! - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(16)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](16)), backwards_incompatible: None } span: $DIR/thir-tree-loop-match.rs:12:25: 15:18 (#0) kind: Block { @@ -227,7 +227,7 @@ body: body: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None } span: $DIR/thir-tree-loop-match.rs:16:26: 16:38 (#0) kind: Scope { @@ -236,21 +236,21 @@ body: value: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None } span: $DIR/thir-tree-loop-match.rs:16:26: 16:38 (#0) kind: NeverToAny { source: Expr { ty: ! - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None } span: $DIR/thir-tree-loop-match.rs:16:26: 16:38 (#0) kind: Return { value: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None } span: $DIR/thir-tree-loop-match.rs:16:33: 16:38 (#0) kind: Scope { @@ -259,7 +259,7 @@ body: value: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(24)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](24)), backwards_incompatible: None } span: $DIR/thir-tree-loop-match.rs:16:33: 16:38 (#0) kind: VarRef { diff --git a/tests/ui/thir-print/thir-tree-match.stdout b/tests/ui/thir-print/thir-tree-match.stdout index 910582ae4d9e9..ccb9e2d0868db 100644 --- a/tests/ui/thir-print/thir-tree-match.stdout +++ b/tests/ui/thir-print/thir-tree-match.stdout @@ -123,7 +123,7 @@ body: body: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(14)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](14)), backwards_incompatible: None } span: $DIR/thir-tree-match.rs:17:36: 17:40 (#0) kind: Scope { @@ -132,7 +132,7 @@ body: value: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(14)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](14)), backwards_incompatible: None } span: $DIR/thir-tree-match.rs:17:36: 17:40 (#0) kind: Literal( lit: Spanned { node: Bool(true), span: $DIR/thir-tree-match.rs:17:36: 17:40 (#0) }, neg: false) @@ -175,7 +175,7 @@ body: body: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(20)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](20)), backwards_incompatible: None } span: $DIR/thir-tree-match.rs:18:27: 18:32 (#0) kind: Scope { @@ -184,7 +184,7 @@ body: value: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(20)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](20)), backwards_incompatible: None } span: $DIR/thir-tree-match.rs:18:27: 18:32 (#0) kind: Literal( lit: Spanned { node: Bool(false), span: $DIR/thir-tree-match.rs:18:27: 18:32 (#0) }, neg: false) @@ -219,7 +219,7 @@ body: body: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(26)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](26)), backwards_incompatible: None } span: $DIR/thir-tree-match.rs:19:24: 19:28 (#0) kind: Scope { @@ -228,7 +228,7 @@ body: value: Expr { ty: bool - temp_lifetime: TempLifetime { temp_lifetime: Some(Node(26)), backwards_incompatible: None } + temp_lifetime: TempLifetime { temp_lifetime: Some(IfThen[edition2024](26)), backwards_incompatible: None } span: $DIR/thir-tree-match.rs:19:24: 19:28 (#0) kind: Literal( lit: Spanned { node: Bool(true), span: $DIR/thir-tree-match.rs:19:24: 19:28 (#0) }, neg: false)