From c7c2fa102463f484348850c6a8c8850abed486c2 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 15 Mar 2020 00:24:47 +0100 Subject: [PATCH 1/3] Make `needs_drop` less pessimistic on generators --- src/librustc/ty/util.rs | 8 +++----- src/librustc_ty/needs_drop.rs | 17 +++++++++++++++++ src/test/ui/generator/borrowing.stderr | 15 ++++++--------- src/test/ui/generator/retain-resume-ref.stderr | 7 +++---- 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 1f512f1dde7d6..8ace84b7832e6 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -1047,10 +1047,7 @@ pub fn needs_drop_components( // Foreign types can never have destructors. ty::Foreign(..) => Ok(SmallVec::new()), - // Pessimistically assume that all generators will require destructors - // as we don't know if a destructor is a noop or not until after the MIR - // state transformation pass. - ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop), + ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop), ty::Slice(ty) => needs_drop_components(ty, target_layout), ty::Array(elem_ty, size) => { @@ -1083,7 +1080,8 @@ pub fn needs_drop_components( | ty::Placeholder(..) | ty::Opaque(..) | ty::Infer(_) - | ty::Closure(..) => Ok(smallvec![ty]), + | ty::Closure(..) + | ty::Generator(..) => Ok(smallvec![ty]), } } diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs index 37af8168f87d9..cbbb2aa19fd83 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -99,6 +99,23 @@ where } } + ty::Generator(def_id, substs, _) => { + let substs = substs.as_generator(); + for upvar_ty in substs.upvar_tys(def_id, tcx) { + queue_type(self, upvar_ty); + } + + let witness = substs.witness(def_id, tcx); + let interior_tys = match &witness.kind { + ty::GeneratorWitness(tys) => tcx.erase_late_bound_regions(tys), + _ => bug!(), + }; + + for interior_ty in interior_tys { + queue_type(self, interior_ty); + } + } + // Check for a `Drop` impl and whether this is a union or // `ManuallyDrop`. If it's a struct or enum without a `Drop` // impl then check whether the field types need `Drop`. diff --git a/src/test/ui/generator/borrowing.stderr b/src/test/ui/generator/borrowing.stderr index 83987e19839ce..38e1ace8c4efb 100644 --- a/src/test/ui/generator/borrowing.stderr +++ b/src/test/ui/generator/borrowing.stderr @@ -1,19 +1,16 @@ error[E0597]: `a` does not live long enough --> $DIR/borrowing.rs:9:33 | +LL | let _b = { + | -- borrow later stored here +LL | let a = 3; LL | Pin::new(&mut || yield &a).resume(()) - | ----------^ - | | | - | | borrowed value does not live long enough + | -- ^ borrowed value does not live long enough + | | | value captured here by generator - | a temporary with access to the borrow is created here ... LL | LL | }; - | -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for generator - | | - | `a` dropped here while still borrowed - | - = note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block. + | - `a` dropped here while still borrowed error[E0597]: `a` does not live long enough --> $DIR/borrowing.rs:16:20 diff --git a/src/test/ui/generator/retain-resume-ref.stderr b/src/test/ui/generator/retain-resume-ref.stderr index bc715c7030eb3..e33310d12d9ef 100644 --- a/src/test/ui/generator/retain-resume-ref.stderr +++ b/src/test/ui/generator/retain-resume-ref.stderr @@ -4,10 +4,9 @@ error[E0499]: cannot borrow `thing` as mutable more than once at a time LL | gen.as_mut().resume(&mut thing); | ---------- first mutable borrow occurs here LL | gen.as_mut().resume(&mut thing); - | ^^^^^^^^^^ second mutable borrow occurs here -LL | -LL | } - | - first borrow might be used here, when `gen` is dropped and runs the destructor for generator + | ------ ^^^^^^^^^^ second mutable borrow occurs here + | | + | first borrow later used by call error: aborting due to previous error From 9ebc72fdf0422e1a1fe7925e0a203b42b76f54f2 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 15 Mar 2020 02:28:48 +0100 Subject: [PATCH 2/3] Adjust mir-opt test and make it drop something --- src/test/mir-opt/generator-drop-cleanup.rs | 45 ++++++++++++++-------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/src/test/mir-opt/generator-drop-cleanup.rs b/src/test/mir-opt/generator-drop-cleanup.rs index 278dc49c92605..c4742e8156938 100644 --- a/src/test/mir-opt/generator-drop-cleanup.rs +++ b/src/test/mir-opt/generator-drop-cleanup.rs @@ -5,6 +5,7 @@ fn main() { let gen = || { + let _s = String::new(); yield; }; } @@ -13,35 +14,49 @@ fn main() { // START rustc.main-{{closure}}.generator_drop.0.mir // bb0: { -// _7 = discriminant((*_1)); -// switchInt(move _7) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; +// _9 = discriminant((*_1)); +// switchInt(move _9) -> [0u32: bb7, 3u32: bb11, otherwise: bb12]; // } -// bb1: { -// StorageDead(_4); -// StorageDead(_3); -// goto -> bb5; +// bb1 (cleanup): { +// resume; // } -// bb2: { -// return; +// bb2 (cleanup): { +// nop; +// goto -> bb8; // } // bb3: { -// return; +// StorageDead(_5); +// StorageDead(_4); +// drop((((*_1) as variant#3).0: std::string::String)) -> [return: bb4, unwind: bb2]; // } // bb4: { -// goto -> bb6; +// nop; +// goto -> bb9; // } // bb5: { -// goto -> bb2; +// return; // } // bb6: { -// goto -> bb3; +// return; // } // bb7: { -// StorageLive(_3); -// StorageLive(_4); +// goto -> bb10; +// } +// bb8 (cleanup): { // goto -> bb1; // } -// bb8: { +// bb9: { +// goto -> bb5; +// } +// bb10: { +// goto -> bb6; +// } +// bb11: { +// StorageLive(_4); +// StorageLive(_5); +// goto -> bb3; +// } +// bb12: { // return; // } // END rustc.main-{{closure}}.generator_drop.0.mir From 1df764174d389849af8f36cc924da28bbc802b91 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 23 Mar 2020 19:47:24 +0100 Subject: [PATCH 3/3] Fix rebase fallout --- src/librustc_ty/needs_drop.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs index cbbb2aa19fd83..f8149cdce4d30 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -99,13 +99,13 @@ where } } - ty::Generator(def_id, substs, _) => { + ty::Generator(_, substs, _) => { let substs = substs.as_generator(); - for upvar_ty in substs.upvar_tys(def_id, tcx) { + for upvar_ty in substs.upvar_tys() { queue_type(self, upvar_ty); } - let witness = substs.witness(def_id, tcx); + let witness = substs.witness(); let interior_tys = match &witness.kind { ty::GeneratorWitness(tys) => tcx.erase_late_bound_regions(tys), _ => bug!(),