Skip to content

Commit d409163

Browse files
matthewjaspermatthewjasper
committed
Give temporaries in if let guards correct scopes
- Make temporaries in if-let guards be the same variable in MIR when the guard is duplicated due to or-patterns. - Change the "destruction scope" for match arms to be the arm scope rather than the arm body scope. - Add tests.
1 parent 68d684c commit d409163

File tree

10 files changed

+178
-10
lines changed

10 files changed

+178
-10
lines changed

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -179,10 +179,10 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h
179179
fn resolve_arm<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, arm: &'tcx hir::Arm<'tcx>) {
180180
let prev_cx = visitor.cx;
181181

182-
visitor.enter_scope(Scope { id: arm.hir_id.local_id, data: ScopeData::Node });
183-
visitor.cx.var_parent = visitor.cx.parent;
182+
visitor.terminating_scopes.insert(arm.hir_id.local_id);
184183

185-
visitor.terminating_scopes.insert(arm.body.hir_id.local_id);
184+
visitor.enter_node_scope_with_dtor(arm.hir_id.local_id);
185+
visitor.cx.var_parent = visitor.cx.parent;
186186

187187
if let Some(hir::Guard::If(expr)) = arm.guard {
188188
visitor.terminating_scopes.insert(expr.hir_id.local_id);

‎compiler/rustc_mir_build/src/build/expr/as_temp.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
4343
}
4444

4545
let expr_ty = expr.ty;
46-
let temp = {
46+
let deduplicate_temps =
47+
this.fixed_temps_scope.is_some() && this.fixed_temps_scope == temp_lifetime;
48+
let temp = if deduplicate_temps && let Some(temp_index) = this.fixed_temps.get(&expr_id) {
49+
*temp_index
50+
} else {
4751
let mut local_decl = LocalDecl::new(expr_ty, expr_span);
4852
if mutability.is_not() {
4953
local_decl = local_decl.immutable();
@@ -72,6 +76,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
7276
**local_decl.local_info.as_mut().assert_crate_local() = local_info;
7377
this.local_decls.push(local_decl)
7478
};
79+
if deduplicate_temps {
80+
this.fixed_temps.insert(expr_id, temp);
81+
}
7582
let temp_place = Place::from(temp);
7683

7784
match expr.kind {

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

+5
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
396396
let arm_scope = (arm.scope, arm_source_info);
397397
let match_scope = self.local_scope();
398398
self.in_scope(arm_scope, arm.lint_level, |this| {
399+
let old_dedup_scope =
400+
mem::replace(&mut this.fixed_temps_scope, Some(arm.scope));
401+
399402
// `try_to_place` may fail if it is unable to resolve the given
400403
// `PlaceBuilder` inside a closure. In this case, we don't want to include
401404
// a scrutinee place. `scrutinee_place_builder` will fail to be resolved
@@ -427,6 +430,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
427430
false,
428431
);
429432

433+
this.fixed_temps_scope = old_dedup_scope;
434+
430435
if let Some(source_scope) = scope {
431436
this.source_scope = source_scope;
432437
}

‎compiler/rustc_mir_build/src/build/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,12 @@ struct Builder<'a, 'tcx> {
210210
/// finish building it.
211211
guard_context: Vec<GuardFrame>,
212212

213+
/// Temporaries with fixed indexes. Used so that if-let guards on arms
214+
/// with an or-pattern are only created once.
215+
fixed_temps: FxHashMap<ExprId, Local>,
216+
/// Scope of temporaries that should be deduplicated using [fixed_temps]
217+
fixed_temps_scope: Option<region::Scope>,
218+
213219
/// Maps `HirId`s of variable bindings to the `Local`s created for them.
214220
/// (A match binding can have two locals; the 2nd is for the arm's guard.)
215221
var_indices: FxHashMap<LocalVarId, LocalsForNode>,
@@ -752,6 +758,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
752758
source_scopes: IndexVec::new(),
753759
source_scope: OUTERMOST_SOURCE_SCOPE,
754760
guard_context: vec![],
761+
fixed_temps: Default::default(),
762+
fixed_temps_scope: None,
755763
in_scope_unsafe: safety,
756764
local_decls: IndexVec::from_elem_n(LocalDecl::new(return_ty, return_span), 1),
757765
canonical_user_type_annotations: IndexVec::new(),

‎tests/ui/drop/dynamic-drop.rs

+14
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,17 @@ fn if_let_guard(a: &Allocator, c: bool, d: i32) {
343343
}
344344
}
345345

346+
fn if_let_guard_2(a: &Allocator, num: i32) {
347+
let d = a.alloc();
348+
match num {
349+
#[allow(irrefutable_let_patterns)]
350+
1 | 2 if let Ptr(ref _idx, _) = a.alloc() => {
351+
a.alloc();
352+
}
353+
_ => {}
354+
}
355+
}
356+
346357
fn panic_after_return(a: &Allocator) -> Ptr<'_> {
347358
// Panic in the drop of `p` or `q` can leak
348359
let exceptions = vec![8, 9];
@@ -514,6 +525,9 @@ fn main() {
514525
run_test(|a| if_let_guard(a, false, 0));
515526
run_test(|a| if_let_guard(a, false, 1));
516527
run_test(|a| if_let_guard(a, false, 2));
528+
run_test(|a| if_let_guard_2(a, 0));
529+
run_test(|a| if_let_guard_2(a, 1));
530+
run_test(|a| if_let_guard_2(a, 2));
517531

518532
run_test(|a| {
519533
panic_after_return(a);

‎tests/ui/rfcs/rfc-2294-if-let-guard/drop-order.rs

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// check drop order of temporaries create in match guards.
2+
// For normal guards all temporaries are dropped before the body of the arm.
3+
// For let guards temporaries live until the end of the arm.
4+
5+
// run-pass
6+
7+
#![feature(if_let_guard)]
8+
#![allow(irrefutable_let_patterns)]
9+
10+
use std::sync::Mutex;
11+
12+
static A: Mutex<Vec<i32>> = Mutex::new(Vec::new());
13+
14+
struct D(i32);
15+
16+
fn make_d(x: i32) -> D {
17+
A.lock().unwrap().push(x);
18+
D(x)
19+
}
20+
21+
impl Drop for D {
22+
fn drop(&mut self) {
23+
A.lock().unwrap().push(!self.0);
24+
}
25+
}
26+
27+
fn if_guard(num: i32) {
28+
let _d = make_d(1);
29+
match num {
30+
1 | 2 if make_d(2).0 == 2 => {
31+
make_d(3);
32+
}
33+
_ => {}
34+
}
35+
}
36+
37+
fn if_let_guard(num: i32) {
38+
let _d = make_d(1);
39+
match num {
40+
1 | 2 if let D(ref _x) = make_d(2) => {
41+
make_d(3);
42+
}
43+
_ => {}
44+
}
45+
}
46+
47+
fn main() {
48+
if_guard(1);
49+
if_guard(2);
50+
if_let_guard(1);
51+
if_let_guard(2);
52+
let expected = [
53+
1, 2, !2, 3, !3, !1,
54+
1, 2, !2, 3, !3, !1,
55+
1, 2, 3, !3, !2, !1,
56+
1, 2, 3, !3, !2, !1,
57+
];
58+
assert_eq!(*A.lock().unwrap(), expected);
59+
}

‎tests/ui/rfcs/rfc-2294-if-let-guard/loop-mutability.rs

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// check-pass
2+
3+
#![feature(if_let_guard)]
4+
5+
fn split_last(_: &()) -> Option<(&i32, &i32)> {
6+
None
7+
}
8+
9+
fn assign_twice() {
10+
loop {
11+
match () {
12+
#[allow(irrefutable_let_patterns)]
13+
() if let _ = split_last(&()) => {}
14+
_ => {}
15+
}
16+
}
17+
}
18+
19+
fn main() {}

‎tests/ui/rfcs/rfc-2294-if-let-guard/scoping-consistency-async.rs

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Check that temporaries in if-let guards are correctly scoped.
2+
// Regression test for #116079.
3+
4+
// build-pass
5+
// edition:2018
6+
// -Zvalidate-mir
7+
8+
#![feature(if_let_guard)]
9+
10+
static mut A: [i32; 5] = [1, 2, 3, 4, 5];
11+
12+
async fn fun() {
13+
unsafe {
14+
match A {
15+
_ => (),
16+
i if let Some(1) = async { Some(1) }.await => (),
17+
_ => (),
18+
}
19+
}
20+
}
21+
22+
async fn funner() {
23+
unsafe {
24+
match A {
25+
_ => (),
26+
_ | _ if let Some(1) = async { Some(1) }.await => (),
27+
_ => (),
28+
}
29+
}
30+
}
31+
32+
fn main() {}

‎tests/ui/rfcs/rfc-2294-if-let-guard/scoping-consistency.rs

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Check that temporaries in if-let guards are correctly scoped.
2+
3+
// build-pass
4+
// -Zvalidate-mir
5+
6+
#![feature(if_let_guard)]
7+
8+
fn fun() {
9+
match 0 {
10+
_ => (),
11+
_ if let Some(s) = std::convert::identity(&Some(String::new())) => {}
12+
_ => (),
13+
}
14+
}
15+
16+
fn funner() {
17+
match 0 {
18+
_ => (),
19+
_ | _ if let Some(s) = std::convert::identity(&Some(String::new())) => {}
20+
_ => (),
21+
}
22+
}
23+
24+
fn main() {}

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ body:
124124
body:
125125
Expr {
126126
ty: bool
127-
temp_lifetime: Some(Node(13))
127+
temp_lifetime: Some(Node(12))
128128
span: $DIR/thir-tree-match.rs:17:36: 17:40 (#0)
129129
kind:
130130
Scope {
@@ -133,7 +133,7 @@ body:
133133
value:
134134
Expr {
135135
ty: bool
136-
temp_lifetime: Some(Node(13))
136+
temp_lifetime: Some(Node(12))
137137
span: $DIR/thir-tree-match.rs:17:36: 17:40 (#0)
138138
kind:
139139
Literal( lit: Spanned { node: Bool(true), span: $DIR/thir-tree-match.rs:17:36: 17:40 (#0) }, neg: false)
@@ -176,7 +176,7 @@ body:
176176
body:
177177
Expr {
178178
ty: bool
179-
temp_lifetime: Some(Node(19))
179+
temp_lifetime: Some(Node(18))
180180
span: $DIR/thir-tree-match.rs:18:27: 18:32 (#0)
181181
kind:
182182
Scope {
@@ -185,7 +185,7 @@ body:
185185
value:
186186
Expr {
187187
ty: bool
188-
temp_lifetime: Some(Node(19))
188+
temp_lifetime: Some(Node(18))
189189
span: $DIR/thir-tree-match.rs:18:27: 18:32 (#0)
190190
kind:
191191
Literal( lit: Spanned { node: Bool(false), span: $DIR/thir-tree-match.rs:18:27: 18:32 (#0) }, neg: false)
@@ -220,7 +220,7 @@ body:
220220
body:
221221
Expr {
222222
ty: bool
223-
temp_lifetime: Some(Node(24))
223+
temp_lifetime: Some(Node(23))
224224
span: $DIR/thir-tree-match.rs:19:24: 19:28 (#0)
225225
kind:
226226
Scope {
@@ -229,7 +229,7 @@ body:
229229
value:
230230
Expr {
231231
ty: bool
232-
temp_lifetime: Some(Node(24))
232+
temp_lifetime: Some(Node(23))
233233
span: $DIR/thir-tree-match.rs:19:24: 19:28 (#0)
234234
kind:
235235
Literal( lit: Spanned { node: Bool(true), span: $DIR/thir-tree-match.rs:19:24: 19:28 (#0) }, neg: false)

0 commit comments

Comments
 (0)