From 876f04d5edfbc26038c1addd0af4364351186759 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Sun, 2 Jan 2022 23:21:14 +0800 Subject: [PATCH 1/7] refine scopes Co-authored-by: Felix S --- Cargo.lock | 1 + compiler/rustc_middle/src/middle/region.rs | 33 ++++ compiler/rustc_passes/Cargo.toml | 1 + compiler/rustc_passes/src/region.rs | 156 +++++++++++++++++- ...age_markers.main.RemoveStorageMarkers.diff | 3 +- .../async-await/issue-64130-4-async-move.rs | 11 +- .../issue-64130-4-async-move.stderr | 16 +- 7 files changed, 199 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0f344dc920e57..dfe35e20a4bcb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4134,6 +4134,7 @@ dependencies = [ "rustc_session", "rustc_span", "rustc_target", + "smallvec", "tracing", ] diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index 75dd223d014d4..6d25dac689aea 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -234,6 +234,8 @@ pub struct ScopeTree { /// escape into 'static and should have no local cleanup scope. rvalue_scopes: FxHashMap>, + eager_scopes: FxHashMap, + /// If there are any `yield` nested within a scope, this map /// stores the `Span` of the last one and its index in the /// postorder of the Visitor traversal on the HIR. @@ -358,6 +360,32 @@ impl ScopeTree { self.rvalue_scopes.insert(var, lifetime); } + pub fn record_local_access_scope(&mut self, var: hir::ItemLocalId, proposed_lifetime: Scope) { + debug!("record_local_access_scope(sub={:?}, sup={:?})", var, proposed_lifetime); + let mut id = Scope { id: var, data: ScopeData::Node }; + + while let Some(&(p, _)) = self.parent_map.get(&id) { + match p.data { + ScopeData::Destruction => return, + _ => id = p, + } + if id == proposed_lifetime { + // proposed lifetime outlives the destruction lifetime + self.record_rvalue_scope(var, Some(proposed_lifetime)); + return; + } + } + } + + pub fn record_eager_scope(&mut self, var: hir::ItemLocalId, proposed_lifetime: Scope) { + if let Some(destruction) = self.temporary_scope(var) { + if self.is_subscope_of(destruction, proposed_lifetime) { + return; + } + } + self.eager_scopes.insert(var, proposed_lifetime); + } + /// Returns the narrowest scope that encloses `id`, if any. pub fn opt_encl_scope(&self, id: Scope) -> Option { self.parent_map.get(&id).cloned().map(|(p, _)| p) @@ -393,6 +421,9 @@ impl ScopeTree { } _ => id = p, } + if let Some(&eager_scope) = self.eager_scopes.get(&id.item_local_id()) { + return Some(eager_scope); + } } debug!("temporary_scope({:?}) = None", expr_id); @@ -444,6 +475,7 @@ impl<'a> HashStable> for ScopeTree { ref var_map, ref destruction_scopes, ref rvalue_scopes, + ref eager_scopes, ref yield_in_scope, } = *self; @@ -456,6 +488,7 @@ impl<'a> HashStable> for ScopeTree { var_map.hash_stable(hcx, hasher); destruction_scopes.hash_stable(hcx, hasher); rvalue_scopes.hash_stable(hcx, hasher); + eager_scopes.hash_stable(hcx, hasher); yield_in_scope.hash_stable(hcx, hasher); } } diff --git a/compiler/rustc_passes/Cargo.toml b/compiler/rustc_passes/Cargo.toml index 39e578bce7ef6..1ed7c26f056f7 100644 --- a/compiler/rustc_passes/Cargo.toml +++ b/compiler/rustc_passes/Cargo.toml @@ -5,6 +5,7 @@ edition = "2021" [dependencies] tracing = "0.1" +smallvec = { version = "1.6.1", features = ["union", "may_dangle"] } rustc_middle = { path = "../rustc_middle" } rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } diff --git a/compiler/rustc_passes/src/region.rs b/compiler/rustc_passes/src/region.rs index fdf93e5893247..f2e712f262f1d 100644 --- a/compiler/rustc_passes/src/region.rs +++ b/compiler/rustc_passes/src/region.rs @@ -18,6 +18,7 @@ use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; use rustc_span::source_map; use rustc_span::Span; +use smallvec::SmallVec; use std::mem; @@ -203,6 +204,106 @@ fn resolve_stmt<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, stmt: &'tcx h visitor.cx.parent = prev_parent; } +fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet { + struct LocalAccessResolutionVisitor<'a> { + locals: &'a mut FxHashSet, + } + impl<'a> LocalAccessResolutionVisitor<'a> { + fn probe<'b>(&mut self, expr: &'b Expr<'b>) { + if self.locals.contains(&expr.hir_id.local_id) { + return; + } + let mut nested_expr = expr; + let mut ops = SmallVec::<[_; 4]>::new(); + enum OpTy { + Ref, + Deref, + Project, + Local, + } + loop { + match nested_expr.kind { + hir::ExprKind::Path(hir::QPath::Resolved( + _, + hir::Path { res: hir::def::Res::Local(_), .. }, + )) => { + ops.push((nested_expr, OpTy::Local)); + break; + } + hir::ExprKind::AddrOf(_, _, subexpr) => { + ops.push((nested_expr, OpTy::Ref)); + nested_expr = subexpr; + } + hir::ExprKind::Unary(hir::UnOp::Deref, subexpr) => { + ops.push((nested_expr, OpTy::Deref)); + nested_expr = subexpr; + } + hir::ExprKind::Field(subexpr, _) => { + ops.push((nested_expr, OpTy::Project)); + nested_expr = subexpr; + } + hir::ExprKind::Index(subexpr, idxexpr) => { + ops.push((nested_expr, OpTy::Project)); + nested_expr = subexpr; + intravisit::walk_expr(self, idxexpr); + } + _ => { + drop(ops); + intravisit::walk_expr(self, expr); + return; + } + } + } + let mut ref_level = SmallVec::<[_; 4]>::new(); + let mut ops_iter = ops.into_iter().rev(); + ops_iter.next().unwrap(); + for (expr, op) in ops_iter { + match op { + OpTy::Ref => { + ref_level.push(expr); + } + OpTy::Deref => { + if let Some(ref_expr) = ref_level.pop() { + self.locals.insert(ref_expr.hir_id.local_id); + } + } + OpTy::Project => { + ref_level.clear(); + } + OpTy::Local => { + panic!("unexpected encounter of Local") + } + } + } + self.locals.insert(expr.hir_id.local_id); + } + } + impl<'a, 'b> Visitor<'a> for LocalAccessResolutionVisitor<'b> { + type Map = intravisit::ErasedMap<'a>; + fn nested_visit_map(&mut self) -> NestedVisitorMap { + NestedVisitorMap::None + } + fn visit_expr(&mut self, expr: &'a Expr<'a>) { + match expr.kind { + hir::ExprKind::AddrOf(..) + | hir::ExprKind::Unary(hir::UnOp::Deref, _) + | hir::ExprKind::Field(..) + | hir::ExprKind::Index(..) + | hir::ExprKind::Path(..) => self.probe(expr), + hir::ExprKind::Block(..) + | hir::ExprKind::Closure(..) + | hir::ExprKind::ConstBlock(..) => {} + _ => intravisit::walk_expr(self, expr), + } + } + } + let mut locals = Default::default(); + let mut resolution_visitor = LocalAccessResolutionVisitor { locals: &mut locals }; + resolution_visitor.visit_expr(expr); + // visitor.terminating_scopes.extend(locals.iter().copied()); + locals +} + fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx hir::Expr<'tcx>) { debug!("resolve_expr - pre-increment {} expr = {:?}", visitor.expr_and_pat_count, expr); @@ -396,7 +497,15 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h let expr_cx = visitor.cx; visitor.enter_scope(Scope { id: then.hir_id.local_id, data: ScopeData::IfThen }); visitor.cx.var_parent = visitor.cx.parent; - visitor.visit_expr(cond); + { + visitor.visit_expr(cond); + let lifetime = Scope { id: cond.hir_id.local_id, data: ScopeData::Node }; + for local in mark_local_terminating_scopes(cond) { + if local != cond.hir_id.local_id { + visitor.scope_tree.record_local_access_scope(local, lifetime); + } + } + } visitor.visit_expr(then); visitor.cx = expr_cx; visitor.visit_expr(otherwise); @@ -406,11 +515,39 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h let expr_cx = visitor.cx; visitor.enter_scope(Scope { id: then.hir_id.local_id, data: ScopeData::IfThen }); visitor.cx.var_parent = visitor.cx.parent; - visitor.visit_expr(cond); + { + visitor.visit_expr(cond); + let lifetime = Scope { id: cond.hir_id.local_id, data: ScopeData::Node }; + for local in mark_local_terminating_scopes(cond) { + if local != cond.hir_id.local_id { + visitor.scope_tree.record_local_access_scope(local, lifetime); + } + } + } visitor.visit_expr(then); visitor.cx = expr_cx; } + hir::ExprKind::Match(subexpr, arms, _) => { + visitor.visit_expr(subexpr); + let lifetime = Scope { id: subexpr.hir_id.local_id, data: ScopeData::Node }; + for local in mark_local_terminating_scopes(subexpr) { + if local != subexpr.hir_id.local_id { + visitor.scope_tree.record_local_access_scope(local, lifetime); + } + } + walk_list!(visitor, visit_arm, arms); + } + + hir::ExprKind::Index(subexpr, idxexpr) => { + visitor.visit_expr(subexpr); + visitor.visit_expr(idxexpr); + visitor.scope_tree.record_eager_scope( + idxexpr.hir_id.local_id, + Scope { id: expr.hir_id.local_id, data: ScopeData::Node }, + ); + } + _ => intravisit::walk_expr(visitor, expr), } @@ -683,10 +820,17 @@ fn resolve_local<'tcx>( visitor.scope_tree.record_rvalue_scope(expr.hir_id.local_id, blk_scope); match expr.kind { - hir::ExprKind::AddrOf(_, _, ref subexpr) - | hir::ExprKind::Unary(hir::UnOp::Deref, ref subexpr) - | hir::ExprKind::Field(ref subexpr, _) - | hir::ExprKind::Index(ref subexpr, _) => { + hir::ExprKind::AddrOf(_, _, subexpr) + | hir::ExprKind::Unary(hir::UnOp::Deref, subexpr) + | hir::ExprKind::Field(subexpr, _) + | hir::ExprKind::Index(subexpr, _) => { + if let hir::ExprKind::Path(hir::QPath::Resolved( + _, + hir::Path { res: hir::def::Res::Local(_), .. }, + )) = &subexpr.kind + { + return; + } expr = &subexpr; } _ => { diff --git a/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff b/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff index 2dfc94f21868e..124017463d06a 100644 --- a/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff +++ b/src/test/mir-opt/remove_storage_markers.main.RemoveStorageMarkers.diff @@ -71,7 +71,6 @@ - StorageDead(_13); // scope 3 at $DIR/remove_storage_markers.rs:9:16: 9:17 _6 = const (); // scope 3 at $DIR/remove_storage_markers.rs:8:20: 10:6 - StorageDead(_12); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 -- StorageDead(_9); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 - StorageDead(_7); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 - StorageDead(_6); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 _5 = const (); // scope 2 at $DIR/remove_storage_markers.rs:8:5: 10:6 @@ -80,7 +79,6 @@ bb3: { _0 = const (); // scope 2 at $DIR/remove_storage_markers.rs:8:5: 10:6 -- StorageDead(_9); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 - StorageDead(_7); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 - StorageDead(_6); // scope 2 at $DIR/remove_storage_markers.rs:10:5: 10:6 - StorageDead(_4); // scope 1 at $DIR/remove_storage_markers.rs:10:5: 10:6 @@ -91,6 +89,7 @@ bb4: { - StorageDead(_14); // scope 5 at $DIR/remove_storage_markers.rs:8:14: 8:19 +- StorageDead(_9); // scope 2 at $DIR/remove_storage_markers.rs:8:18: 8:19 - StorageDead(_8); // scope 2 at $DIR/remove_storage_markers.rs:8:18: 8:19 _10 = discriminant(_7); // scope 2 at $DIR/remove_storage_markers.rs:8:14: 8:19 switchInt(move _10) -> [0_isize: bb3, otherwise: bb2]; // scope 2 at $DIR/remove_storage_markers.rs:8:14: 8:19 diff --git a/src/test/ui/async-await/issue-64130-4-async-move.rs b/src/test/ui/async-await/issue-64130-4-async-move.rs index 2538f34351e5a..e74c740f893f3 100644 --- a/src/test/ui/async-await/issue-64130-4-async-move.rs +++ b/src/test/ui/async-await/issue-64130-4-async-move.rs @@ -10,16 +10,15 @@ impl Client { } } -async fn get() { } +async fn get() {} pub fn foo() -> impl Future + Send { - //~^ ERROR future cannot be sent between threads safely - let client = Client(Box::new(true)); - async move { - match client.status() { + async { + //~^ ERROR future cannot be sent between threads safely + match Client(Box::new(true)).status() { 200 => { let _x = get().await; - }, + } _ => (), } } diff --git a/src/test/ui/async-await/issue-64130-4-async-move.stderr b/src/test/ui/async-await/issue-64130-4-async-move.stderr index d631e6dc7f7e9..0eeed986cd7eb 100644 --- a/src/test/ui/async-await/issue-64130-4-async-move.stderr +++ b/src/test/ui/async-await/issue-64130-4-async-move.stderr @@ -6,21 +6,21 @@ LL | pub fn foo() -> impl Future + Send { | = help: the trait `Sync` is not implemented for `(dyn Any + Send + 'static)` note: future is not `Send` as this value is used across an await - --> $DIR/issue-64130-4-async-move.rs:21:31 + --> $DIR/issue-64130-4-async-move.rs:20:31 | -LL | match client.status() { - | ------ has type `&Client` which is not `Send` +LL | match Client(Box::new(true)).status() { + | ---------------------- has type `&Client` which is not `Send` LL | 200 => { LL | let _x = get().await; - | ^^^^^^ await occurs here, with `client` maybe used later + | ^^^^^^ await occurs here, with `Client(Box::new(true))` maybe used later ... LL | } - | - `client` is later dropped here + | - `Client(Box::new(true))` is later dropped here help: consider moving this into a `let` binding to create a shorter lived borrow - --> $DIR/issue-64130-4-async-move.rs:19:15 + --> $DIR/issue-64130-4-async-move.rs:18:15 | -LL | match client.status() { - | ^^^^^^^^^^^^^^^ +LL | match Client(Box::new(true)).status() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error From bc114c47affc43338d59bd93fea9c3d3128f7b7c Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Mon, 3 Jan 2022 12:10:02 +0800 Subject: [PATCH 2/7] tests from the relevant issue trackers --- .../issue-69663-lingering-local-borrows.rs | 51 +++++++++++++++++++ .../refined-region-for-local-accesses.rs | 32 ++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 src/test/ui/async-await/issue-69663-lingering-local-borrows.rs create mode 100644 src/test/ui/generator/refined-region-for-local-accesses.rs diff --git a/src/test/ui/async-await/issue-69663-lingering-local-borrows.rs b/src/test/ui/async-await/issue-69663-lingering-local-borrows.rs new file mode 100644 index 0000000000000..9c77a1fc0266e --- /dev/null +++ b/src/test/ui/async-await/issue-69663-lingering-local-borrows.rs @@ -0,0 +1,51 @@ +// edition:2018 +// run-pass + +use std::ops::Index; + +/// A `Send + !Sync` for demonstration purposes. +struct Banana(*mut ()); +unsafe impl Send for Banana {} + +impl Banana { + /// Make a static mutable reference to Banana for convenience purposes. + /// + /// Any potential unsoundness here is not super relevant to the issue at hand. + fn new() -> &'static mut Banana { + static mut BANANA: Banana = Banana(std::ptr::null_mut()); + unsafe { &mut BANANA } + } +} + +// Peach is still Send (because `impl Send for &mut T where T: Send`) +struct Peach<'a>(&'a mut Banana); + +impl<'a> std::ops::Index for Peach<'a> { + type Output = (); + fn index(&self, _: usize) -> &() { + &() + } +} + +async fn baz(_: &()) {} + +async fn bar() { + let peach = Peach(Banana::new()); + let r = &*peach.index(0); + baz(r).await; + peach.index(0); // make sure peach is retained across yield point +} + +async fn bat() { + let peach = Peach(Banana::new()); + let r = &peach[0]; + baz(r).await; + peach[0]; // make sure peach is retained across yield point +} + +fn assert_send(_: T) {} + +pub fn main() { + assert_send(bar()); + assert_send(bat()); +} diff --git a/src/test/ui/generator/refined-region-for-local-accesses.rs b/src/test/ui/generator/refined-region-for-local-accesses.rs new file mode 100644 index 0000000000000..340408f3eca89 --- /dev/null +++ b/src/test/ui/generator/refined-region-for-local-accesses.rs @@ -0,0 +1,32 @@ +// edition:2018 +// run-pass +#![feature(generators, generator_trait, negative_impls)] + +fn assert_send(_: T) {} + +struct Client; + +impl Client { + fn status(&self) -> i16 { + 200 + } +} + +impl !Sync for Client {} + +fn status(_: &Client) -> i16 { + 200 +} + +fn main() { + let g = || { + let x = Client; + match status(&x) { + _ => yield, + } + match (&*&x).status() { + _ => yield, + } + }; + assert_send(g); +} From 4097274050dc5c9c74019b8589f76381c328d11d Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Mon, 3 Jan 2022 12:48:47 +0800 Subject: [PATCH 3/7] comments about the workings --- compiler/rustc_middle/src/middle/region.rs | 9 +++++++++ compiler/rustc_passes/src/region.rs | 12 +++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index 6d25dac689aea..6323dad57daf3 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -234,6 +234,13 @@ pub struct ScopeTree { /// escape into 'static and should have no local cleanup scope. rvalue_scopes: FxHashMap>, + /// Sometimes destruction scopes are too overarching in the sense that + /// all temporaries are dropped while it may be the case that + /// only a subset of temporaries is requested to be dropped. + /// For instance, `x[y.z()]` indexing operation can discard the temporaries + /// generated in evaluating `y.z()`. + /// `eager_scopes` is a map from items to a region smaller than a destruction scope + /// so that the temporaries do not need to linger unnecessarily. eager_scopes: FxHashMap, /// If there are any `yield` nested within a scope, this map @@ -380,6 +387,8 @@ impl ScopeTree { pub fn record_eager_scope(&mut self, var: hir::ItemLocalId, proposed_lifetime: Scope) { if let Some(destruction) = self.temporary_scope(var) { if self.is_subscope_of(destruction, proposed_lifetime) { + // If the current temporary scope is already a subset of the proposed region, + // it is safe to keep this scope and discard the proposal. return; } } diff --git a/compiler/rustc_passes/src/region.rs b/compiler/rustc_passes/src/region.rs index f2e712f262f1d..6f13666edbe31 100644 --- a/compiler/rustc_passes/src/region.rs +++ b/compiler/rustc_passes/src/region.rs @@ -221,6 +221,12 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet Project, Local, } + // Here we are looking for a chain of access to extract a place reference for + // a local variable. + // For instance, given `&*(&*x[y]).z().u`, `probe` will pick out + // the sub-expressions `&*x[y]` and `y` as expressions making accesses to locals. + // The place references generated can be discarded earlier as MIR allows, + // and, thus, can be assigned a smaller region. loop { match nested_expr.kind { hir::ExprKind::Path(hir::QPath::Resolved( @@ -268,6 +274,9 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet } } OpTy::Project => { + // The generated MIR produces temporaries for the each of the nested references, + // such as `&x`, `&&x` in `(&&&x).y` for the field access of `y`. + // These temporaries must stay alive even after the field access. ref_level.clear(); } OpTy::Local => { @@ -290,6 +299,8 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet | hir::ExprKind::Field(..) | hir::ExprKind::Index(..) | hir::ExprKind::Path(..) => self.probe(expr), + + // We do not probe into other function bodies or blocks. hir::ExprKind::Block(..) | hir::ExprKind::Closure(..) | hir::ExprKind::ConstBlock(..) => {} @@ -300,7 +311,6 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet let mut locals = Default::default(); let mut resolution_visitor = LocalAccessResolutionVisitor { locals: &mut locals }; resolution_visitor.visit_expr(expr); - // visitor.terminating_scopes.extend(locals.iter().copied()); locals } From 906cdbfa6f65bb5c7c5e4f055b626689cc03cbf6 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Sat, 8 Jan 2022 15:31:55 +0800 Subject: [PATCH 4/7] simplify tracking and opt for build-pass testing --- compiler/rustc_middle/src/middle/region.rs | 6 +++++- compiler/rustc_passes/src/region.rs | 11 +++-------- .../issue-69663-lingering-local-borrows.rs | 2 +- .../ui/generator/refined-region-for-local-accesses.rs | 2 +- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index 6323dad57daf3..5f9ddae5f8abc 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -368,7 +368,10 @@ impl ScopeTree { } pub fn record_local_access_scope(&mut self, var: hir::ItemLocalId, proposed_lifetime: Scope) { - debug!("record_local_access_scope(sub={:?}, sup={:?})", var, proposed_lifetime); + debug!( + "record_local_access_scope(var={:?}, proposed_lifetime={:?})", + var, proposed_lifetime + ); let mut id = Scope { id: var, data: ScopeData::Node }; while let Some(&(p, _)) = self.parent_map.get(&id) { @@ -385,6 +388,7 @@ impl ScopeTree { } pub fn record_eager_scope(&mut self, var: hir::ItemLocalId, proposed_lifetime: Scope) { + debug!("record_eager_scope(var={:?}, proposed_lifetime={:?})", var, proposed_lifetime); if let Some(destruction) = self.temporary_scope(var) { if self.is_subscope_of(destruction, proposed_lifetime) { // If the current temporary scope is already a subset of the proposed region, diff --git a/compiler/rustc_passes/src/region.rs b/compiler/rustc_passes/src/region.rs index 6f13666edbe31..b48c3b091834c 100644 --- a/compiler/rustc_passes/src/region.rs +++ b/compiler/rustc_passes/src/region.rs @@ -219,7 +219,6 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet Ref, Deref, Project, - Local, } // Here we are looking for a chain of access to extract a place reference for // a local variable. @@ -233,7 +232,8 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet _, hir::Path { res: hir::def::Res::Local(_), .. }, )) => { - ops.push((nested_expr, OpTy::Local)); + // We have reached the end of the access path + // because a local variable access is encountered break; } hir::ExprKind::AddrOf(_, _, subexpr) => { @@ -261,9 +261,7 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet } } let mut ref_level = SmallVec::<[_; 4]>::new(); - let mut ops_iter = ops.into_iter().rev(); - ops_iter.next().unwrap(); - for (expr, op) in ops_iter { + for (expr, op) in ops.into_iter().rev() { match op { OpTy::Ref => { ref_level.push(expr); @@ -279,9 +277,6 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet // These temporaries must stay alive even after the field access. ref_level.clear(); } - OpTy::Local => { - panic!("unexpected encounter of Local") - } } } self.locals.insert(expr.hir_id.local_id); diff --git a/src/test/ui/async-await/issue-69663-lingering-local-borrows.rs b/src/test/ui/async-await/issue-69663-lingering-local-borrows.rs index 9c77a1fc0266e..7dfde370f31d8 100644 --- a/src/test/ui/async-await/issue-69663-lingering-local-borrows.rs +++ b/src/test/ui/async-await/issue-69663-lingering-local-borrows.rs @@ -1,5 +1,5 @@ // edition:2018 -// run-pass +// build-pass use std::ops::Index; diff --git a/src/test/ui/generator/refined-region-for-local-accesses.rs b/src/test/ui/generator/refined-region-for-local-accesses.rs index 340408f3eca89..680770a48f96a 100644 --- a/src/test/ui/generator/refined-region-for-local-accesses.rs +++ b/src/test/ui/generator/refined-region-for-local-accesses.rs @@ -1,5 +1,5 @@ // edition:2018 -// run-pass +// build-pass #![feature(generators, generator_trait, negative_impls)] fn assert_send(_: T) {} From 7cad314fc04d25ad27d5040fd7789eafadd7efb1 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Tue, 11 Jan 2022 21:57:10 +0800 Subject: [PATCH 5/7] stop descending to avoid duplicated effort --- compiler/rustc_passes/src/region.rs | 11 ++++------- .../ui/generator/refined-region-for-local-accesses.rs | 8 ++++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_passes/src/region.rs b/compiler/rustc_passes/src/region.rs index b48c3b091834c..678b0c6e5e222 100644 --- a/compiler/rustc_passes/src/region.rs +++ b/compiler/rustc_passes/src/region.rs @@ -244,15 +244,10 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet ops.push((nested_expr, OpTy::Deref)); nested_expr = subexpr; } - hir::ExprKind::Field(subexpr, _) => { + hir::ExprKind::Field(subexpr, _) | hir::ExprKind::Index(subexpr, _) => { ops.push((nested_expr, OpTy::Project)); nested_expr = subexpr; } - hir::ExprKind::Index(subexpr, idxexpr) => { - ops.push((nested_expr, OpTy::Project)); - nested_expr = subexpr; - intravisit::walk_expr(self, idxexpr); - } _ => { drop(ops); intravisit::walk_expr(self, expr); @@ -296,7 +291,9 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet | hir::ExprKind::Path(..) => self.probe(expr), // We do not probe into other function bodies or blocks. - hir::ExprKind::Block(..) + hir::ExprKind::If(..) + | hir::ExprKind::Match(..) + | hir::ExprKind::Block(..) | hir::ExprKind::Closure(..) | hir::ExprKind::ConstBlock(..) => {} _ => intravisit::walk_expr(self, expr), diff --git a/src/test/ui/generator/refined-region-for-local-accesses.rs b/src/test/ui/generator/refined-region-for-local-accesses.rs index 680770a48f96a..67daf6bc6aac5 100644 --- a/src/test/ui/generator/refined-region-for-local-accesses.rs +++ b/src/test/ui/generator/refined-region-for-local-accesses.rs @@ -7,6 +7,10 @@ fn assert_send(_: T) {} struct Client; impl Client { + fn zero(&self) -> usize { + 0 + } + fn status(&self) -> i16 { 200 } @@ -27,6 +31,10 @@ fn main() { match (&*&x).status() { _ => yield, } + let a = [0]; + match a[Client.zero()] { + _ => yield, + } }; assert_send(g); } From 849eb4b2bdd5a63494e5d7901b3257ce3e138d4b Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Mon, 24 Jan 2022 23:27:19 +0800 Subject: [PATCH 6/7] use the new visitor api --- compiler/rustc_passes/src/region.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_passes/src/region.rs b/compiler/rustc_passes/src/region.rs index 678b0c6e5e222..de26585e9014e 100644 --- a/compiler/rustc_passes/src/region.rs +++ b/compiler/rustc_passes/src/region.rs @@ -278,10 +278,6 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet } } impl<'a, 'b> Visitor<'a> for LocalAccessResolutionVisitor<'b> { - type Map = intravisit::ErasedMap<'a>; - fn nested_visit_map(&mut self) -> NestedVisitorMap { - NestedVisitorMap::None - } fn visit_expr(&mut self, expr: &'a Expr<'a>) { match expr.kind { hir::ExprKind::AddrOf(..) @@ -290,7 +286,8 @@ fn mark_local_terminating_scopes<'tcx>(expr: &'tcx hir::Expr<'tcx>) -> FxHashSet | hir::ExprKind::Index(..) | hir::ExprKind::Path(..) => self.probe(expr), - // We do not probe into other function bodies or blocks. + // We do not probe into other function bodies or blocks, + // neither `if`s and `match`es because they will be covered in deeper visits hir::ExprKind::If(..) | hir::ExprKind::Match(..) | hir::ExprKind::Block(..) From 348584d3e2cd3feb3aa76b2dbe0bbbd8f34b971e Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Thu, 10 Feb 2022 09:53:26 +0800 Subject: [PATCH 7/7] rebase and update tests --- src/test/ui/async-await/issue-64130-4-async-move.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/async-await/issue-64130-4-async-move.rs b/src/test/ui/async-await/issue-64130-4-async-move.rs index e74c740f893f3..99a3b28c5e00f 100644 --- a/src/test/ui/async-await/issue-64130-4-async-move.rs +++ b/src/test/ui/async-await/issue-64130-4-async-move.rs @@ -13,8 +13,8 @@ impl Client { async fn get() {} pub fn foo() -> impl Future + Send { + //~^ ERROR future cannot be sent between threads safely async { - //~^ ERROR future cannot be sent between threads safely match Client(Box::new(true)).status() { 200 => { let _x = get().await;