diff options
| author | bors <bors@rust-lang.org> | 2020-04-19 10:01:16 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2020-04-19 10:01:16 +0000 |
| commit | 36b1a9296cde2b773771710e9bbd608fd2eca35f (patch) | |
| tree | 947177af14398a42dc1727bd075488b53cd65f63 | |
| parent | e7497a8ccb8885823e413f4e3a54412812b60528 (diff) | |
| parent | ae533151c3a29be2edb71398b7d9ed6e51e77f25 (diff) | |
| download | rust-36b1a9296cde2b773771710e9bbd608fd2eca35f.tar.gz rust-36b1a9296cde2b773771710e9bbd608fd2eca35f.zip | |
Auto merge of #70015 - jonas-schievink:gen-needs-drop, r=matthewjasper
Make `needs_drop` less pessimistic on generators Generators only have non-trivial drop logic when they may store (in upvars or across yields) a type that does. This prevents generation of some unnecessary MIR in simple generators. There might be some impact on compile times, but this is probably limited in real-world applications. ~~This builds off of https://github.com/rust-lang/rust/pull/69814 since that contains some fixes that are made relevant by *this* PR (see https://github.com/rust-lang/rust/pull/69814#issuecomment-599147269).~~ (this has been merged)
| -rw-r--r-- | src/librustc_middle/ty/util.rs | 8 | ||||
| -rw-r--r-- | src/librustc_ty/needs_drop.rs | 17 | ||||
| -rw-r--r-- | src/test/mir-opt/generator-drop-cleanup.rs | 3 | ||||
| -rw-r--r-- | src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir | 79 | ||||
| -rw-r--r-- | src/test/ui/generator/borrowing.stderr | 15 | ||||
| -rw-r--r-- | src/test/ui/generator/retain-resume-ref.stderr | 7 |
6 files changed, 85 insertions, 44 deletions
diff --git a/src/librustc_middle/ty/util.rs b/src/librustc_middle/ty/util.rs index 86575b01333..239507e19e0 100644 --- a/src/librustc_middle/ty/util.rs +++ b/src/librustc_middle/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 6b0104e164b..97994b465b5 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -99,6 +99,23 @@ where } } + ty::Generator(_, substs, _) => { + let substs = substs.as_generator(); + for upvar_ty in substs.upvar_tys() { + queue_type(self, upvar_ty); + } + + let witness = substs.witness(); + 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/mir-opt/generator-drop-cleanup.rs b/src/test/mir-opt/generator-drop-cleanup.rs index 1c3025d5e3f..3e9707c6491 100644 --- a/src/test/mir-opt/generator-drop-cleanup.rs +++ b/src/test/mir-opt/generator-drop-cleanup.rs @@ -1,11 +1,14 @@ #![feature(generators, generator_trait)] +// ignore-wasm32-bare compiled with panic=abort by default + // Regression test for #58892, generator drop shims should not have blocks // spuriously marked as cleanup // EMIT_MIR rustc.main-{{closure}}.generator_drop.0.mir fn main() { let gen = || { + let _s = String::new(); yield; }; } diff --git a/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir b/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir index 05ed0a641af..887a4dd2cd3 100644 --- a/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir +++ b/src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir @@ -1,53 +1,80 @@ // MIR for `main::{{closure}}#0` 0 generator_drop -// generator_layout = GeneratorLayout { field_tys: [], variant_fields: [[], [], [], []], storage_conflicts: BitMatrix { num_rows: 0, num_columns: 0, words: [], marker: PhantomData } } +// generator_layout = GeneratorLayout { field_tys: [std::string::String], variant_fields: [[], [], [], [_0]], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } } -fn main::{{closure}}#0(_1: *mut [generator@$DIR/generator-drop-cleanup.rs:8:15: 10:6 {()}]) -> () { - let mut _0: (); // return place in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 - let mut _2: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 - let _3: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:9:9: 9:14 - let mut _4: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:9:9: 9:14 - let mut _5: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:8:18: 8:18 - let mut _6: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 - let mut _7: isize; // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 +fn main::{{closure}}#0(_1: *mut [generator@$DIR/generator-drop-cleanup.rs:10:15: 13:6 {std::string::String, ()}]) -> () { + let mut _0: (); // return place in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + let mut _2: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + let _3: std::string::String; // in scope 0 at $DIR/generator-drop-cleanup.rs:11:13: 11:15 + let _4: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:12:9: 12:14 + let mut _5: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:12:9: 12:14 + let mut _7: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:10:18: 10:18 + let mut _8: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + let mut _9: isize; // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + scope 1 { + debug _s => (((*_1) as variant#3).0: std::string::String); // in scope 1 at $DIR/generator-drop-cleanup.rs:11:13: 11:15 + } + scope 2 { + let mut _6: std::vec::Vec<u8>; // in scope 2 at $DIR/generator-drop-cleanup.rs:11:18: 11:31 + scope 3 { + } + } bb0: { - _7 = discriminant((*_1)); // bb0[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 - switchInt(move _7) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; // bb0[1]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 + _9 = discriminant((*_1)); // bb0[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + switchInt(move _9) -> [0u32: bb7, 3u32: bb11, otherwise: bb12]; // bb0[1]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 } - bb1: { - StorageDead(_4); // bb1[0]: scope 0 at $DIR/generator-drop-cleanup.rs:9:13: 9:14 - StorageDead(_3); // bb1[1]: scope 0 at $DIR/generator-drop-cleanup.rs:9:14: 9:15 - goto -> bb5; // bb1[2]: scope 0 at $DIR/generator-drop-cleanup.rs:10:5: 10:6 + bb1 (cleanup): { + resume; // bb1[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 } - bb2: { - return; // bb2[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 + bb2 (cleanup): { + nop; // bb2[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6 + goto -> bb8; // bb2[1]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6 } bb3: { - return; // bb3[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 + StorageDead(_5); // bb3[0]: scope 1 at $DIR/generator-drop-cleanup.rs:12:13: 12:14 + StorageDead(_4); // bb3[1]: scope 1 at $DIR/generator-drop-cleanup.rs:12:14: 12:15 + drop((((*_1) as variant#3).0: std::string::String)) -> [return: bb4, unwind: bb2]; // bb3[2]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6 } bb4: { - goto -> bb6; // bb4[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 + nop; // bb4[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6 + goto -> bb9; // bb4[1]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6 } bb5: { - goto -> bb2; // bb5[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:5: 10:6 + return; // bb5[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 } bb6: { - goto -> bb3; // bb6[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 + return; // bb6[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 } bb7: { - StorageLive(_3); // bb7[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 - StorageLive(_4); // bb7[1]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 - goto -> bb1; // bb7[2]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 + goto -> bb10; // bb7[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + } + + bb8 (cleanup): { + goto -> bb1; // bb8[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6 + } + + bb9: { + goto -> bb5; // bb9[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6 + } + + bb10: { + goto -> bb6; // bb10[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + } + + bb11: { + StorageLive(_4); // bb11[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + StorageLive(_5); // bb11[1]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 + goto -> bb3; // bb11[2]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 } - bb8: { - return; // bb8[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6 + bb12: { + return; // bb12[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6 } } diff --git a/src/test/ui/generator/borrowing.stderr b/src/test/ui/generator/borrowing.stderr index 83987e19839..38e1ace8c4e 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 bc715c7030e..e33310d12d9 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 |
