From ab8c50f9648065d3f4d2423cfa39a9750d04823d Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Tue, 10 May 2022 17:58:18 -0700 Subject: Add drop tracking version of yielding-in-match-guard.rs --- .../drop-tracking-yielding-in-match-guards.rs | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs (limited to 'src/test') diff --git a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs new file mode 100644 index 00000000000..e6206249d91 --- /dev/null +++ b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs @@ -0,0 +1,25 @@ +// build-pass +// edition:2018 +// compile-flags: -Zdrop-tracking + +// This test is derived from +// https://github.com/rust-lang/rust/issues/72651#issuecomment-668720468 + +// This test demonstrates that, in `async fn g()`, +// indeed a temporary borrow `y` from `x` is live +// while `f().await` is being evaluated. +// Thus, `&'_ u8` should be included in type signature +// of the underlying generator. + +async fn f() -> u8 { 1 } + +async fn i(x: u8) { + match x { + y if f().await == y + 1 => (), + _ => (), + } +} + +fn main() { + let _ = i(8); +} -- cgit 1.4.1-3-g733a5 From 577bf0f3542d7cd49967d24811dd58b94ab7ae19 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Fri, 13 May 2022 13:38:36 -0700 Subject: Further reduce test case Thanks to @tmiasko for this one! --- .../ui/generator/drop-tracking-yielding-in-match-guards.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'src/test') diff --git a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs index e6206249d91..c818963e466 100644 --- a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs +++ b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs @@ -11,15 +11,11 @@ // Thus, `&'_ u8` should be included in type signature // of the underlying generator. -async fn f() -> u8 { 1 } - -async fn i(x: u8) { - match x { - y if f().await == y + 1 => (), - _ => (), - } -} +#![feature(generators)] fn main() { - let _ = i(8); + let _ = static |x: u8| match x { + y if { yield } == y + 1 => (), + _ => (), + }; } -- cgit 1.4.1-3-g733a5 From d5b72058fe24834807c13991b580c712330ec806 Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Fri, 13 May 2022 14:24:41 -0700 Subject: Count copies of locals as borrowed temporaries --- .../drop_ranges/record_consumed_borrow.rs | 8 +++++++- .../ui/generator/drop-tracking-yielding-in-match-guards.rs | 14 +++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) (limited to 'src/test') diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs index e89a8961996..ec83acf08b7 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs @@ -171,7 +171,13 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> { .insert(TrackedValue::from_place_with_projections_allowed(place_with_id)); // For copied we treat this mostly like a borrow except that we don't add the place - // to borrowed_temporaries because the copy is consumed. + // to borrowed_temporaries if it is not a local because the copy is consumed. + match place_with_id.place.base { + PlaceBase::Rvalue | PlaceBase::StaticItem | PlaceBase::Upvar(_) => (), + PlaceBase::Local(_) => { + self.places.borrowed_temporaries.insert(place_with_id.hir_id); + } + } } fn mutate( diff --git a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs index c818963e466..9efe64a62e7 100644 --- a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs +++ b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs @@ -14,8 +14,20 @@ #![feature(generators)] fn main() { - let _ = static |x: u8| match x { + let _a = static |x: u8| match x { y if { yield } == y + 1 => (), _ => (), }; + + static STATIC: u8 = 42; + let _b = static |x: u8| match x { + y if { yield } == STATIC + 1 => (), + _ => (), + }; + + let upvar = 42u8; + let _c = static |x: u8| match x { + y if { yield } == upvar + 1 => (), + _ => (), + }; } -- cgit 1.4.1-3-g733a5 From 7db4c0277de996531c2e794ba16bb6746af65fef Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Tue, 17 May 2022 15:04:05 -0700 Subject: Revert "Count copies of locals as borrowed temporaries" This reverts commit 0d270b5e9f48268735f9a05462df65c9d1039855. --- .../drop_ranges/record_consumed_borrow.rs | 8 +------- .../ui/generator/drop-tracking-yielding-in-match-guards.rs | 14 +------------- 2 files changed, 2 insertions(+), 20 deletions(-) (limited to 'src/test') diff --git a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs index ec83acf08b7..e89a8961996 100644 --- a/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs +++ b/compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs @@ -171,13 +171,7 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> { .insert(TrackedValue::from_place_with_projections_allowed(place_with_id)); // For copied we treat this mostly like a borrow except that we don't add the place - // to borrowed_temporaries if it is not a local because the copy is consumed. - match place_with_id.place.base { - PlaceBase::Rvalue | PlaceBase::StaticItem | PlaceBase::Upvar(_) => (), - PlaceBase::Local(_) => { - self.places.borrowed_temporaries.insert(place_with_id.hir_id); - } - } + // to borrowed_temporaries because the copy is consumed. } fn mutate( diff --git a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs index 9efe64a62e7..c818963e466 100644 --- a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs +++ b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs @@ -14,20 +14,8 @@ #![feature(generators)] fn main() { - let _a = static |x: u8| match x { + let _ = static |x: u8| match x { y if { yield } == y + 1 => (), _ => (), }; - - static STATIC: u8 = 42; - let _b = static |x: u8| match x { - y if { yield } == STATIC + 1 => (), - _ => (), - }; - - let upvar = 42u8; - let _c = static |x: u8| match x { - y if { yield } == upvar + 1 => (), - _ => (), - }; } -- cgit 1.4.1-3-g733a5 From d08efdec1c0e3c8135a547b2853af49a7f107f7e Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Tue, 17 May 2022 15:36:39 -0700 Subject: Borrow guard patterns for the body of the guard --- compiler/rustc_hir/src/hir.rs | 14 ++++++++ .../rustc_typeck/src/check/generator_interior.rs | 39 ++++++++++++++-------- compiler/rustc_typeck/src/expr_use_visitor.rs | 21 +++++++++--- .../drop-tracking-yielding-in-match-guards.rs | 9 ----- 4 files changed, 57 insertions(+), 26 deletions(-) (limited to 'src/test') diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 81d544c7b96..5bd6632640f 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1323,6 +1323,20 @@ pub enum Guard<'hir> { IfLet(&'hir Let<'hir>), } +impl<'hir> Guard<'hir> { + /// Returns the body of the guard + /// + /// In other words, returns the e in either of the following: + /// + /// - `if e` + /// - `if let x = e` + pub fn body(&self) -> &'hir Expr<'hir> { + match self { + Guard::If(e) | Guard::IfLet(_, e) => e, + } + } +} + #[derive(Debug, HashStable_Generic)] pub struct ExprField<'hir> { #[stable_hasher(ignore)] diff --git a/compiler/rustc_typeck/src/check/generator_interior.rs b/compiler/rustc_typeck/src/check/generator_interior.rs index 92a2584a6de..d530504489a 100644 --- a/compiler/rustc_typeck/src/check/generator_interior.rs +++ b/compiler/rustc_typeck/src/check/generator_interior.rs @@ -285,14 +285,13 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { self.visit_pat(pat); if let Some(ref g) = guard { self.guard_bindings.push(<_>::default()); - ArmPatCollector { - guard_bindings_set: &mut self.guard_bindings_set, - guard_bindings: self - .guard_bindings - .last_mut() - .expect("should have pushed at least one earlier"), + { + ArmPatCollector { + interior_visitor: self, + scope: Scope { id: g.body().hir_id.local_id, data: ScopeData::Node }, + } + .visit_pat(pat); } - .visit_pat(pat); match g { Guard::If(ref e) => { @@ -459,17 +458,31 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { } } -struct ArmPatCollector<'a> { - guard_bindings_set: &'a mut HirIdSet, - guard_bindings: &'a mut SmallVec<[HirId; 4]>, +struct ArmPatCollector<'a, 'b, 'tcx> { + interior_visitor: &'a mut InteriorVisitor<'b, 'tcx>, + scope: Scope, } -impl<'a, 'tcx> Visitor<'tcx> for ArmPatCollector<'a> { +impl<'a, 'b, 'tcx> Visitor<'tcx> for ArmPatCollector<'a, 'b, 'tcx> { fn visit_pat(&mut self, pat: &'tcx Pat<'tcx>) { intravisit::walk_pat(self, pat); if let PatKind::Binding(_, id, ..) = pat.kind { - self.guard_bindings.push(id); - self.guard_bindings_set.insert(id); + self.interior_visitor + .guard_bindings + .last_mut() + .expect("should have pushed at least one earlier") + .push(id); + self.interior_visitor.guard_bindings_set.insert(id); + + let ty = self.interior_visitor.fcx.typeck_results.borrow().node_type(id); + let ty = self.interior_visitor.fcx.tcx.mk_ref( + // Use `ReErased` as `resolve_interior` is going to replace all the regions anyway. + self.interior_visitor.fcx.tcx.mk_region(ty::ReErased), + ty::TypeAndMut { ty, mutbl: hir::Mutability::Not }, + ); + // FIXME: use the right span + let span = rustc_span::DUMMY_SP; + self.interior_visitor.record(ty, id, Some(self.scope), None, span, true); } } } diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 6de6b6ee479..ad44adb68c6 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -17,6 +17,7 @@ use rustc_middle::hir::place::ProjectionKind; use rustc_middle::mir::FakeReadCause; use rustc_middle::ty::{self, adjustment, AdtKind, Ty, TyCtxt}; use rustc_target::abi::VariantIdx; +use ty::BorrowKind::ImmBorrow; use crate::mem_categorization as mc; @@ -621,7 +622,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { FakeReadCause::ForMatchedPlace(closure_def_id), discr_place.hir_id, ); - self.walk_pat(discr_place, arm.pat); + self.walk_pat(discr_place, arm.pat, arm.guard.is_some()); if let Some(hir::Guard::If(e)) = arm.guard { self.consume_expr(e) @@ -645,12 +646,17 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { FakeReadCause::ForLet(closure_def_id), discr_place.hir_id, ); - self.walk_pat(discr_place, pat); + self.walk_pat(discr_place, pat, false); } /// The core driver for walking a pattern - fn walk_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) { - debug!("walk_pat(discr_place={:?}, pat={:?})", discr_place, pat); + fn walk_pat( + &mut self, + discr_place: &PlaceWithHirId<'tcx>, + pat: &hir::Pat<'_>, + has_guard: bool, + ) { + debug!("walk_pat(discr_place={:?}, pat={:?}, has_guard={:?})", discr_place, pat, has_guard); let tcx = self.tcx(); let ExprUseVisitor { ref mc, body_owner: _, ref mut delegate } = *self; @@ -671,6 +677,13 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { delegate.bind(binding_place, binding_place.hir_id); } + // Subtle: MIR desugaring introduces immutable borrows for each pattern + // binding when lowering pattern guards to ensure that the guard does not + // modify the scrutinee. + if has_guard { + delegate.borrow(place, discr_place.hir_id, ImmBorrow); + } + // It is also a borrow or copy/move of the value being matched. // In a cases of pattern like `let pat = upvar`, don't use the span // of the pattern, as this just looks confusing, instead use the span diff --git a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs index c818963e466..646365e4359 100644 --- a/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs +++ b/src/test/ui/generator/drop-tracking-yielding-in-match-guards.rs @@ -2,15 +2,6 @@ // edition:2018 // compile-flags: -Zdrop-tracking -// This test is derived from -// https://github.com/rust-lang/rust/issues/72651#issuecomment-668720468 - -// This test demonstrates that, in `async fn g()`, -// indeed a temporary borrow `y` from `x` is live -// while `f().await` is being evaluated. -// Thus, `&'_ u8` should be included in type signature -// of the underlying generator. - #![feature(generators)] fn main() { -- cgit 1.4.1-3-g733a5