mirror of
https://github.com/rust-lang/rust.git
synced 2025-04-13 12:36:47 +00:00
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.
This commit is contained in:
Matthew Jasper
parent
68d684cbff
commit
d437a111f5
@ -179,10 +179,10 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h
|
||||
fn resolve_arm<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, arm: &'tcx hir::Arm<'tcx>) {
|
||||
let prev_cx = visitor.cx;
|
||||
|
||||
visitor.enter_scope(Scope { id: arm.hir_id.local_id, data: ScopeData::Node });
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
visitor.terminating_scopes.insert(arm.hir_id.local_id);
|
||||
|
||||
visitor.terminating_scopes.insert(arm.body.hir_id.local_id);
|
||||
visitor.enter_node_scope_with_dtor(arm.hir_id.local_id);
|
||||
visitor.cx.var_parent = visitor.cx.parent;
|
||||
|
||||
if let Some(hir::Guard::If(expr)) = arm.guard {
|
||||
visitor.terminating_scopes.insert(expr.hir_id.local_id);
|
||||
|
||||
@ -43,7 +43,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
||||
}
|
||||
|
||||
let expr_ty = expr.ty;
|
||||
let temp = {
|
||||
let deduplicate_temps =
|
||||
this.fixed_temps_scope.is_some() && this.fixed_temps_scope == temp_lifetime;
|
||||
let temp = if deduplicate_temps && let Some(temp_index) = this.fixed_temps.get(&expr_id) {
|
||||
*temp_index
|
||||
} else {
|
||||
let mut local_decl = LocalDecl::new(expr_ty, expr_span);
|
||||
if mutability.is_not() {
|
||||
local_decl = local_decl.immutable();
|
||||
@ -72,6 +76,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
||||
**local_decl.local_info.as_mut().assert_crate_local() = local_info;
|
||||
this.local_decls.push(local_decl)
|
||||
};
|
||||
if deduplicate_temps {
|
||||
this.fixed_temps.insert(expr_id, temp);
|
||||
}
|
||||
let temp_place = Place::from(temp);
|
||||
|
||||
match expr.kind {
|
||||
|
||||
@ -396,6 +396,9 @@ 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
|
||||
@ -427,6 +430,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
||||
false,
|
||||
);
|
||||
|
||||
this.fixed_temps_scope = old_dedup_scope;
|
||||
|
||||
if let Some(source_scope) = scope {
|
||||
this.source_scope = source_scope;
|
||||
}
|
||||
|
||||
@ -210,6 +210,12 @@ struct Builder<'a, 'tcx> {
|
||||
/// finish building it.
|
||||
guard_context: Vec<GuardFrame>,
|
||||
|
||||
/// Temporaries with fixed indexes. Used so that if-let guards on arms
|
||||
/// with an or-pattern are only created once.
|
||||
fixed_temps: FxHashMap<ExprId, Local>,
|
||||
/// Scope of temporaries that should be deduplicated using [Self::fixed_temps].
|
||||
fixed_temps_scope: Option<region::Scope>,
|
||||
|
||||
/// Maps `HirId`s of variable bindings to the `Local`s created for them.
|
||||
/// (A match binding can have two locals; the 2nd is for the arm's guard.)
|
||||
var_indices: FxHashMap<LocalVarId, LocalsForNode>,
|
||||
@ -752,6 +758,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
|
||||
source_scopes: IndexVec::new(),
|
||||
source_scope: OUTERMOST_SOURCE_SCOPE,
|
||||
guard_context: vec![],
|
||||
fixed_temps: Default::default(),
|
||||
fixed_temps_scope: None,
|
||||
in_scope_unsafe: safety,
|
||||
local_decls: IndexVec::from_elem_n(LocalDecl::new(return_ty, return_span), 1),
|
||||
canonical_user_type_annotations: IndexVec::new(),
|
||||
|
||||
@ -343,6 +343,17 @@ fn if_let_guard(a: &Allocator, c: bool, d: i32) {
|
||||
}
|
||||
}
|
||||
|
||||
fn if_let_guard_2(a: &Allocator, num: i32) {
|
||||
let d = a.alloc();
|
||||
match num {
|
||||
#[allow(irrefutable_let_patterns)]
|
||||
1 | 2 if let Ptr(ref _idx, _) = a.alloc() => {
|
||||
a.alloc();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn panic_after_return(a: &Allocator) -> Ptr<'_> {
|
||||
// Panic in the drop of `p` or `q` can leak
|
||||
let exceptions = vec![8, 9];
|
||||
@ -514,6 +525,9 @@ fn main() {
|
||||
run_test(|a| if_let_guard(a, false, 0));
|
||||
run_test(|a| if_let_guard(a, false, 1));
|
||||
run_test(|a| if_let_guard(a, false, 2));
|
||||
run_test(|a| if_let_guard_2(a, 0));
|
||||
run_test(|a| if_let_guard_2(a, 1));
|
||||
run_test(|a| if_let_guard_2(a, 2));
|
||||
|
||||
run_test(|a| {
|
||||
panic_after_return(a);
|
||||
|
||||
59
tests/ui/rfcs/rfc-2294-if-let-guard/drop-order.rs
Normal file
59
tests/ui/rfcs/rfc-2294-if-let-guard/drop-order.rs
Normal file
@ -0,0 +1,59 @@
|
||||
// check drop order of temporaries create in match guards.
|
||||
// For normal guards all temporaries are dropped before the body of the arm.
|
||||
// For let guards temporaries live until the end of the arm.
|
||||
|
||||
// run-pass
|
||||
|
||||
#![feature(if_let_guard)]
|
||||
#![allow(irrefutable_let_patterns)]
|
||||
|
||||
use std::sync::Mutex;
|
||||
|
||||
static A: Mutex<Vec<i32>> = Mutex::new(Vec::new());
|
||||
|
||||
struct D(i32);
|
||||
|
||||
fn make_d(x: i32) -> D {
|
||||
A.lock().unwrap().push(x);
|
||||
D(x)
|
||||
}
|
||||
|
||||
impl Drop for D {
|
||||
fn drop(&mut self) {
|
||||
A.lock().unwrap().push(!self.0);
|
||||
}
|
||||
}
|
||||
|
||||
fn if_guard(num: i32) {
|
||||
let _d = make_d(1);
|
||||
match num {
|
||||
1 | 2 if make_d(2).0 == 2 => {
|
||||
make_d(3);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn if_let_guard(num: i32) {
|
||||
let _d = make_d(1);
|
||||
match num {
|
||||
1 | 2 if let D(ref _x) = make_d(2) => {
|
||||
make_d(3);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
if_guard(1);
|
||||
if_guard(2);
|
||||
if_let_guard(1);
|
||||
if_let_guard(2);
|
||||
let expected = [
|
||||
1, 2, !2, 3, !3, !1,
|
||||
1, 2, !2, 3, !3, !1,
|
||||
1, 2, 3, !3, !2, !1,
|
||||
1, 2, 3, !3, !2, !1,
|
||||
];
|
||||
assert_eq!(*A.lock().unwrap(), expected);
|
||||
}
|
||||
19
tests/ui/rfcs/rfc-2294-if-let-guard/loop-mutability.rs
Normal file
19
tests/ui/rfcs/rfc-2294-if-let-guard/loop-mutability.rs
Normal file
@ -0,0 +1,19 @@
|
||||
// check-pass
|
||||
|
||||
#![feature(if_let_guard)]
|
||||
|
||||
fn split_last(_: &()) -> Option<(&i32, &i32)> {
|
||||
None
|
||||
}
|
||||
|
||||
fn assign_twice() {
|
||||
loop {
|
||||
match () {
|
||||
#[allow(irrefutable_let_patterns)]
|
||||
() if let _ = split_last(&()) => {}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
@ -0,0 +1,32 @@
|
||||
// Check that temporaries in if-let guards are correctly scoped.
|
||||
// Regression test for #116079.
|
||||
|
||||
// build-pass
|
||||
// edition:2018
|
||||
// -Zvalidate-mir
|
||||
|
||||
#![feature(if_let_guard)]
|
||||
|
||||
static mut A: [i32; 5] = [1, 2, 3, 4, 5];
|
||||
|
||||
async fn fun() {
|
||||
unsafe {
|
||||
match A {
|
||||
_ => (),
|
||||
i if let Some(1) = async { Some(1) }.await => (),
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn funner() {
|
||||
unsafe {
|
||||
match A {
|
||||
_ => (),
|
||||
_ | _ if let Some(1) = async { Some(1) }.await => (),
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
24
tests/ui/rfcs/rfc-2294-if-let-guard/scoping-consistency.rs
Normal file
24
tests/ui/rfcs/rfc-2294-if-let-guard/scoping-consistency.rs
Normal file
@ -0,0 +1,24 @@
|
||||
// Check that temporaries in if-let guards are correctly scoped.
|
||||
|
||||
// build-pass
|
||||
// -Zvalidate-mir
|
||||
|
||||
#![feature(if_let_guard)]
|
||||
|
||||
fn fun() {
|
||||
match 0 {
|
||||
_ => (),
|
||||
_ if let Some(s) = std::convert::identity(&Some(String::new())) => {}
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
|
||||
fn funner() {
|
||||
match 0 {
|
||||
_ => (),
|
||||
_ | _ if let Some(s) = std::convert::identity(&Some(String::new())) => {}
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
@ -124,7 +124,7 @@ body:
|
||||
body:
|
||||
Expr {
|
||||
ty: bool
|
||||
temp_lifetime: Some(Node(13))
|
||||
temp_lifetime: Some(Node(12))
|
||||
span: $DIR/thir-tree-match.rs:17:36: 17:40 (#0)
|
||||
kind:
|
||||
Scope {
|
||||
@ -133,7 +133,7 @@ body:
|
||||
value:
|
||||
Expr {
|
||||
ty: bool
|
||||
temp_lifetime: Some(Node(13))
|
||||
temp_lifetime: Some(Node(12))
|
||||
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)
|
||||
@ -176,7 +176,7 @@ body:
|
||||
body:
|
||||
Expr {
|
||||
ty: bool
|
||||
temp_lifetime: Some(Node(19))
|
||||
temp_lifetime: Some(Node(18))
|
||||
span: $DIR/thir-tree-match.rs:18:27: 18:32 (#0)
|
||||
kind:
|
||||
Scope {
|
||||
@ -185,7 +185,7 @@ body:
|
||||
value:
|
||||
Expr {
|
||||
ty: bool
|
||||
temp_lifetime: Some(Node(19))
|
||||
temp_lifetime: Some(Node(18))
|
||||
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)
|
||||
@ -220,7 +220,7 @@ body:
|
||||
body:
|
||||
Expr {
|
||||
ty: bool
|
||||
temp_lifetime: Some(Node(24))
|
||||
temp_lifetime: Some(Node(23))
|
||||
span: $DIR/thir-tree-match.rs:19:24: 19:28 (#0)
|
||||
kind:
|
||||
Scope {
|
||||
@ -229,7 +229,7 @@ body:
|
||||
value:
|
||||
Expr {
|
||||
ty: bool
|
||||
temp_lifetime: Some(Node(24))
|
||||
temp_lifetime: Some(Node(23))
|
||||
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)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user